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

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

On 13/10/2026 11:10, Rafał Wojdyła wrote:
> On 13.10.2026 10:10, Tu Dinh wrote:
>> 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.
> 
> I'll finally be sending my rework of the grant/map code soon, hopefully 
> this week. Got caught up with other projects, but I'm testing it now. It 
> includes new interface versions with bounded batched maps/unmaps, per- 
> page error reporting and other improvements.

Great to hear. I can share our WIP test suite of evtchn/gnttab ioctls as 
well as a few private fixes we've been working on on our side.

> 
> What's the policy on retaining old xenbus interface versions?

Currently, old xenbus interface versions have to be retained 
indefinitely, because since b0b9cc139600 old drivers could still bind to 
devices without being aware of compatibility changes.

> 
> 
> Rafał Wojdyła
> Invisible Things Lab
> 



--
Ngoc Tu Dinh | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech