[v2] block: genhd: don't call probe function with major_names_lock held

syzbot is reporting circular locking problem at blk_request_module() [1],
for blk_request_module() is calling probe function with major_names_lock
held while major_names_lock is held during module's __init and __exit
functions.

                                         loop_exit() {
                                           mutex_lock(&loop_ctl_mutex);
  blk_request_module() {
    mutex_lock(&major_names_lock);
    loop_probe() {
      mutex_lock(&loop_ctl_mutex); // Blocked by loop_exit()
      mutex_unlock(&loop_ctl_mutex);
    }
    mutex_unlock(&major_names_lock);
                                           unregister_blkdev() {
                                             mutex_lock(&major_names_lock); // Blocked by blk_request_module()
                                             mutex_unlock(&major_names_lock);
                                           }
                                           mutex_unlock(&loop_ctl_mutex);
  }
                                         }

Based on an assumption that a probe callback passed to __register_blkdev()
belongs to a module which calls __register_blkdev(), drop major_names_lock
before calling probe function by holding a reference to that module which
contains that probe function. If there is a module where this assumption
does not hold, such module can call ____register_blkdev() directly.

  blk_request_module() {
    mutex_lock(&major_names_lock);
    // Block loop_exit()
    mutex_unlock(&major_names_lock);
    loop_probe() {
      mutex_lock(&loop_ctl_mutex);
      mutex_unlock(&loop_ctl_mutex);
    }
    // Unblock loop_exit()
  }
                                         loop_exit() {
                                           mutex_lock(&loop_ctl_mutex);
                                           unregister_blkdev() {
                                             mutex_lock(&major_names_lock);
                                             mutex_unlock(&major_names_lock);
                                           }
                                           mutex_unlock(&loop_ctl_mutex);
                                         }

Note that regardless of this patch, it is up to probe function to
serialize module's __init function and probe function in that module
by using e.g. a mutex. This patch simply makes sure that module's __exit
function won't be called when probe function is about to be called.

While Desmond Cheong Zhi Xi already proposed a patch for breaking ABCD
circular dependency [2], I consider that this patch is still needed for
safely breaking AB-BA dependency upon module unloading. (Note that syzbot
does not test module unloading code because the syzbot kernels are built
with almost all modules built-in. We need manual inspection.)

By doing kmalloc(GFP_KERNEL) in ____register_blkdev() before holding
major_names_lock, we could convert major_names_lock from a mutex to
a spinlock. But that is beyond the scope of this patch.

Link: https://syzkaller.appspot.com/bug?id=7bd106c28e846d1023d4ca915718b1a0905444cb [1]
Link: https://lkml.kernel.org/r/20210617160904.570111-1-desmondcheongzx@gmail.com [2]
Reported-by: syzbot <syzbot+6a8a0d93c91e8fbf2e80@syzkaller.appspotmail.com>
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Tested-by: syzbot <syzbot+6a8a0d93c91e8fbf2e80@syzkaller.appspotmail.com>
---
 block/genhd.c         | 36 +++++++++++++++++++++++++++---------
 include/linux/genhd.h |  8 +++++---
 2 files changed, 32 insertions(+), 12 deletions(-)

Message ID	f790f8fb-5758-ea4e-a527-0ee4af82dd44@i-love.sakura.ne.jp (mailing list archive)
State	New, archived
Headers	show Return-Path: <linux-block-owner@kernel.org> In-Reply-To: To: hch@infradead.org Cc: axboe@kernel.dk, desmondcheongzx@gmail.com, gregkh@linuxfoundation.org, linux-block@vger.kernel.org, linux-kernel-mentees@lists.linuxfoundation.org, linux-kernel@vger.kernel.org, linux-mtd@lists.infradead.org, miquel.raynal@bootlin.com, richard@nod.at, Shuah Khan <skhan@linuxfoundation.org>, syzbot+6a8a0d93c91e8fbf2e80@syzkaller.appspotmail.com, vigneshr@ti.com From: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp> Subject: [PATCH v2] block: genhd: don't call probe function with major_names_lock held Message-ID: <f790f8fb-5758-ea4e-a527-0ee4af82dd44@i-love.sakura.ne.jp> Date: Sat, 19 Jun 2021 10:05:44 +0900 User-Agent: Mozilla/5.0 (Windows NT 6.3; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Precedence: bulk
Series	[v2] block: genhd: don't call probe function with major_names_lock held \| expand [v2] block: genhd: don't call probe function with major_names_lock held

[v2] block: genhd: don't call probe function with major_names_lock held

Commit Message

Comments

Patch