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

[Bart Van Assche]

On 8/27/26 8:30 AM, Tetsuo Handa wrote:
> 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.

I'm going to drop patch "loop: Serialize I/O and queue limits updates"
from my patch series and leave it to someone else to solve this issue
since none of the proposed fixes that have been discussed so far make me
enthusiast.

Thanks,

Bart.