Re: [PATCH 5/5] xeniface: Bound NumberPermissions before checking the input length

[Mounah Honore Cesaire <cesaire.mounah@xxxxxxxxxx>]

On 10/20/26 2:23 PM, Tu Dinh wrote:
> On 20/10/2026 13:55, Césaire Mounah wrote:
>> IoctlStoreSetPermissions() compares InLen with
>> FIELD_OFFSET(XENIFACE_STORE_SET_PERMISSIONS_IN, Permissions[N]), whose
>> 64-bit result is truncated to ULONG. The count was only capped to 255
>> later, in __ConvertPermissions(), so the length check alone could be
>> satisfied by a huge count.
>>
>> Check the cap first, with the same STATUS_INVALID_PARAMETER as before.
>> The check in __ConvertPermissions() is kept.
>>
>> Assisted-by: Claude:Opus-5.5
>> Signed-off-by: Césaire Mounah <cesaire.mounah@xxxxxxxxxx>
>> ---
>>    src/xeniface/ioctl_store.c | 5 +++++
>>    1 file changed, 5 insertions(+)
>>
>> diff --git a/src/xeniface/ioctl_store.c b/src/xeniface/ioctl_store.c
>> index 97bef75..cb9f2ff 100644
>> --- a/src/xeniface/ioctl_store.c
>> +++ b/src/xeniface/ioctl_store.c
>> @@ -417,6 +417,11 @@ IoctlStoreSetPermissions(
>>            goto fail1;
>>        }
>>    
>> +    status = STATUS_INVALID_PARAMETER;
>> +    if (In->NumberPermissions > 255)
>> +        goto fail2;
>> +
> I think the check in __ConvertPermissions can then be removed.
Oh! I didn't get this comment right. Thought it was the check after 
__ConvertPermissions. Sorry.
>
>> +    status = STATUS_INVALID_BUFFER_SIZE;
>>        if (InLen != (ULONG)FIELD_OFFSET(XENIFACE_STORE_SET_PERMISSIONS_IN, 
>> Permissions[In->NumberPermissions]))
>>            goto fail2;
>>    
>
>


--
Césaire Mounah | Vates Windows Guest Tools Engineer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech