Re: [PATCH] 9p/xen: drain response work after unbinding IRQ

[Dominique Martinet]

Yizhou Zhao wrote on Tue, Jun 09, 2026 at 11:52:01PM +0800:
> commit ea4f1009408e ("9p/xen : Fix use after free bug in
> xen_9pfs_front_remove due to race condition") added cancel_work_sync()
> to keep xen_9pfs_front_free() from freeing a ring while
> p9_xen_response() is still running.
> 
> The work is currently drained before the event channel IRQ is unbound.
> That leaves a race window where xen_9pfs_front_event_handler() can run
> after cancel_work_sync() returns and queue p9_xen_response() again.  The
> following unbind_from_irqhandler() synchronizes with the IRQ handler, but
> it does not cancel work that the handler already queued.  The teardown
> can then free the ring and private data while the response work remains
> pending, and the worker later dereferences ring->priv and priv->client.
> 
> Unbind the event channel IRQ before draining the work.  Once the IRQ is
> unbound, no new response work can be queued by the backend event channel,
> and cancel_work_sync() waits for any work that was queued before or
> during the unbind.  It is then safe to release the ring resources.
> 
> Fixes: ea4f1009408e ("9p/xen : Fix use after free bug in xen_9pfs_front_remove due to race condition")
> Cc: stable@xxxxxxxxxxxxxxx
> Reported-by: Yizhou Zhao <zhaoyz24@xxxxxxxxxxxxxxxxxxxxx>
> Reported-by: Yuxiang Yang <yangyx22@xxxxxxxxxxxxxxxxxxxxx>
> Reported-by: Ao Wang <wangao@xxxxxxxxxx>
> Reported-by: Xuewei Feng <fengxw06@xxxxxxx>
> Reported-by: Qi Li <qli01@xxxxxxxxxxxxxxx>
> Reported-by: Ke Xu <xuke@xxxxxxxxxxxxxxx>
> Assisted-by: GLM:GLM-5.1
> Signed-off-by: Yizhou Zhao <zhaoyz24@xxxxxxxxxxxxxxxxxxxxx>

This one makes sense to me and I've picked it up, but I think
"9p/xen: fix use-after-free in p9_xen_request"[1] adding a refcount to
the front_priv is too much complexity vs. my understanding of the
current threat model -- I can pick it up if someome who understands xen
better than me has a proper look.

Stefano or Jürgen if you care / have time?

[1] https://lore.kernel.org/r/20260529103416.81378-1-zhaoyz24@xxxxxxxxxxxxxxxxxxxxx

Thanks,

> ---
>  net/9p/trans_xen.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/net/9p/trans_xen.c b/net/9p/trans_xen.c
> index f9fb2db7a066..73c75fbdfa21 100644
> --- a/net/9p/trans_xen.c
> +++ b/net/9p/trans_xen.c
> @@ -281,14 +281,14 @@ static void xen_9pfs_front_free(struct xen_9pfs_front_priv *priv)
>  		for (i = 0; i < XEN_9PFS_NUM_RINGS; i++) {
>  			struct xen_9pfs_dataring *ring = &priv->rings[i];
>  
> -			cancel_work_sync(&ring->work);
> -
>  			if (!ring->intf)
>  				break;
>  			if (ring->irq >= 0) {
>  				unbind_from_irqhandler(ring->irq, ring);
>  				ring->irq = -1;
>  			}
> +			cancel_work_sync(&ring->work);
> +
>  			if (ring->data.in) {
>  				for (j = 0; j < (1 << ring->intf->ring_order);
>  				     j++) {

-- 
Dominique Martinet | Asmadeus