lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-Id: <20250803134114.2707-1-ujwal.kundur@gmail.com>
Date: Sun,  3 Aug 2025 19:11:14 +0530
From: Ujwal Kundur <ujwal.kundur@...il.com>
To: axboe@...nel.dk
Cc: ming.lei@...hat.com,
	linux-block@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	Ujwal Kundur <ujwal.kundur@...il.com>,
	syzbot+2e9e529ac0b319316453@...kaller.appspotmail.com
Subject: [PATCH] block: prevent deadlock in del_gendisk()

A potential unsafe locking scenario presents itself when
mutex_lock(&disk->open_mutex) is called with reader's lock held on
update_nr_hwq_lock:
       CPU0                    CPU1
       ----                    ----
rlock(&set->update_nr_hwq_lock)
                               lock(&nbd->config_lock);
                               lock(&set->update_nr_hwq_lock);
lock(&disk->open_mutex)

When the gendisk is added back concurrently, a writer's lock is
attempted to be held on update_nr_hwq_lock while holding other locks in
the call-path, becoming a potential source of deadlock(s).

Scope read-critical section to blk_unregister_queue, which is the only
function that interacts with switching elevator and requires
synchronization with update_nr_hwq_lock.

Reported-by: syzbot+2e9e529ac0b319316453@...kaller.appspotmail.com
Signed-off-by: Ujwal Kundur <ujwal.kundur@...il.com>
---
 block/genhd.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)
 mode change 100644 => 100755 block/genhd.c

diff --git a/block/genhd.c b/block/genhd.c
old mode 100644
new mode 100755
index c26733f6324b..b56f09f5699b
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -696,6 +696,7 @@ static void __del_gendisk(struct gendisk *disk)
 	struct block_device *part;
 	unsigned long idx;
 	bool start_drain;
+	struct blk_mq_tag_set *set = q->tag_set;
 
 	might_sleep();
 
@@ -740,7 +741,9 @@ static void __del_gendisk(struct gendisk *disk)
 		bdi_unregister(disk->bdi);
 	}
 
+	down_read(&set->update_nr_hwq_lock);
 	blk_unregister_queue(disk);
+	up_read(&set->update_nr_hwq_lock);
 
 	kobject_put(disk->part0->bd_holder_dir);
 	kobject_put(disk->slave_dir);
@@ -808,20 +811,15 @@ static void disable_elv_switch(struct request_queue *q)
  */
 void del_gendisk(struct gendisk *disk)
 {
-	struct blk_mq_tag_set *set;
 	unsigned int memflags;
 
 	if (!queue_is_mq(disk->queue)) {
 		__del_gendisk(disk);
 	} else {
-		set = disk->queue->tag_set;
-
 		disable_elv_switch(disk->queue);
 
 		memflags = memalloc_noio_save();
-		down_read(&set->update_nr_hwq_lock);
 		__del_gendisk(disk);
-		up_read(&set->update_nr_hwq_lock);
 		memalloc_noio_restore(memflags);
 	}
 }
-- 
2.30.2


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ