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

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


  • To: Rafał Wojdyła <omeg@xxxxxxxxxxxxxxxxxxxxxx>, win-pv-devel@xxxxxxxxxxxxxxxxxxxx
  • From: Tu Dinh <ngoc-tu.dinh@xxxxxxxxxx>
  • Date: Wed, 27 May 2026 11:02:07 +0200
  • Authentication-results: eu.smtp.expurgate.cloud; dkim=pass header.s=selector1 header.d=vates.tech header.i="@vates.tech" header.h="From:Subject:Date:Message-ID:To:MIME-Version:Content-Type:In-Reply-To:References:Feedback-ID"
  • Delivery-date: Wed, 27 May 2026 09:02:16 +0000
  • Feedback-id: default:8631fc262581453bbf619ec5b2062170:Sweego
  • List-id: Developer list for the Windows PV Drivers subproject <win-pv-devel.lists.xenproject.org>

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.


--
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®.