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

Re: [PATCH 1/2] Fix use-after-free in evtchn DPC


  • To: Rafał Wojdyła <omeg@xxxxxxxxxxxxxxxxxxxxxx>, win-pv-devel@xxxxxxxxxxxxxxxxxxxx
  • From: Tu Dinh <ngoc-tu.dinh@xxxxxxxxxx>
  • Date: Wed, 27 May 2026 03:41:36 +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: Wed, 27 May 2026 01:41:49 +0000
  • Feedback-id: default:8631fc262581453bbf619ec5b2062170:Sweego
  • List-id: Developer list for the Windows PV Drivers subproject <win-pv-devel.lists.xenproject.org>

On 26/05/2026 18:30, Rafał Wojdyła wrote:
> EvtchnFree() calls XENBUS_EVTCHN(Close) and then KeFlushQueuedDpcs()
> to drain any pending DPCs. A DPC queued just before Close returns can
> run in the window between Close completing and the flush draining it,
> at which point it dereferences Context->Channel inside
> XENBUS_EVTCHN(Unmask).
> 
> Also move Context->Fdo initialization to before opening the channel
> so Context is fully populated by the time the channel can fire its first
> interrupt.
> 
> Signed-off-by: Rafał Wojdyła <omeg@xxxxxxxxxxxxxxxxxxxxxx>
> ---
>   src/xeniface/ioctl_evtchn.c | 21 ++++++++++++++-------
>   src/xeniface/ioctls.h       |  1 +
>   2 files changed, 15 insertions(+), 7 deletions(-)
> 
> diff --git a/src/xeniface/ioctl_evtchn.c b/src/xeniface/ioctl_evtchn.c
> index 6d63996..e38e25f 100644
> --- a/src/xeniface/ioctl_evtchn.c
> +++ b/src/xeniface/ioctl_evtchn.c
> @@ -58,11 +58,14 @@ EvtchnNotificationDpc(
>   
>       KeSetEvent(Context->Event, 0, FALSE);
>   
> -    (VOID) XENBUS_EVTCHN(Unmask,
> -                         &Context->Fdo->EvtchnInterface,
> -                         Context->Channel,
> -                         FALSE,
> -                         TRUE);
> +    if (InterlockedCompareExchange(&Context->Ready, 1, 1) == 1) {
> +        (VOID) XENBUS_EVTCHN(Unmask,
> +                             &Context->Fdo->EvtchnInterface,
> +                             Context->Channel,
> +                             FALSE,
> +                             TRUE);
> +
> +    }
>   }
>   
>   _Function_class_(KSERVICE_ROUTINE)
> @@ -103,6 +106,8 @@ EvtchnFree(
>       Trace("Context %p, LocalPort %d, FO %p\n",
>                          Context, Context->LocalPort, Context->FileObject);
>   
> +    (VOID) InterlockedExchange(&Context->Ready, 0);
> +

Shouldn't KeFlushQueuedDpcs() be called before closing the channel? 
Otherwise there could still be a DPC which has captured Context->Ready=1 
before InterlockedExchange is called, and then still running 
concurrently with the XENBUS_EVTCHN(Close).

>       XENBUS_EVTCHN(Close,
>                     &Fdo->EvtchnInterface,
>                     Context->Channel);
> @@ -176,6 +181,7 @@ IoctlEvtchnBindUnbound(
>           goto fail2;
>   
>       Context->FileObject = FileObject;
> +    Context->Fdo = Fdo;
>   
>       Trace("> RemoteDomain %d, Mask %d, FO %p\n",
>                          In->RemoteDomain, In->Mask, FileObject);
> @@ -206,7 +212,7 @@ IoctlEvtchnBindUnbound(
>                                          &Fdo->EvtchnInterface,
>                                          Context->Channel);
>   
> -    Context->Fdo = Fdo;
> +    (VOID) InterlockedExchange(&Context->Ready, 1);
>   
>       ExInterlockedInsertTailList(&Fdo->EvtchnList, &Context->Entry, 
> &Fdo->EvtchnLock);
>   
> @@ -269,6 +275,7 @@ IoctlEvtchnBindInterdomain(
>           goto fail2;
>   
>       Context->FileObject = FileObject;
> +    Context->Fdo = Fdo;
>   
>       Trace("> RemoteDomain %d, RemotePort %lu, Mask %d, FO %p\n",
>                          In->RemoteDomain, In->RemotePort, In->Mask, 
> FileObject);
> @@ -300,7 +307,7 @@ IoctlEvtchnBindInterdomain(
>                                          &Fdo->EvtchnInterface,
>                                          Context->Channel);
>   
> -    Context->Fdo = Fdo;
> +    (VOID) InterlockedExchange(&Context->Ready, 1);
>   
>       ExInterlockedInsertTailList(&Fdo->EvtchnList, &Context->Entry, 
> &Fdo->EvtchnLock);
>   
> diff --git a/src/xeniface/ioctls.h b/src/xeniface/ioctls.h
> index b4c8de2..f5eddbf 100644
> --- a/src/xeniface/ioctls.h
> +++ b/src/xeniface/ioctls.h
> @@ -53,6 +53,7 @@ typedef struct _XENIFACE_EVTCHN_CONTEXT {
>       PXENIFACE_FDO          Fdo;
>       KDPC                   Dpc;
>       PVOID                  FileObject;
> +    volatile LONG          Ready;
>   } XENIFACE_EVTCHN_CONTEXT, *PXENIFACE_EVTCHN_CONTEXT;
>   
>   typedef struct _XENIFACE_SUSPEND_CONTEXT {



--
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®.