Re: [PATCH] Add ConfigFreeActive / ConfigFreeSzValue

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

On 13/11/2025 15:08, Owen Smith wrote:
> Values returned by ConfigGetActive should be freed by ConfigFreeActive,
> and values returned by ConfigQuerySystemStartOption should be freed by
> ConfigFreeSzValue, so that pool tag tracking is correct.
> 
> Signed-off-by: Owen Smith <owen.smith@xxxxxxxxxx>

Reviewed-by: Tu Dinh <ngoc-tu.dinh@xxxxxxxxxx>

> ---
>   include/xen.h       | 12 ++++++++++++
>   src/xen/config.c    | 25 ++++++++++++++++++++++---
>   src/xenbus/fdo.c    |  6 +++---
>   src/xenbus/unplug.c |  2 +-
>   4 files changed, 38 insertions(+), 7 deletions(-)
> 
> diff --git a/include/xen.h b/include/xen.h
> index 7016c86..8691a0f 100644
> --- a/include/xen.h
> +++ b/include/xen.h
> @@ -573,6 +573,12 @@ ConfigClearActive(
>       VOID
>       );
>   
> +XEN_API
> +VOID
> +ConfigFreeActive(
> +    _In_ PSTR   Value
> +    );
> +
>   XEN_API
>   NTSTATUS
>   ConfigRequestReboot(
> @@ -587,4 +593,10 @@ ConfigQuerySystemStartOption(
>       _Outptr_ PANSI_STRING   *Option
>       );
>   
> +XEN_API
> +VOID
> +ConfigFreeSzValue(
> +    _In_ PANSI_STRING       Array
> +    );
> +
>   #endif  // _XEN_H
> diff --git a/src/xen/config.c b/src/xen/config.c
> index 99980a6..2bc05df 100644
> --- a/src/xen/config.c
> +++ b/src/xen/config.c
> @@ -46,6 +46,7 @@
>   #include "util.h"
>   
>   #define MAXNAMELEN  128
> +#define CONFIG_TAG  'GFCX'
>   
>   //
>   // The canonical location for active device information is the XENFILT
> @@ -88,7 +89,7 @@ ConfigGetActive(
>           goto fail2;
>   
>       Length = Ansi[0].Length + sizeof (CHAR);
> -    *Value = __AllocatePoolWithTag(PagedPool, Length, 'SUB');
> +    *Value = __AllocatePoolWithTag(PagedPool, Length, CONFIG_TAG);
>   
>       status = STATUS_NO_MEMORY;
>       if (*Value == NULL)
> @@ -305,7 +306,7 @@ ConfigUpdateActive(
>   
>       status = ConfigGetActive("InstanceID", &ActiveInstanceID);
>       if (NT_SUCCESS(status)) {
> -        ExFreePool(ActiveInstanceID);
> +        ConfigFreeActive(ActiveInstanceID);
>       } else {
>           RtlInitAnsiString(&Ansi[0], InstanceID);
>   
> @@ -319,7 +320,7 @@ ConfigUpdateActive(
>   
>       status = ConfigGetActive("LocationInformation", 
> &ActiveLocationInformation);
>       if (NT_SUCCESS(status)) {
> -        ExFreePool(ActiveLocationInformation);
> +        ConfigFreeActive(ActiveLocationInformation);
>       } else {
>           RtlInitAnsiString(&Ansi[0], LocationInformation);
>   
> @@ -408,6 +409,15 @@ fail1:
>       return status;
>   }
>   
> +XEN_API
> +VOID
> +ConfigFreeActive(
> +    _In_ PSTR   Value
> +    )
> +{
> +    __FreePoolWithTag(Value, CONFIG_TAG);
> +}
> +
>   XEN_API
>   NTSTATUS
>   ConfigRequestReboot(
> @@ -496,3 +506,12 @@ ConfigQuerySystemStartOption(
>   {
>       return RegistryQuerySystemStartOption(Key, Option);
>   }
> +
> +XEN_API
> +VOID
> +ConfigFreeSzValue(
> +    _In_ PANSI_STRING       Array
> +    )
> +{
> +    RegistryFreeSzValue(Array);
> +}
> diff --git a/src/xenbus/fdo.c b/src/xenbus/fdo.c
> index e3de8b9..d093db8 100644
> --- a/src/xenbus/fdo.c
> +++ b/src/xenbus/fdo.c
> @@ -806,7 +806,7 @@ fallback:
>           if (Fdo->Active)
>               (VOID) ConfigUpdateActive(DeviceID, InstanceID, 
> LocationInformation);
>   
> -        ExFreePool(ActiveDeviceID);
> +        ConfigFreeActive(ActiveDeviceID);
>       } else {
>           status = ConfigSetActive(DeviceID, InstanceID, LocationInformation);
>           if (NT_SUCCESS(status))
> @@ -5746,7 +5746,7 @@ FdoBalloonInitialize(
>       else if (strcmp(Value, "ON") != 0)
>           Warning("UNRECOGNIZED VALUE OF %s: %s\n", Key, Value);
>   
> -    RegistryFreeSzValue(Option);
> +    ConfigFreeSzValue(Option);
>   
>   done:
>       return Enabled ?
> @@ -5782,7 +5782,7 @@ FdoSetWatchdog(
>   
>       Value = strtoul(Option->Buffer + sizeof (Key) - 1, NULL, 0);
>   
> -    RegistryFreeSzValue(Option);
> +    ConfigFreeSzValue(Option);
>   
>       if (Value && Value < 10) {
>           Warning("%us TOO SHORT (ROUNDING UP TO 10s)\n");
> diff --git a/src/xenbus/unplug.c b/src/xenbus/unplug.c
> index ca58c31..6bf8fb3 100644
> --- a/src/xenbus/unplug.c
> +++ b/src/xenbus/unplug.c
> @@ -175,7 +175,7 @@ UnplugBootEmulated(
>       else if (strcmp(Value, "FALSE") != 0)
>           Warning("UNRECOGNIZED VALUE OF %s: %s\n", KeyName, Value);
>   
> -    RegistryFreeSzValue(Option);
> +    ConfigFreeSzValue(Option);
>   
>   done:
>       ReleaseMutex(&Context->Mutex);



--
Ngoc Tu Dinh | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech