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

[Juergen Gross]

On 13.09.26 15:10, Dominique Martinet wrote:
> 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

I think [1] should be taken.

We try to harden frontends against potentially malicious backends running
in a driver domain. Such a backend could cause the device to be removed on
the frontend side, so the attack vector is present.

The only reason this doesn't qualify as a Xen security issue is that the
9pfs frontend hasn't been yet declared to be security supported regarding
attacks by the backend. Hopefully we will get there.


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature.asc
Description: OpenPGP digital signature