[Top] [All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

  • To: Tu Dinh <ngoc-tu.dinh@xxxxxxxxxx>, win-pv-devel@xxxxxxxxxxxxxxxxxxxx
  • From: Rafał Wojdyła <omeg@xxxxxxxxxxxxxxxxxxxxxx>
  • Date: Thu, 28 May 2026 14:42:35 +0200
  • Authentication-results: eu.smtp.expurgate.cloud; dkim=pass header.s=fm3 header.d=invisiblethingslab.com header.i="@invisiblethingslab.com" header.h="Content-Transfer-Encoding:Content-Type:Date:From:In-Reply-To:Message-ID:MIME-Version:References:Subject:To"; dkim=pass header.s=fm3 header.d=messagingengine.com header.i="@messagingengine.com" header.h="Content-Transfer-Encoding:Content-Type:Date:Feedback-ID:From:In-Reply-To:Message-ID:MIME-Version:References:Subject:To:X-ME-Proxy:X-ME-Sender"
  • Delivery-date: Thu, 28 May 2026 12:42:44 +0000
  • Feedback-id: i409c4082:Fastmail
  • List-id: Developer list for the Windows PV Drivers subproject <win-pv-devel.lists.xenproject.org>


W dniu 28.05.2026 o 11:46, Tu Dinh pisze:

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).
You're right, two flushes is probably the best approach here. I'll send a fixup. 




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

 

©2013 Xen Project, A Linux Foundation Collaborative Project. All Rights Reserved.
Linux Foundation is a registered trademark of The Linux Foundation.
Xen Project is a trademark of The Linux Foundation.

Rackspace

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