Re: [PATCH] xenbus: fix memory leak in failure path by freeing context

["Tu Dinh" <ngoc-tu.dinh@xxxxxxxxxx>]

On 27/08/2025 11:22, david ambu wrote:
> In certain error paths, the context memory was not being released,
> causing a potential memory leak. Ensure that __CacheFree() and
> __GnttabFree() are called on failure to properly release resources
> and reset the context.
> 
> Signed-off-by: david ambu <david.preetham@xxxxxxxxx>

The commit subject should be "Fix memory leak in failure path by freeing 
context" to follow the existing convention.

I'd also prefer seeing the patch split into 2 separate ones for cache 
and gnttab, in which case they can have the "cache: " and "gnttab: " 
prefixes to specify which component is being fixed.

> ---
>   src/xenbus/cache.c  | 2 ++
>   src/xenbus/gnttab.c | 2 ++
>   2 files changed, 4 insertions(+)
> 
> diff --git a/src/xenbus/cache.c b/src/xenbus/cache.c
> index d60ae29..e34504b 100644
> --- a/src/xenbus/cache.c
> +++ b/src/xenbus/cache.c
> @@ -1274,6 +1274,8 @@ fail2:
>   
>       RtlZeroMemory(&(*Context)->DebugInterface,
>                     sizeof (XENBUS_DEBUG_INTERFACE));

Please add a blank line here.

> +    __CacheFree(*Context);
> +    *Context = NULL;
>   
>   fail1:
>       Error("fail1 (%08x)\n", status);
> diff --git a/src/xenbus/gnttab.c b/src/xenbus/gnttab.c
> index 693e9b3..d3bd0ae 100644
> --- a/src/xenbus/gnttab.c
> +++ b/src/xenbus/gnttab.c
> @@ -1138,6 +1138,8 @@ GnttabInitialize(
>   
>   fail2:
>       Error("fail2\n");

Same here.

> +    __GnttabFree(*Context);
> +    *Context = NULL;
>   
>   fail1:
>       Error("fail1 (%08x)\n", status);



Ngoc Tu Dinh | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech