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

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

On 13/10/2026 10:01, Owen Smith wrote:
>>>> 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.
>>
>> MAXULONG / PAGE_SIZE would be a better formulation still, only problem
>> being PAGE_SIZE is not defined for usermode targets.
> 
> Maximum number of pages allowed is also checked in IoctlGnttabMapForeignPages
> against a hard coded 1024*1024.
> It would be good to have the same constant/#define referred to in both places.
> 
> Eventually, this ends up in FdoPciHoleAllocate or FdoMemoryHoleAllocate, 
> depending
> on the "UseMemoryHole" setting.
> FdoPciHoleAllocate is limited to 4GiB - PAGE_SIZE bytes (i.e. 1024*1024 - 1 
> pages)
> FdoMemoryHoleAllocate's allocations doesnt specify a maximum size, but the 
> chances
> of allocating a contiguous 4GB set of pages would be problematic.
> 
> Its likely this check is excessively large, but I'm unsure as to what the 
> safe correct
> maximum would be.

I think in general it'd be nice to bound the usermode grant consumption 
to the actual free grant table space (and reserve some certain amount 
for the kernel), but that doesn't look simple to implement.

SeLockMemoryPrivilege may also be worth checking in the caller path of 
some ioctls.

> 
>>
>>>
>>>> +        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(
> 
> Owen



--
Ngoc Tu Dinh | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech