[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 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: Wed, 27 May 2026 11:29: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: Wed, 27 May 2026 09:29:43 +0000
- Feedback-id: i409c4082:Fastmail
- List-id: Developer list for the Windows PV Drivers subproject <win-pv-devel.lists.xenproject.org>
W dniu 27.05.2026 o 11:02, Tu Dinh pisze:
On 27/05/2026 10:59, Rafał Wojdyła wrote:
W dniu 27.05.2026 o 03:41, Tu Dinh pisze:
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).
From my understanding KeFlushQueuedDpcs() flushes what's *currently*
queued, so there would still be a window after it completes and before
Close() finishes when new events could arrive.
Yes, but that's enough for protecting Context->Channel, as future queued
DPCs will read Context->Ready=0 and won't try to use the channel.
You are correct, I've been staring at this code for too long. My concern
was also the KeSetEvent in the DPC, but that can be skipped if !Ready
regardless of mask state since the channel is already being torn down
(or being initialized). I'll send v2 after some more tests.
|
