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

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

On 27/05/2026 13:32, 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).
> 
> Use Context->Channel itself as the DPC readiness marker:
> - EvtchnFree() atomically swaps it to NULL as the first teardown step.
> - The DPC atomically reads it and exits without touching the channel
>    or signalling Context->Event if NULL.
> 
> Both bind paths publish the channel into Context->Channel atomically
> after acquiring a port from xenbus.
> 
> Signed-off-by: Rafał Wojdyła <omeg@xxxxxxxxxxxxxxxxxxxxxx>
> ---
>   src/xeniface/ioctl_evtchn.c | 64 ++++++++++++++++++++++---------------
>   1 file changed, 39 insertions(+), 25 deletions(-)
> 
> diff --git a/src/xeniface/ioctl_evtchn.c b/src/xeniface/ioctl_evtchn.c
> index 6d63996..4681a86 100644
> --- a/src/xeniface/ioctl_evtchn.c
> +++ b/src/xeniface/ioctl_evtchn.c
> @@ -49,6 +49,7 @@ EvtchnNotificationDpc(
>       )
>   {
>       PXENIFACE_EVTCHN_CONTEXT Context = _Context;
> +    PXENBUS_EVTCHN_CHANNEL   Channel;
>   
>       UNREFERENCED_PARAMETER(Dpc);
>       UNREFERENCED_PARAMETER(Argument1);
> @@ -56,11 +57,16 @@ EvtchnNotificationDpc(
>   
>       ASSERT(Context != NULL);
>   
> +    Channel = InterlockedCompareExchangePointer((PVOID *)&Context->Channel,
> +                                                NULL, NULL);
> +    if (Channel == NULL)
> +        return;
> +
>       KeSetEvent(Context->Event, 0, FALSE);
>   
>       (VOID) XENBUS_EVTCHN(Unmask,
>                            &Context->Fdo->EvtchnInterface,
> -                         Context->Channel,
> +                         Channel,
>                            FALSE,
>                            TRUE);
>   }
> @@ -98,14 +104,18 @@ EvtchnFree(
>       __inout  PXENIFACE_EVTCHN_CONTEXT Context
>       )
>   {
> +    PXENBUS_EVTCHN_CHANNEL Channel;
> +
>       ASSERT(KeGetCurrentIrql() == PASSIVE_LEVEL);
>   
>       Trace("Context %p, LocalPort %d, FO %p\n",
>                          Context, Context->LocalPort, Context->FileObject);
>   
> +    Channel = InterlockedExchangePointer((PVOID *)&Context->Channel, NULL);
> +
>       XENBUS_EVTCHN(Close,
>                     &Fdo->EvtchnInterface,
> -                  Context->Channel);
> +                  Channel);
>   
>       // There may still be a pending event at this time.
>       // Wait for our DPCs to complete.
> @@ -163,6 +173,7 @@ IoctlEvtchnBindUnbound(
>       PXENIFACE_EVTCHN_BIND_UNBOUND_IN In = Buffer;
>       PXENIFACE_EVTCHN_BIND_UNBOUND_OUT Out = Buffer;
>       PXENIFACE_EVTCHN_CONTEXT Context;
> +    PXENBUS_EVTCHN_CHANNEL Channel;
>   
>       status = STATUS_INVALID_BUFFER_SIZE;
>       if (InLen != sizeof(XENIFACE_EVTCHN_BIND_UNBOUND_IN) ||
> @@ -176,6 +187,7 @@ IoctlEvtchnBindUnbound(
>           goto fail2;
>   
>       Context->FileObject = FileObject;
> +    Context->Fdo = Fdo;
>   
>       Trace("> RemoteDomain %d, Mask %d, FO %p\n",
>                          In->RemoteDomain, In->Mask, FileObject);
> @@ -192,28 +204,28 @@ IoctlEvtchnBindUnbound(
>       KeInitializeDpc(&Context->Dpc, EvtchnNotificationDpc, Context);
>   
>       status = STATUS_UNSUCCESSFUL;
> -    Context->Channel = XENBUS_EVTCHN(Open,
> -                                     &Fdo->EvtchnInterface,
> -                                     XENBUS_EVTCHN_TYPE_UNBOUND,
> -                                     EvtchnInterruptHandler,
> -                                     Context,
> -                                     In->RemoteDomain,
> -                                     TRUE);
> -    if (Context->Channel == NULL)
> +    Channel = XENBUS_EVTCHN(Open,
> +                            &Fdo->EvtchnInterface,
> +                            XENBUS_EVTCHN_TYPE_UNBOUND,
> +                            EvtchnInterruptHandler,
> +                            Context,
> +                            In->RemoteDomain,
> +                            TRUE);
> +    if (Channel == NULL)
>           goto fail4;
>   
>       Context->LocalPort = XENBUS_EVTCHN(GetPort,
>                                          &Fdo->EvtchnInterface,
> -                                       Context->Channel);
> +                                       Channel);
>   
> -    Context->Fdo = Fdo;
> +    (VOID) InterlockedExchangePointer((PVOID *)&Context->Channel, Channel);
>   
>       ExInterlockedInsertTailList(&Fdo->EvtchnList, &Context->Entry, 
> &Fdo->EvtchnLock);
>   
>       if (!In->Mask) {
>           (VOID) XENBUS_EVTCHN(Unmask,
>                                &Fdo->EvtchnInterface,
> -                             Context->Channel,
> +                             Channel,
>                                FALSE,
>                                TRUE);
>       }
> @@ -256,6 +268,7 @@ IoctlEvtchnBindInterdomain(
>       PXENIFACE_EVTCHN_BIND_INTERDOMAIN_IN In = Buffer;
>       PXENIFACE_EVTCHN_BIND_INTERDOMAIN_OUT Out = Buffer;
>       PXENIFACE_EVTCHN_CONTEXT Context;
> +    PXENBUS_EVTCHN_CHANNEL Channel;
>   
>       status = STATUS_INVALID_BUFFER_SIZE;
>       if (InLen != sizeof(XENIFACE_EVTCHN_BIND_INTERDOMAIN_IN) ||
> @@ -269,6 +282,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);
> @@ -285,29 +299,29 @@ IoctlEvtchnBindInterdomain(
>       KeInitializeDpc(&Context->Dpc, EvtchnNotificationDpc, Context);
>   
>       status = STATUS_UNSUCCESSFUL;
> -    Context->Channel = XENBUS_EVTCHN(Open,
> -                                     &Fdo->EvtchnInterface,
> -                                     XENBUS_EVTCHN_TYPE_INTER_DOMAIN,
> -                                     EvtchnInterruptHandler,
> -                                     Context,
> -                                     In->RemoteDomain,
> -                                     In->RemotePort,
> -                                     TRUE);
> -    if (Context->Channel == NULL)
> +    Channel = XENBUS_EVTCHN(Open,
> +                            &Fdo->EvtchnInterface,
> +                            XENBUS_EVTCHN_TYPE_INTER_DOMAIN,
> +                            EvtchnInterruptHandler,
> +                            Context,
> +                            In->RemoteDomain,
> +                            In->RemotePort,
> +                            TRUE);
> +    if (Channel == NULL)
>           goto fail4;
>   
>       Context->LocalPort = XENBUS_EVTCHN(GetPort,
>                                          &Fdo->EvtchnInterface,
> -                                       Context->Channel);
> +                                       Channel);
>   
> -    Context->Fdo = Fdo;
> +    (VOID) InterlockedExchangePointer((PVOID *)&Context->Channel, Channel);
>   

KeFlushQueuedDpcs() is still needed before closing the channel to avoid 
racing with EvtchnNotificationDpc. I think overall it'd require 2 
flushes (1 for the channel itself and another to prevent pending DPCs 
from dereferencing a freed channel).

>       ExInterlockedInsertTailList(&Fdo->EvtchnList, &Context->Entry, 
> &Fdo->EvtchnLock);
>   
>       if (!In->Mask) {
>           (VOID) XENBUS_EVTCHN(Unmask,
>                                &Fdo->EvtchnInterface,
> -                             Context->Channel,
> +                             Channel,
>                                FALSE,
>                                TRUE);
>       }



--
Ngoc Tu Dinh | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech