Re: [PATCH 2/5] Fix potential Use-After-Free in __MacGetSpeed

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

On 10/09/2026 14:54, Owen Smith wrote:
> If the backend provides a xenstore value for "speed", it can include a unit
> character. When parsing this string, the Unit pointer would refer to a buffer
> that gets freed earlier.
> Change the parsing to store the Unit value as a single CHAR, so that the
> buffer can be freed without issue.
> 
> Assisted-by: ClaudeCode:claude-opus-4.8
> Signed-off-by: Owen Smith <owen.smith@xxxxxxxxxx>

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

> ---
>   src/xenvif/mac.c | 34 +++++++++++++++++++---------------
>   1 file changed, 19 insertions(+), 15 deletions(-)
> 
> diff --git a/src/xenvif/mac.c b/src/xenvif/mac.c
> index c20f281..3e4e5b7 100644
> --- a/src/xenvif/mac.c
> +++ b/src/xenvif/mac.c
> @@ -700,38 +700,42 @@ __MacGetSpeed(
>       PXENVIF_FRONTEND    Frontend;
>       PCHAR               Buffer;
>       ULONG64             Speed;
> -    PCHAR               Unit;
> +    CHAR                Unit;
>       NTSTATUS            status;
>   
>       Frontend = Mac->Frontend;
>   
> +    Speed = Mac->Speed;
> +    Unit = 'G';
> +
>       status = XENBUS_STORE(Read,
>                             &Mac->StoreInterface,
>                             NULL,
>                             FrontendGetPath(Mac->Frontend),
>                             "speed",
>                             &Buffer);
> -    if (!NT_SUCCESS(status)) {
> -        Speed = Mac->Speed;
> -        Unit = "G";
> -    } else {
> -        Speed = _strtoui64(Buffer, &Unit, 10);
> +    if (NT_SUCCESS(status)) {
> +        PCHAR           End;
> +
> +        Speed = _strtoui64(Buffer, &End, 10);
>           if (Speed == _UI64_MAX)
>               Speed = Mac->Speed;
> -        if (*Unit == '\0')
> -            Unit = "G";
> +
> +        if (*End != '\0') {
> +            Unit = *End;
> +
> +            if (*(End + 1) != '\0') {
> +                Warning("INVALID SPEED: %s\n", Buffer);
> +                Speed = 0;
> +            }
> +        }
>   
>           XENBUS_STORE(Free,
>                        &Mac->StoreInterface,
>                        Buffer);
>       }
>   
> -    if (*(Unit + 1) != '\0') {
> -        Warning("INVALID SPEED: %s\n", Buffer);
> -        return 0;
> -    }
> -
> -    switch (*Unit) {
> +    switch (Unit) {
>       case 'g':
>       case 'G':
>           Speed *= 1000000000ull;
> @@ -748,7 +752,7 @@ __MacGetSpeed(
>           break;
>   
>       default:
> -        Warning("INVALID SPEED UNIT: %c\n", *Unit);
> +        Warning("INVALID SPEED UNIT: %c\n", Unit);
>           return 0;
>       }
>   



--
Ngoc Tu Dinh | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech