Re: [PATCH 10/10] xencontrol: guard against NumberPages overflow in gnttab size calculation

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

On 08/10/2026 14:05, david ambu wrote:
> FIELD_OFFSET(T, References[NumberPages]) with a caller-controlled
> NumberPages can overflow ULONG, producing a Size smaller than the
> actual allocation needed, leading to a heap buffer overrun.
> 
> Add a guard rejecting NumberPages == 0 or > XENCONTROL_GNTTAB_MAX_PAGES
> (1M pages = 4 GB) in both XcGnttabPermitForeignAccess2 and
> XcGnttabMapForeignPages. Define the limit as a named constant in
> xencontrol.h.
> 
> Signed-off-by: david ambu <david.preetham@xxxxxxxxxx>
> Assisted by Claude Sonnet 4.6
> ---
>   include/xencontrol.h        |  3 +++
>   src/xencontrol/xencontrol.c | 10 ++++++++++
>   2 files changed, 13 insertions(+)
> 
> diff --git a/include/xencontrol.h b/include/xencontrol.h
> index 4b916ec..e95e257 100644
> --- a/include/xencontrol.h
> +++ b/include/xencontrol.h
> @@ -163,6 +163,9 @@ XcEvtchnUnmask(
>       _In_ ULONG                  LocalPort
>       );
>   
> +/*! Maximum NumberPages accepted by gnttab permit/map functions */
> +#define XENCONTROL_GNTTAB_MAX_PAGES (1024 * 1024)
> +
>   /*! \brief Grant a \a RemoteDomain permission to access local newly 
> allocated memory pages
>       \param Xc Xencontrol handle returned by XcOpen()
>       \param RemoteDomain ID of a remote domain that is being granted access
> diff --git a/src/xencontrol/xencontrol.c b/src/xencontrol/xencontrol.c
> index a68dcf6..8f21867 100644
> --- a/src/xencontrol/xencontrol.c
> +++ b/src/xencontrol/xencontrol.c
> @@ -407,6 +407,11 @@ XcGnttabPermitForeignAccess2(
>       BOOL                                            Success;
>       DWORD                                           Status;
>   
> +    if (NumberPages == 0 || NumberPages > XENCONTROL_GNTTAB_MAX_PAGES) {

I believe the boundary condition of NumberPages == 
XENCONTROL_GNTTAB_MAX_PAGES still causes an overflow? So 
XENCONTROL_GNTTAB_MAX_PAGES should be (1024 * 1024 - 1) instead.

> +        Log(XLL_ERROR, L"Invalid NumberPages %lu", NumberPages);
> +        return ERROR_INVALID_PARAMETER;
> +    }
> +
>       In.RemoteDomain = RemoteDomain;
>       In.Address = Address;
>       In.NumberPages = NumberPages;
> @@ -508,6 +513,11 @@ XcGnttabMapForeignPages(
>       BOOL                                        Success;
>       DWORD                                       Status = ERROR_OUTOFMEMORY;
>   
> +    if (NumberPages == 0 || NumberPages > XENCONTROL_GNTTAB_MAX_PAGES) {
> +        Log(XLL_ERROR, L"Invalid NumberPages %lu", NumberPages);
> +        return ERROR_INVALID_PARAMETER;
> +    }
> +
>       Size = (ULONG)FIELD_OFFSET(XENIFACE_GNTTAB_MAP_FOREIGN_PAGES_IN_V2, 
> References[NumberPages]);
>       In = malloc(Size);
>       if (!In)



--
Ngoc Tu Dinh | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech