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

[Juergen Gross <jgross@suse.com>]

[-- Attachment #1.1.1: Type: text/plain, Size: 3916 bytes --]

On 13.07.22 17:03, dmitry.semenets@gmail.com wrote:
> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@epam.com>
> 
> 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@epam.com>
> Signed-off-by: Dmytro Semenets <dmytro_semenets@epam.com>
> ---
>   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 9195bd3b98..38658f30af 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;
> @@ -259,6 +259,12 @@ static int init_xs_srv(struct libxenvchan *ctrl, int domain, const char* xs_base
>   	if (!ctrl->xs_path)
>   		return -1;
>   
> +	buf = malloc(XENSTORE_ABS_PATH_MAX);
> +	if (!buf) {
> +		free(ctrl);
> +		return 0;
> +	}
> +
>   	xs = xs_open(0);
>   	if (!xs)
>   		goto fail;
> @@ -280,14 +286,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))
> @@ -303,6 +309,7 @@ retry_transaction:
>   	free(domid_str);
>   	xs_close(xs);
>    fail:
> +	free(buf);
>   	return ret;
>   }
>   
> @@ -419,13 +426,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;
> @@ -436,8 +450,9 @@ struct libxenvchan *libxenvchan_client_init(struct xentoollog_logger *logger,
>   	if (!xs)
>   		goto fail;
>   
> +
>   // 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;
> @@ -445,7 +460,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;
> @@ -475,6 +490,7 @@ struct libxenvchan *libxenvchan_client_init(struct xentoollog_logger *logger,
>    out:
>   	if (xs)
>   		xs_close(xs);
> +	free(buf);
>   	return ctrl;
>    fail:
>   	libxenvchan_close(ctrl);

I think you are leaking buf in case of "goto fail".


Juergen

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 3149 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 495 bytes --]