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

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

[Yizhou Zhao]

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@vger.kernel.org
Reported-by: Yizhou Zhao <zhaoyz24@mails.tsinghua.edu.cn>
Reported-by: Yuxiang Yang <yangyx22@mails.tsinghua.edu.cn>
Reported-by: Ao Wang <wangao@seu.edu.cn>
Reported-by: Xuewei Feng <fengxw06@126.com>
Reported-by: Qi Li <qli01@tsinghua.edu.cn>
Reported-by: Ke Xu <xuke@tsinghua.edu.cn>
Assisted-by: GLM:GLM-5.1
Signed-off-by: Yizhou Zhao <zhaoyz24@mails.tsinghua.edu.cn>
---
 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++) {
-- 
2.43.0

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@vger.kernel.org
> Reported-by: Yizhou Zhao <zhaoyz24@mails.tsinghua.edu.cn>
> Reported-by: Yuxiang Yang <yangyx22@mails.tsinghua.edu.cn>
> Reported-by: Ao Wang <wangao@seu.edu.cn>
> Reported-by: Xuewei Feng <fengxw06@126.com>
> Reported-by: Qi Li <qli01@tsinghua.edu.cn>
> Reported-by: Ke Xu <xuke@tsinghua.edu.cn>
> Assisted-by: GLM:GLM-5.1
> Signed-off-by: Yizhou Zhao <zhaoyz24@mails.tsinghua.edu.cn>

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@mails.tsinghua.edu.cn

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