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

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


  • To: win-pv-devel@xxxxxxxxxxxxxxxxxxxx
  • From: "Tu Dinh" <ngoc-tu.dinh@xxxxxxxxxx>
  • Date: Mon, 30 Mar 2026 08:33:04 +0000
  • Authentication-results: eu.smtp.expurgate.cloud; dkim=pass header.s=mte1 header.d=mandrillapp.com header.i="@mandrillapp.com" header.h="From:Subject:To:Cc:Message-Id:In-Reply-To:References:Feedback-ID:Date:MIME-Version:Content-Type:Content-Transfer-Encoding"; dkim=pass header.s=mte1 header.d=vates.tech header.i="ngoc-tu.dinh@xxxxxxxxxx" header.h="From:Subject:To:Cc:Message-Id:In-Reply-To:References:Feedback-ID:Date:MIME-Version:Content-Type:Content-Transfer-Encoding"
  • Cc: "Tu Dinh" <ngoc-tu.dinh@xxxxxxxxxx>, "Owen Smith" <owen.smith@xxxxxxxxxx>
  • Delivery-date: Mon, 30 Mar 2026 08:33:08 +0000
  • Feedback-id: 30504962:30504962.20260330:md
  • List-id: Developer list for the Windows PV Drivers subproject <win-pv-devel.lists.xenproject.org>

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




 


Rackspace

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