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

[Hillf Danton <hdanton@sina.com>]

On Fri, 4 Sep 2026 08:50:46 +0900 Tetsuo Handa wrote:
> syzbot is reporting NULL pointer dereference in lo_rw_aio() [1][2].
> An analysis by the Gemini AI collaborator [3] considers that this problem
> is caused by a timing shift primarily exposed by commit 65565ca5f99b
> ("block: unify the synchronous bi_end_io callbacks"), along with helper
> refactorings like commit 92c3737a2473 ("block: add a bio_submit_or_kill
> helper").
> 
> But due to difficulty of reproducing this race, discussion about what is
> happening and how to fix this problem is stalling. Also, we haven't
> identified how many filesystems are subjected to this problem.
> 
> Therefore, introduce a grace period for flushing outstanding I/O
> (which should be a good thing from the perspective of defensive
> programming) so that we won't hit NULL pointer dereference problem.
> 
> However, calling drain_workqueue() from __loop_clr_fd() with
> disk->open_mutex held causes lockdep warnings. We need to flush
> outstanding I/O without disk->open_mutex held. But we can't use task work
> context, for there is no way to reliably wait for completion of a task work
> function inside a loadable module when module unloading code for that
> loadable module has started. We need to use a callback function which is
> embedded into a built-in module so that it can reliably wait for completion
> of synchronous teardown for the loop driver module.
> 
> Therefore, add a dedicated callback for the loop module to the block core
> layer, and invoke that callback immediately after disk->open_mutex is
> released. It is possible that multiple threads invoke that callback
> when a teardown work was scheduled because disk->open_mutex was already
> released, but concurrently calling flush_work() in order to wait for
> completion of an outstanding teardown work will be safe.
> 
> Link: https://syzkaller.appspot.com/bug?extid=cd8a9a308e879a4e2c28 [1]
> Link: https://syzkaller.appspot.com/bug?extid=bc273027d5643e48e5b3 [2]
> Link: https://lkml.kernel.org/r/fbb3edda-f108-4e5b-acf2-266f043f8125@I-love.SAKURA.ne.jp [3]
> Link: https://lkml.kernel.org/r/9f8b5ab0-efbc-4cf3-a1f8-b43377416946@I-love.SAKURA.ne.jp [4]
> Fixes: 65565ca5f99b ("block: unify the synchronous bi_end_io callbacks")
> Assisted-by: Gemini-Pro
> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> ---
> What this patch does is basically the same with the v6 patch. Can this be
> a possible alternative for Bart's
> 
>   Releasing and reacquiring disk->open_mutex from __loop_clr_fd() seems
>   risky to me. There is plenty of code in block/bdev.c that assumes that
>   disk->open_mutex is not released by lo_release().
> 
> comment?
> 
>  block/bdev.c           |  2 ++
>  drivers/block/loop.c   | 65 +++++++++++++++++++++++++++++++++---------
>  include/linux/blkdev.h |  5 ++++
>  3 files changed, 59 insertions(+), 13 deletions(-)
> 
> diff --git a/block/bdev.c b/block/bdev.c
> index cd8323083740..7ce5acaacf43 100644
> --- a/block/bdev.c
> +++ b/block/bdev.c
> @@ -1188,6 +1188,8 @@ void bdev_release(struct file *bdev_file)
>  	else
>  		blkdev_put_whole(bdev);
>  	mutex_unlock(&disk->open_mutex);
> +	if (bdev->bd_disk->fops->post_release)
> +		bdev->bd_disk->fops->post_release(bdev->bd_disk);
>  
A great leap forward, I like this.

> +
> +static void lo_post_release(struct gendisk *disk)
> +{
> +	struct loop_device *lo = disk->private_data;
> +
> +	/* Wait for __loop_clr_fd() to complete. */
> +	flush_work(&lo->lo_clr_work);
>  }