linux-kernel - [RFC PATCH 5/9] ublk_drv: refactor ublk_stop

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>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20220824054744.77812-6-ZiyangZhang@linux.alibaba.com>
Date:   Wed, 24 Aug 2022 13:47:40 +0800
From:   ZiyangZhang <ZiyangZhang@...ux.alibaba.com>
To:     ming.lei@...hat.com, axboe@...nel.dk
Cc:     xiaoguang.wang@...ux.alibaba.com, linux-block@...r.kernel.org,
        linux-kernel@...r.kernel.org, joseph.qi@...ux.alibaba.com,
        ZiyangZhang <ZiyangZhang@...ux.alibaba.com>
Subject: [RFC PATCH 5/9] ublk_drv: refactor ublk_stop_dev()

Since we rely on rq's status to end(abort) it in monitor_work, calling
del_gendisk() too early can result in UAF. So add blk_mq_freeze_queue()
to end all inflight rqs. del_gendisk() can be then safely called with a
frozen queue.

Note: blk_mq_freeze_queue() hangs until:
(1) all rqs are aborted and ioucmds are copmleted(freed) on a dying ubq
    by monitor_work scheduled.
(2) all rqs are ended on other ubqs.

We cancel monitor_work before del_gendisk() because monitor_work also
touches rq's status. Monitor_work is guaranteed to be canceled because
we mark ub's state as UBLK_S_DEV_DEAD.

Signed-off-by: ZiyangZhang <ZiyangZhang@...ux.alibaba.com>
---
 drivers/block/ublk_drv.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
index 1b1c0190bba4..df8751ea3711 100644
--- a/drivers/block/ublk_drv.c
+++ b/drivers/block/ublk_drv.c
@@ -1018,18 +1018,23 @@ static void ublk_cancel_dev(struct ublk_device *ub)
 static void ublk_stop_dev(struct ublk_device *ub)
 {
 	mutex_lock(&ub->mutex);
-	if (ub->dev_info.state != UBLK_S_DEV_LIVE)
-		goto unlock;
+	/* No gendisk is live. ubq may be ready or not */
+	if (ub->dev_info.state == UBLK_S_DEV_DEAD)
+		goto out_cancel_dev;
 
-	del_gendisk(ub->ub_disk);
+	mod_delayed_work(system_wq, &ub->monitor_work, 0);
+	pr_devel("%s: Wait for all requests ended...\n", __func__);
+	blk_mq_freeze_queue(ub->ub_disk->queue);
 	ub->dev_info.state = UBLK_S_DEV_DEAD;
+	cancel_delayed_work_sync(&ub->monitor_work);
+	pr_devel("%s: All requests are ended.\n", __func__);
+	del_gendisk(ub->ub_disk);
 	ub->dev_info.ublksrv_pid = -1;
 	put_disk(ub->ub_disk);
 	ub->ub_disk = NULL;
- unlock:
+ out_cancel_dev:
 	ublk_cancel_dev(ub);
 	mutex_unlock(&ub->mutex);
-	cancel_delayed_work_sync(&ub->monitor_work);
 }
 
 /* device can only be started after all IOs are ready */
-- 
2.27.0