Re: [PATCH v2] Call RingPoll/EvtchnUnmask within the ring lock

[Owen Smith <owen.smith@xxxxxxxxxx>]

Reviewed-by: Owen Smith <owen.smith@xxxxxxxxxx>

________________________________________
From: Tu Dinh <ngoc-tu.dinh@xxxxxxxxxx>
Sent: 30 March 2026 9:33 AM
To: win-pv-devel@xxxxxxxxxxxxxxxxxxxx
Cc: Tu Dinh; Owen Smith
Subject: [PATCH v2] Call RingPoll/EvtchnUnmask within the ring lock

The call to EvtchnUnmask accesses Channel outside of the ring lock.
Therefore, it can access a stale channel if the DPC is still running
after the channel has been closed in RingDisconnect. Since
RingDisconnect runs at DISPATCH_LEVEL, we cannot use KeFlushQueuedDpcs
and have to guard against the event channel's closure via the Enabled
flag instead.

Note that RingPoll is now also called within the ring lock, since it's
also vulnerable to teardown of the shared ring area. It also gains a
check to Ring->Enabled following the structure of XenVbd.

Signed-off-by: Tu Dinh <ngoc-tu.dinh@xxxxxxxxxx>
---
v2: Invert conditions. Call RingPoll within the lock too. Add a check
    for Ring->Enabled in RingPoll.
---
 src/xencons/ring.c | 26 ++++++++++++--------------
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/src/xencons/ring.c b/src/xencons/ring.c
index afa9311..866dc74 100644
--- a/src/xencons/ring.c
+++ b/src/xencons/ring.c
@@ -471,6 +471,9 @@ RingPoll(
     PCHAR               Buffer;
     NTSTATUS            status;

+    if (!Ring->Enabled)
+        return FALSE;
+
     for (;;) {
         ULONG           Read;

@@ -567,30 +570,25 @@ RingDpc(
     ASSERT(Ring != NULL);

     for (;;) {
-        BOOLEAN Enabled;
         BOOLEAN Retry;
         KIRQL   Irql;

         KeAcquireSpinLock(&Ring->Lock, &Irql);
-        Enabled = Ring->Enabled;
-        KeReleaseSpinLock(&Ring->Lock, Irql);
+        Retry = RingPoll(Ring);

-        if (!Enabled)
-            break;
+        if (!Retry && Ring->Enabled) {
+            (VOID) XENBUS_EVTCHN(Unmask,
+                                 &Ring->EvtchnInterface,
+                                 Ring->Channel,
+                                 FALSE,
+                                 FALSE);
+        }

-        KeRaiseIrql(DISPATCH_LEVEL, &Irql);
-        Retry = RingPoll(Ring);
-        KeLowerIrql(Irql);
+        KeReleaseSpinLock(&Ring->Lock, Irql);

         if (!Retry)
             break;
     }
-
-    (VOID) XENBUS_EVTCHN(Unmask,
-                         &Ring->EvtchnInterface,
-                         Ring->Channel,
-                         FALSE,
-                         FALSE);
 }

 _Function_class_(KSERVICE_ROUTINE)
--
2.53.0.windows.2


--
Ngoc Tu Dinh | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech