Re: [PATCH v2] loop: Fix NULL pointer dereference by synchronizing lo_release and loop_queue_rq

[Ming Lei <tom.leiming@gmail.com>]

On Wed, May 20, 2026 at 05:20:01PM +0900, Tetsuo Handa wrote:
> On 2026/05/20 16:49, Ming Lei wrote:
> > On Wed, May 20, 2026 at 03:36:12PM +0900, Tetsuo Handa wrote:
> >> On 2026/05/20 12:06, Ming Lei wrote:
> >>> The IO after close(loop) should be from writeback. rcu/sruc isn't necessary,
> >>
> >> Gemini's comment is that drain_workqueue() is not sufficient for waiting for
> >> do_req_filebacked(REQ_OP_WRITE) requests with cmd->use_aio == true case to complete.
> > 
> > Anything cleared in __loop_clr_fd() is not used by lo_rw_aio_complete() & lo_complete_rq().
> 
> "struct inode *inode = file_inode(iocb->ki_filp);" in kiocb_end_write() from
> lo_rw_aio_do_completion() can dereference "struct file *" with refcount == 0 (UAF)
> because fput() in __loop_clr_fd() can be the last reference to that file.

OK, you are right.

It can be handled by adding sync_blockdev(lo->lo_device) in __loop_clr_fd()
because IO can be from writeback only when loop disk is closed.

Please feel free to test the following change:

diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index 0000913f7efc..bbd15974a082 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -1118,6 +1118,8 @@ static void __loop_clr_fd(struct loop_device *lo)
        struct file *filp;
        gfp_t gfp = lo->old_gfp_mask;

+       sync_blockdev(lo->lo_device);
+
        spin_lock_irq(&lo->lo_lock);
        filp = lo->lo_backing_file;
        lo->lo_backing_file = NULL;

> > 
> > So why isn't drain_workqueue() enough for cmd->use_aio?
> 
> In addition to possible UAF above, the assumption at
> https://elixir.bootlin.com/linux/v7.1-rc4/source/drivers/block/loop.c#L1134
> is currently broken due to this race problem.

That shouldn't be one issue otherwise bio based driver can't work with updating
limits.


Thanks, 
Ming