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

[Tetsuo Handa]

On 2026/08/26 8:16, Bart Van Assche wrote:
> On 8/25/26 8:13 AM, Tetsuo Handa wrote:
>> On 2026/08/25 7:53, Bart Van Assche wrote:
>>> A correction: it is not safe to call drain_workqueue() nor to freeze the
>>> request queue in __loop_clr_fd(). I'm considering to modify the comment
>>> in that function as follows:
>>
>> you also recognized that we can't call drain_workqueue() or flush_workqueue().
>>
>> Therefore, I chose to temporarily drop lo->lo_disk->open_mutex in order to
>> make it possible to safely perform drain_workqueue().
> 
> My sentence was incomplete: I should have written that it is not safe to
> call drain_workqueue() nor flush_workqueue() from __loop_clr_fd() while
> the request queue is frozen. It is not clear to me how draining or
> flushing the workqueue from inside __loop_clr_fd() could cause trouble
> if this happens with the queue unfrozen since disk->open_mutex is not
> acquired by the memory reclaim code?

I don't know how "since disk->open_mutex is not acquired by the memory reclaim
code" is relevant...

Current situation is a result of what we had considered 4 years ago; we don't need
to destroy workqueue (note that destroy_workqueue() implies drain_workqueue()) from
__loop_clr_fd() ( https://lkml.kernel.org/r/20220330052917.2566582-16-hch@xxxxxx ).

Since there is a

  Chain exists of:
    (wq_completion)loop0 --> system_transition_mutex/1 --> &disk->open_mutex
  
   Possible unsafe locking scenario:
  
         CPU0                    CPU1
         ----                    ----
    lock(&disk->open_mutex);
                                 lock(system_transition_mutex/1);
                                 lock(&disk->open_mutex);
    lock((wq_completion)loop0);

dependency, but my proposal to forbid binding loop device to pseudo files
( https://lkml.kernel.org/r/148efba2-a0b6-47d7-ac76-b19d2f4b696c@xxxxxxxxxxxxxxxxxxx )
was rejected by Christoph, we are stuck in a

  Draining workqueue with open_mutex held causes creating a complex lock dependency
  chain involving the global system_transition_mutex. (Maybe there are other paths
  that create similar dependency chain if we drain workqueue with open_mutex held.)

versus

  Not draining workqueue causes NULL pointer dereference in lo_rw_aio().

collision. Therefore,

  Draining workqueue *without open_mutex held* can avoid creating a complex lock
  dependency chain involving the global system_transition_mutex and can also avoid
  NULL pointer dereference in lo_rw_aio().

is my solution.

Can you agree with my solution?