linux-kernel - [PATCH -next] null_blk: fix null-ptr-dereference while configuring 'power' and 'submit

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: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Date:   Sat, 10 Jun 2023 11:01:43 +0800
From:   Yu Kuai <yukuai1@...weicloud.com>
To:     axboe@...nel.dk, bvanassche@....org, kch@...dia.com,
        damien.lemoal@...nsource.wdc.com, hare@...e.de,
        vincent.fu@...sung.com, akinobu.mita@...il.com
Cc:     linux-block@...r.kernel.org, linux-kernel@...r.kernel.org,
        yukuai3@...wei.com, yukuai1@...weicloud.com, yi.zhang@...wei.com,
        yangerkun@...wei.com
Subject: [PATCH -next] null_blk: fix null-ptr-dereference while configuring 'power' and 'submit_queues'

From: Yu Kuai <yukuai3@...wei.com>

Writing 'power' and 'submit_queues' concurrently will trigger kernel
panic:

Test script:

modprobe null_blk nr_devices=0
mkdir -p /sys/kernel/config/nullb/nullb0
while true; do echo 1 > submit_queues; echo 4 > submit_queues; done &
while true; do echo 1 > power; echo 0 > power; done

Test result:

BUG: kernel NULL pointer dereference, address: 0000000000000148
Oops: 0000 [#1] PREEMPT SMP
RIP: 0010:__lock_acquire+0x41d/0x28f0
Call Trace:
 <TASK>
 lock_acquire+0x121/0x450
 down_write+0x5f/0x1d0
 simple_recursive_removal+0x12f/0x5c0
 blk_mq_debugfs_unregister_hctxs+0x7c/0x100
 blk_mq_update_nr_hw_queues+0x4a3/0x720
 nullb_update_nr_hw_queues+0x71/0xf0 [null_blk]
 nullb_device_submit_queues_store+0x79/0xf0 [null_blk]
 configfs_write_iter+0x119/0x1e0
 vfs_write+0x326/0x730
 ksys_write+0x74/0x150

This is because del_gendisk() can concurrent with
blk_mq_update_nr_hw_queues():

nullb_device_power_store	nullb_apply_submit_queues
 null_del_dev
 del_gendisk
				 nullb_update_nr_hw_queues
				  if (!dev->nullb)
				  // still set while gendisk is deleted
				   return 0
				  blk_mq_update_nr_hw_queues
 dev->nullb = NULL

Fix this problem by synchronize nullb_device_power_store() and
nullb_update_nr_hw_queues() with a mutex.

Fixes: 45919fbfe1c4 ("null_blk: Enable modifying 'submit_queues' after an instance has been configured")
Signed-off-by: Yu Kuai <yukuai3@...wei.com>
---
 drivers/block/null_blk/main.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
index b3fedafe301e..21c668998d95 100644
--- a/drivers/block/null_blk/main.c
+++ b/drivers/block/null_blk/main.c
@@ -1810,6 +1810,7 @@ static void null_del_dev(struct nullb *nullb)
 		return;
 
 	dev = nullb->dev;
+	dev->nullb = NULL;
 
 	ida_simple_remove(&nullb_indexes, nullb->index);
 
@@ -1831,7 +1832,6 @@ static void null_del_dev(struct nullb *nullb)
 	if (null_cache_active(nullb))
 		null_free_device_storage(nullb->dev, true);
 	kfree(nullb);
-	dev->nullb = NULL;
 }
 
 static void null_config_discard(struct nullb *nullb)
@@ -2087,7 +2087,6 @@ static int null_add_dev(struct nullb_device *dev)
 		goto out;
 	}
 	nullb->dev = dev;
-	dev->nullb = nullb;
 
 	spin_lock_init(&nullb->lock);
 
@@ -2179,6 +2178,7 @@ static int null_add_dev(struct nullb_device *dev)
 	if (rv)
 		goto out_ida_free;
 
+	dev->nullb = nullb;
 	mutex_lock(&lock);
 	list_add_tail(&nullb->list, &nullb_list);
 	mutex_unlock(&lock);
@@ -2200,7 +2200,6 @@ static int null_add_dev(struct nullb_device *dev)
 	cleanup_queues(nullb);
 out_free_nullb:
 	kfree(nullb);
-	dev->nullb = NULL;
 out:
 	return rv;
 }
-- 
2.39.2