Re: [PATCH] block/loop: Fix NULL pointer dereference in lo_rw_aio()

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

On Tue, May 19, 2026 at 02:06:26PM +0800, Hongling Zeng wrote:
> Hi,
> 
> Thank you for the review.
> 
> 在 2026年05月19日 12:42, Ming Lei 写道:
> > On Tue, May 19, 2026 at 12:01:03PM +0800, Hongling Zeng wrote:
> > > lo->lo_backing_file can be NULL when the loop device is being cleared,
> > > causing NULL pointer dereference in lo_rw_aio(). Add a defensive check
> > > to prevent kernel crash.
> > > 
> > > In v7.1-rc tree, lo->lo_backing_file is cleared in __loop_clr_fd(), which
> > > is called from lo_release() only, when it is guaranteed that there isn't
> > > inflight IO.
> > > 
> > > So care to share your stack trace?
>   Syzkaller has managed to trigger NULL pointer dereference in lo_rw_aio().
>   Tetsuo Handa encountered the same issue (commit 1a83d226268cf), but his
>   patch only adds diagnostic logging and is marked "not for upstream".
> > > Also fix loop_attr_backing_file_show() to use PTR_ERR_OR_ZERO()
> > > for correct NULL pointer handling.
> > > 
> > > Fixes: bc07c10a3603a ("block: loop: support DIO & AIO")
> > Can you explain how the issue is introduced in above commit?
> The issue was introduced when lo_rw_aio() was added in that commit,
> which dereferences lo->lo_backing_file without NULL checks. While the
> design assumes no inflight IO during __loop_clr_fd(), fuzzers have
> found edge cases where this assumption doesn't hold.

What are the edge cases?

> 
> > > Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
> > > ---
> > >   drivers/block/loop.c | 5 ++++-
> > >   1 file changed, 4 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/block/loop.c b/drivers/block/loop.c
> > > index 0000913f7efc..d8db1b0d8018 100644
> > > --- a/drivers/block/loop.c
> > > +++ b/drivers/block/loop.c
> > > @@ -397,6 +397,9 @@ static int lo_rw_aio(struct loop_device *lo, struct loop_cmd *cmd,
> > >   		cmd->iocb.ki_flags = 0;
> > >   	}
> > > +	if (!file)
> > > +		return -EIO;
> > If lo->lo_backing_file may become NULL, it isn't enough to add check here
> > only, cause it can be observed in lo_req_flush() & lo_fallocate() too.
> > 
> > 
> > Thanks,
> > Ming
> You're absolutely right. Tetsuo's approach only adds logging to help
> diagnose the issue (and he marked it "not for upstream"), but it doesn't
> prevent the crash.
> 
> To properly fix this, I need to add NULL checks to all functions that
> dereference lo->lo_backing_file:
>   - lo_rw_aio()
>   - lo_req_flush()
>   - lo_fallocate()
>   - loop_update_limits()
>   - lo_can_use_dio()
> 
> What's your preferred approach?
> 1. Simple NULL checks in all functions
> 2. A helper function with proper locking
> 3. Your recommendation
> 
> Let me know which approach you think is best, and I'll update the patch
> accordingly.

It isn't supposed to happen, so please root-cause first before sending
any workaround.

Thanks,
Ming