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

[Tetsuo Handa]

On 2026/08/27 2:44, Bart Van Assche wrote:
> 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().

sashiko's review did not find problems
( https://sashiko.dev/#/patchset/8dedfc40-9cae-44ff-9960-e0eb1825e963%40I-love.SAKURA.ne.jp ).
This callback is the last code which is performed immediately before releasing disk->open_mutex.

> 
> I think there is another solution: instead of draining the workqueue
> from inside __loop_clr_fd(), postpone it until the next time the loop
> device is bound. See also the patch below.

That is too late to avoid NULL pointer dereference problem.
We need to flush before setting lo->lo_backing_file to NULL.

> 
> Regarding your earlier request for a Sashiko review: I will look into
> configuring Sashiko such that I can run "sashiko review ${commit_id}"
> locally. The only part I'm missing right now is a Sashiko API key.
While Sashiko considers a static blk_mq_freeze_queue() sufficient to avoid
the NULL pointer dereference, relying on freezing the queue inside __loop_clr_fd()
while holding `disk->open_mutex` is a dangerous decision.

It is true that blk_mq_freeze_queue() alone is equivalent to
synchronize_rcu() + drain_workqueue() + blk_mq_freeze_queue().

Although omitting drain_workqueue() causes lockdep to stop complaining, not
using drain_workqueue() does not avoid the runtime deadlock situation shown below.
As long as there is a possibility of an in-flight I/O holding or waiting on a
lock that subsequently tries to acquire `disk->open_mutex`, the deadlock
risk remains. Lockdep will remain silent here simply because it cannot inspect
the wake-up conditions inside wait_event() during a queue freeze.

Here is the exact execution timeline that leads to the silent deadlock:

Thread 1:                         Thread 2:                         Thread 3:
========================================================================================
                                                                    Holds a global lock 
                                                                    (e.g., system_transition_mutex).

                                  Block core increments 
                                  `q_usage_counter`.

Block core holds `disk->open_mutex`.

lo_release() tries to wait for 
`q_usage_counter` to reach 0 
via blk_mq_freeze_queue() 
  => BLOCKED by Thread 2.
                                  loop_handle_cmd() tries 
                                  to hold the global lock 
                                    => BLOCKED by Thread 3.
                                                                    Tries to hold 
                                                                    `disk->open_mutex` 
                                                                      => BLOCKED by Thread 1.
========================================================================================

Since Thread 1 will never release `disk->open_mutex`, Thread 3 can never release
the global lock, and Thread 2 can never reach blk_mq_end_request() to unblock Thread 1.

I've tried offloading __loop_clr_fd() entirely to task_work context in v3 patch, but
it did not work due to module lifecycle restrictions
( https://sashiko.dev/#/patchset/fda8abc8-6aa2-463b-bf72-865f6b838034@xxxxxxxxxxxxxxxxxxx ).

Therefore, I consider that temporarily releasing `disk->open_mutex` within the process
context to safely perform flushing/draining before clearing the backing file pointer
is the most robust architectural solution to break this deadlock chain.