[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


  • To: Césaire Mounah <cesaire.mounah@xxxxxxxxxx>, win-pv-devel@xxxxxxxxxxxxxxxxxxxx
  • From: Tu Dinh <ngoc-tu.dinh@xxxxxxxxxx>
  • Date: Thu, 22 Oct 2026 13:56:07 +0200
  • Authentication-results: eu.smtp.expurgate.cloud; dkim=pass header.s=selector1 header.d=vates.tech header.i="@vates.tech" header.h="From:Subject:Date:Message-ID:To:MIME-Version:Content-Type:In-Reply-To:References:Feedback-ID"
  • Delivery-date: Thu, 22 Oct 2026 11:56:14 +0000
  • Feedback-id: default:8631fc262581453bbf619ec5b2062170:Sweego
  • List-id: Developer list for the Windows PV Drivers subproject <win-pv-devel.lists.xenproject.org>

On 22/10/2026 13:54, 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 removed and fails are updated
> accordingly.
> 
> Signed-off-by: Césaire Mounah <cesaire.mounah@xxxxxxxxxx>
> ---
>   src/xeniface/ioctl_store.c | 11 +++++++----
>   1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/src/xeniface/ioctl_store.c b/src/xeniface/ioctl_store.c
> index 97bef75..d0c73ef 100644
> --- a/src/xeniface/ioctl_store.c
> +++ b/src/xeniface/ioctl_store.c
> @@ -417,18 +417,21 @@ IoctlStoreSetPermissions(
>           goto fail1;
>       }
>   
> -    if (InLen != (ULONG)FIELD_OFFSET(XENIFACE_STORE_SET_PERMISSIONS_IN, 
> Permissions[In->NumberPermissions]))
> +    status = STATUS_INVALID_PARAMETER;
> +    if (In->NumberPermissions > 255)
>           goto fail2;
>   
> +    status = STATUS_INVALID_BUFFER_SIZE;
> +    if (InLen != (ULONG)FIELD_OFFSET(XENIFACE_STORE_SET_PERMISSIONS_IN, 
> Permissions[In->NumberPermissions]))
> +        goto fail3;
> +
>       status = STATUS_INVALID_PARAMETER;
>       if (In->PathLength == 0 ||
>           In->PathLength > XENSTORE_ABS_PATH_MAX) {
> -        goto fail3;
> +        goto fail4;
>       }
>   
>       Permissions = __ConvertPermissions(In->NumberPermissions, 
> In->Permissions);
> -    if (Permissions == NULL)
> -        goto fail4;

This check was removed for some reason.

Please double check your patch...

>   
>       status = __CaptureUserBuffer(In->Path, In->PathLength, &Path);
>       if (!NT_SUCCESS(status))



--
Ngoc Tu Dinh | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.