Re: [PATCH 2/4] tools: allow vchan XenStore paths more then 64 bytes long

[Juergen Gross <jgross@xxxxxxxx>]

On 09.07.22 12:10, dmitry.semenets@xxxxxxxxx wrote:
> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx>
>
> Current vchan implementation, while dealing with XenStore paths,
> allocates 64 bytes buffer on the stack which may not be enough for
> some use-cases. Make the buffer longer to respect maximum allowed
> XenStore path of XENSTORE_ABS_PATH_MAX.
>
> Signed-off-by: Oleksandr Andrushchenko <oleksandr_andrushchenko@xxxxxxxx>
> ---
>   tools/libs/vchan/init.c | 28 ++++++++++++++++++++++------
>   1 file changed, 22 insertions(+), 6 deletions(-)
>
> diff --git a/tools/libs/vchan/init.c b/tools/libs/vchan/init.c
> index c6b8674ef5..17945c24a1 100644
> --- a/tools/libs/vchan/init.c
> +++ b/tools/libs/vchan/init.c
> @@ -249,7 +249,7 @@ static int init_xs_srv(struct libxenvchan *ctrl, int 
> domain, const char* xs_base
>         int ret = -1;
>         struct xs_handle *xs;
>         struct xs_permissions perms[2];
> -       char buf[64];
> +       char *buf;
>         char ref[16];
>         char* domid_str = NULL;
>         xs_transaction_t xs_trans = XBT_NULL;
> @@ -257,6 +257,12 @@ static int init_xs_srv(struct libxenvchan *ctrl, int 
> domain, const char* xs_base
>         // store the base path so we can clean up on server closure
>         ctrl->xs_path = strdup(xs_base);
>   
> +	buf = malloc(XENSTORE_ABS_PATH_MAX);
> +       if (!buf) {
> +               free(ctrl);
> +               return 0;
> +       }
> +
>         xs = xs_open(0);
>         if (!xs)
>                 goto fail;
> @@ -278,14 +284,14 @@ retry_transaction:
>                 goto fail_xs_open;
>   
>   	snprintf(ref, sizeof ref, "%d", ring_ref);
> -       snprintf(buf, sizeof buf, "%s/ring-ref", xs_base);
> +       snprintf(buf, XENSTORE_ABS_PATH_MAX, "%s/ring-ref", xs_base);
>         if (!xs_write(xs, xs_trans, buf, ref, strlen(ref)))
>                 goto fail_xs_open;
>         if (!xs_set_permissions(xs, xs_trans, buf, perms, 2))
>                 goto fail_xs_open;
>   
>   	snprintf(ref, sizeof ref, "%d", ctrl->event_port);
> -       snprintf(buf, sizeof buf, "%s/event-channel", xs_base);
> +       snprintf(buf, XENSTORE_ABS_PATH_MAX, "%s/event-channel", xs_base);
>         if (!xs_write(xs, xs_trans, buf, ref, strlen(ref)))
>                 goto fail_xs_open;
>         if (!xs_set_permissions(xs, xs_trans, buf, perms, 2))
> @@ -301,6 +307,7 @@ retry_transaction:
>         free(domid_str);
>         xs_close(xs);
>    fail:
> +       free(buf);
>         return ret;
>   }
>   
> @@ -418,13 +425,20 @@ struct libxenvchan *libxenvchan_client_init(struct xentoollog_logger *logger,
>   {
>         struct libxenvchan *ctrl = malloc(sizeof(struct libxenvchan));
>         struct xs_handle *xs = NULL;
> -       char buf[64];
> +       char *buf;
>         char *ref;
>         int ring_ref;
>         unsigned int len;
>   
>   	if (!ctrl)
>                 return 0;
> +
> +       buf = malloc(XENSTORE_ABS_PATH_MAX);
> +       if (!buf) {
> +               free(ctrl);
> +               return 0;
> +       }
> +
>         ctrl->ring = NULL;
>         ctrl->event = NULL;
>         ctrl->gnttab = NULL;
> @@ -435,8 +449,9 @@ struct libxenvchan *libxenvchan_client_init(struct 
> xentoollog_logger *logger,
>         if (!xs)
>                 goto fail;
You are leaking buf in this case.

>   
> +
>   // find xenstore entry
> -       snprintf(buf, sizeof buf, "%s/ring-ref", xs_path);
> +       snprintf(buf, XENSTORE_ABS_PATH_MAX, "%s/ring-ref", xs_path);
>         ref = xs_read(xs, 0, buf, &len);
>         if (!ref)
>                 goto fail;
> @@ -444,7 +459,7 @@ struct libxenvchan *libxenvchan_client_init(struct 
> xentoollog_logger *logger,
>         free(ref);
>         if (!ring_ref)
>                 goto fail;
> -       snprintf(buf, sizeof buf, "%s/event-channel", xs_path);
> +       snprintf(buf, XENSTORE_ABS_PATH_MAX, "%s/event-channel", xs_path);
>         ref = xs_read(xs, 0, buf, &len);
>         if (!ref)
>                 goto fail;
> @@ -474,6 +489,7 @@ struct libxenvchan *libxenvchan_client_init(struct 
> xentoollog_logger *logger,
>    out:
>         if (xs)
>                 xs_close(xs);
> +       free(buf);
>         return ctrl;
>    fail:
>         libxenvchan_close(ctrl);
Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature
Description: OpenPGP digital signature