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

[Roger Pau Monné <roger@xxxxxxxxxxxxxx>]

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@xxxxxxxxxxxxxxx
> Signed-off-by: Yuchao Zhang <ndaugoing@xxxxxxxxx>
> ---
>  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.