Re: [PATCH] xeniface: fix use-after-free in RemoveWatch/SuspendDeregister

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

On 16/10/2026 15:35, david ambu wrote:
> IoctlStoreRemoveWatch and IoctlSuspendDeregister use the loop variable
> to decide whether the lookup succeeded. When no entry matches, Context
> is left pointing at the last entry visited. If that entry has the
> requested pointer value but belongs to a different FileObject, it is
> skipped (not unlinked), the post-loop check passes, and the context is
> freed while still linked in StoreWatchList / SuspendList. The next list
> walk, insert, cleanup or suspend notification then touches freed pool.
> 
> Track the match in a separate Found pointer that is only set alongside
> RemoveEntryList, and only free when Found is non-NULL.
> 
> Signed-off-by: david ambu <david.preetham@xxxxxxxxxx>

Very nice find, thanks.

Reviewed-by: Tu Dinh <ngoc-tu.dinh@xxxxxxxxxx>

> ---
>   src/xeniface/ioctl_store.c   | 8 +++++---
>   src/xeniface/ioctl_suspend.c | 8 +++++---
>   2 files changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/src/xeniface/ioctl_store.c b/src/xeniface/ioctl_store.c
> index c3beca2..97bef75 100644
> --- a/src/xeniface/ioctl_store.c
> +++ b/src/xeniface/ioctl_store.c
> @@ -678,7 +678,8 @@ IoctlStoreRemoveWatch(
>   {
>       NTSTATUS                        status;
>       PXENIFACE_STORE_REMOVE_WATCH_IN In = Buffer;
> -    PXENIFACE_STORE_CONTEXT         Context = NULL;
> +    PXENIFACE_STORE_CONTEXT         Context;
> +    PXENIFACE_STORE_CONTEXT         Found = NULL;
>       KIRQL                           Irql;
>       PLIST_ENTRY                     Node;
>   
> @@ -702,15 +703,16 @@ IoctlStoreRemoveWatch(
>           }
>   
>           RemoveEntryList(&Context->Entry);
> +        Found = Context;
>           break;
>       }
>       KeReleaseSpinLock(&Fdo->StoreWatchLock, Irql);
>   
>       status = STATUS_NOT_FOUND;
> -    if (Context == NULL || Context != In->Context)
> +    if (Found == NULL)
>           goto fail2;
>   
> -    StoreFreeWatch(Fdo, Context);
> +    StoreFreeWatch(Fdo, Found);
>   
>       return STATUS_SUCCESS;
>   
> diff --git a/src/xeniface/ioctl_suspend.c b/src/xeniface/ioctl_suspend.c
> index 7831c1b..339bc64 100644
> --- a/src/xeniface/ioctl_suspend.c
> +++ b/src/xeniface/ioctl_suspend.c
> @@ -160,7 +160,8 @@ IoctlSuspendDeregister(
>   {
>       NTSTATUS                        status;
>       PXENIFACE_SUSPEND_REGISTER_OUT  In = Buffer;
> -    PXENIFACE_SUSPEND_CONTEXT       Context = NULL;
> +    PXENIFACE_SUSPEND_CONTEXT       Context;
> +    PXENIFACE_SUSPEND_CONTEXT       Found = NULL;
>       KIRQL                           Irql;
>       PLIST_ENTRY                     Node;
>   
> @@ -184,15 +185,16 @@ IoctlSuspendDeregister(
>           }
>   
>           RemoveEntryList(&Context->Entry);
> +        Found = Context;
>           break;
>       }
>       KeReleaseSpinLock(&Fdo->SuspendLock, Irql);
>   
>       status = STATUS_NOT_FOUND;
> -    if (Context == NULL || Context != In->Context)
> +    if (Found == NULL)
>           goto fail2;
>   
> -    SuspendFreeEvent(Fdo, Context);
> +    SuspendFreeEvent(Fdo, Found);
>   
>       return STATUS_SUCCESS;
>   



--
Ngoc Tu Dinh | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech