[PATCH 0/1] xen-blkfront: unbind irq before tearing down ring and shadow requests

[PATCH 0/1] xen-blkfront: unbind irq before tearing down ring and shadow requests

[Yuchao Zhang]

Hi Roger, Juergen, Stefano, and Jens,

This patch addresses a race condition during device disconnect and
ring teardown in drivers/block/xen-blkfront.c.

Problem:

In blkif_free_ring(), the driver currently cleans up all persistent
grants, frees indirect pages, frees the shadow request structures
(rinfo->shadow[i].grants_used and rinfo->shadow[i].sg), and tears
down the shared ring via xenbus_teardown_ring().  Only after all these
deallocations does it invoke unbind_from_irqhandler().

Because the event channel interrupt (blkif_interrupt) remains active
throughout this teardown procedure, a completion interrupt received
from the backend runs blkif_interrupt() concurrently on another CPU.
Since blkif_free_ring() tears the ring and shadow structures down
without holding rinfo->ring_lock, this races against the cleanup loop,
leading to use-after-free and NULL pointer dereferences when accessing
rinfo->ring.sring, rinfo->shadow[id].grants_used, or
rinfo->shadow[id].sg.

Fix:

Move unbind_from_irqhandler() to the beginning of blkif_free_ring().
This immediately unbinds the event channel and synchronizes with any
in-flight interrupt handlers via free_irq(), guaranteeing that no
interrupts execute concurrently while ring memory, grants, and shadow
structures are being freed.

This matches the teardown ordering already used in
drivers/net/xen-netfront.c (xennet_disconnect_backend()).

Best regards,
Yuchao Zhang

Yuchao Zhang (1):
  xen-blkfront: unbind irq before tearing down ring and shadow requests

 drivers/block/xen-blkfront.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

-- 
2.53.0

[PATCH 1/1] xen-blkfront: unbind irq before tearing down ring and shadow requests

[Yuchao Zhang]

In blkif_free_ring(), the driver tears down the ring's persistent grants,
shadow request arrays, and shared ring structure (xenbus_teardown_ring),
and only calls unbind_from_irqhandler() at the very end.

While blkif_free_ring() is freeing persistent grants and clearing the
shadow array, the event channel interrupt (blkif_interrupt) is still
registered and active.  If an interrupt arrives from the backend during
this teardown window, blkif_interrupt() reads rinfo->ring.sring and,
via blkif_completion(), accesses rinfo->shadow[id].grants_used and
rinfo->shadow[id].sg.  blkif_free_ring() tears these structures down
without holding rinfo->ring_lock, and the handler only checks
info->connected at entry, so this is a real race resulting in a
use-after-free or NULL pointer dereference.

Fix this by moving unbind_from_irqhandler() to the beginning of
blkif_free_ring().  Calling unbind_from_irqhandler() first frees the
IRQ and synchronizes with any in-flight interrupt handlers on other CPUs
before ring memory and shadow request structures are deallocated,
matching the teardown order in drivers/net/xen-netfront.c.

Fixes: 907c3eb18e0b ("xen-blkfront: convert to blk-mq APIs")
Cc: stable@vger.kernel.org
Signed-off-by: Yuchao Zhang <ndaugoing@gmail.com>
---
 drivers/block/xen-blkfront.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index 8dad7bf5f664..e70b78ca4df2 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -1210,6 +1210,10 @@ static void blkif_free_ring(struct blkfront_ring_info *rinfo)
 	struct blkfront_info *info = rinfo->dev_info;
 	int i, j, segs;
 
+	if (rinfo->irq)
+		unbind_from_irqhandler(rinfo->irq, rinfo);
+	rinfo->evtchn = rinfo->irq = 0;
+
 	/*
 	 * Remove indirect pages, this only happens when using indirect
 	 * descriptors but not persistent grants
@@ -1292,10 +1296,6 @@ static void blkif_free_ring(struct blkfront_ring_info *rinfo)
 	/* Free resources associated with old device channel. */
 	xenbus_teardown_ring((void **)&rinfo->ring.sring, info->nr_ring_pages,
 			     rinfo->ring_ref);
-
-	if (rinfo->irq)
-		unbind_from_irqhandler(rinfo->irq, rinfo);
-	rinfo->evtchn = rinfo->irq = 0;
 }
 
 static void blkif_free(struct blkfront_info *info, int suspend)
-- 
2.53.0

Re: [PATCH 1/1] xen-blkfront: unbind irq before tearing down ring and shadow requests

[Roger Pau Monné]

On Fri, Sep 18, 2026 at 07:43:54PM +0800, Yuchao Zhang wrote:
> In blkif_free_ring(), the driver tears down the ring's persistent grants,
> shadow request arrays, and shared ring structure (xenbus_teardown_ring),
> and only calls unbind_from_irqhandler() at the very end.
> 
> While blkif_free_ring() is freeing persistent grants and clearing the
> shadow array, the event channel interrupt (blkif_interrupt) is still
> registered and active.  If an interrupt arrives from the backend during
> this teardown window, blkif_interrupt() reads rinfo->ring.sring and,
> via blkif_completion(), accesses rinfo->shadow[id].grants_used and
> rinfo->shadow[id].sg.  blkif_free_ring() tears these structures down
> without holding rinfo->ring_lock, and the handler only checks
> info->connected at entry, so this is a real race resulting in a
> use-after-free or NULL pointer dereference.
> 
> Fix this by moving unbind_from_irqhandler() to the beginning of
> blkif_free_ring().  Calling unbind_from_irqhandler() first frees the
> IRQ and synchronizes with any in-flight interrupt handlers on other CPUs
> before ring memory and shadow request structures are deallocated,
> matching the teardown order in drivers/net/xen-netfront.c.
> 
> Fixes: 907c3eb18e0b ("xen-blkfront: convert to blk-mq APIs")

Are you sure this is the commit that introduced the issue?  I think
it's:

11659569f720 xen/blkfront: split per device io_lock

The commit that split the lock and removed the usage of
rinfo->ring_lock in the interrupt handler.

> Cc: stable@vger.kernel.org
> Signed-off-by: Yuchao Zhang <ndaugoing@gmail.com>
> ---
>  drivers/block/xen-blkfront.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
> index 8dad7bf5f664..e70b78ca4df2 100644
> --- a/drivers/block/xen-blkfront.c
> +++ b/drivers/block/xen-blkfront.c
> @@ -1210,6 +1210,10 @@ static void blkif_free_ring(struct blkfront_ring_info *rinfo)
>  	struct blkfront_info *info = rinfo->dev_info;
>  	int i, j, segs;
>  
> +	if (rinfo->irq)
> +		unbind_from_irqhandler(rinfo->irq, rinfo);
> +	rinfo->evtchn = rinfo->irq = 0;

Please add a comment that interrupt teardown must be done ahead of
freeing of queue related data, otherwise the interrupt handler can
race with the cleanup.

Thanks, Roger.

Re: [PATCH 1/1] xen-blkfront: unbind irq before tearing down ring and shadow requests

[Yuchao Zhang]

Hi Roger,

Thanks for the review and catching the right origin commit.

You are completely right: commit 11659569f720 ("xen/blkfront: split per device
io_lock") removed the lock protection from the interrupt handler and introduced
the race window.

I have updated the Fixes tag to point to 11659569f720 and added the requested
comment before unbind_from_irqhandler() explaining that interrupt teardown must
precede queue data cleanup.

v2 patch has been sent as reply to this thread.

Thanks,
Yuchao

[PATCH v2] xen-blkfront: unbind irq before tearing down ring and shadow requests

[Yuchao Zhang]

In blkif_free_ring(), the driver tears down the ring's persistent grants,
shadow request arrays, and shared ring structure (xenbus_teardown_ring),
and only calls unbind_from_irqhandler() at the very end.

While blkif_free_ring() is freeing persistent grants and clearing the
shadow array, the event channel interrupt (blkif_interrupt) is still
registered and active.  If an interrupt arrives from the backend during
this teardown window, blkif_interrupt() reads rinfo->ring.sring and,
via blkif_completion(), accesses rinfo->shadow[id].grants_used and
rinfo->shadow[id].sg.  blkif_free_ring() tears these structures down
without holding rinfo->ring_lock, and the handler only checks
info->connected at entry, so this is a real race resulting in a
use-after-free or NULL pointer dereference.

Fix this by moving unbind_from_irqhandler() to the beginning of
blkif_free_ring().  Calling unbind_from_irqhandler() first frees the
IRQ and synchronizes with any in-flight interrupt handlers on other CPUs
before ring memory and shadow request structures are deallocated,
matching the teardown order in drivers/net/xen-netfront.c.

Fixes: 11659569f720 ("xen/blkfront: split per device io_lock")
Cc: stable@vger.kernel.org
Signed-off-by: Yuchao Zhang <ndaugoing@gmail.com>
---
v2:
 - Update Fixes tag to 11659569f720 ("xen/blkfront: split per device io_lock")
   per Roger Pau Monné.
 - Add comment in blkif_free_ring() noting that interrupt teardown must precede
   freeing queue-related data.

 drivers/block/xen-blkfront.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index 8dad7bf5f664..86f5dd3aced1 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -1210,6 +1210,14 @@ static void blkif_free_ring(struct blkfront_ring_info *rinfo)
 	struct blkfront_info *info = rinfo->dev_info;
 	int i, j, segs;
 
+	/*
+	 * Interrupt teardown must be done ahead of freeing queue-related
+	 * data, otherwise the interrupt handler can race with the cleanup.
+	 */
+	if (rinfo->irq)
+		unbind_from_irqhandler(rinfo->irq, rinfo);
+	rinfo->evtchn = rinfo->irq = 0;
+
 	/*
 	 * Remove indirect pages, this only happens when using indirect
 	 * descriptors but not persistent grants
@@ -1292,10 +1300,6 @@ static void blkif_free_ring(struct blkfront_ring_info *rinfo)
 	/* Free resources associated with old device channel. */
 	xenbus_teardown_ring((void **)&rinfo->ring.sring, info->nr_ring_pages,
 			     rinfo->ring_ref);
-
-	if (rinfo->irq)
-		unbind_from_irqhandler(rinfo->irq, rinfo);
-	rinfo->evtchn = rinfo->irq = 0;
 }
 
 static void blkif_free(struct blkfront_info *info, int suspend)
-- 
2.53.0

Re: [PATCH v2] xen-blkfront: unbind irq before tearing down ring and shadow requests

[Roger Pau Monné]

On Tue, Sep 22, 2026 at 03:21:55PM +0800, Yuchao Zhang wrote:
> In blkif_free_ring(), the driver tears down the ring's persistent grants,
> shadow request arrays, and shared ring structure (xenbus_teardown_ring),
> and only calls unbind_from_irqhandler() at the very end.
> 
> While blkif_free_ring() is freeing persistent grants and clearing the
> shadow array, the event channel interrupt (blkif_interrupt) is still
> registered and active.  If an interrupt arrives from the backend during
> this teardown window, blkif_interrupt() reads rinfo->ring.sring and,
> via blkif_completion(), accesses rinfo->shadow[id].grants_used and
> rinfo->shadow[id].sg.  blkif_free_ring() tears these structures down
> without holding rinfo->ring_lock, and the handler only checks
> info->connected at entry, so this is a real race resulting in a
> use-after-free or NULL pointer dereference.
> 
> Fix this by moving unbind_from_irqhandler() to the beginning of
> blkif_free_ring().  Calling unbind_from_irqhandler() first frees the
> IRQ and synchronizes with any in-flight interrupt handlers on other CPUs
> before ring memory and shadow request structures are deallocated,
> matching the teardown order in drivers/net/xen-netfront.c.
> 
> Fixes: 11659569f720 ("xen/blkfront: split per device io_lock")
> Cc: stable@vger.kernel.org
> Signed-off-by: Yuchao Zhang <ndaugoing@gmail.com>

Acked-by: Roger Pau Monné <roger@xenproject.org>

Thanks, Roger.