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

[Tu Dinh <ngoc-tu.dinh@xxxxxxxxxx>]

On 27/05/2026 11:30, Owen Smith wrote:
> It does introduce a very small potential race on Open
> 
> EvtchnInterruptHandler and therefore EvtchnNotificationDpc can be executed 
> immediately after
> the call to XENBUS_EVTCHN(Open...), but as Context->Ready could not be set at 
> this point, then
> the event channel wont get unmasked in the DPC.
> 
> Owen
> 
> 

It sounds like Context->Channel is the real target of the DPC readiness 
and should be managed via an interlocked pointer in that case.

________________________________________
> From: win-pv-devel <win-pv-devel-bounces@xxxxxxxxxxxxxxxxxxxx> on behalf of 
> Tu Dinh <ngoc-tu.dinh@xxxxxxxxxx>
> Sent: 27 May 2026 10:02 AM
> To: Rafał Wojdyła; win-pv-devel@xxxxxxxxxxxxxxxxxxxx
> Subject: Re: [PATCH 1/2] Fix use-after-free in evtchn DPC
> 
> 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
> 



--
Ngoc Tu Dinh | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech