Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   ZiyangZhang <ZiyangZhang@linux.alibaba.com>
To:     ming.lei@redhat.com, axboe@kernel.dk
Cc:     xiaoguang.wang@linux.alibaba.com, linux-block@vger.kernel.org,
        linux-kernel@vger.kernel.org, joseph.qi@linux.alibaba.com,
        ZiyangZhang <ZiyangZhang@linux.alibaba.com>
Subject: [RFC PATCH 5/9] ublk_drv: refactor ublk_stop_dev()
Date:   Wed, 24 Aug 2022 13:47:40 +0800
Message-Id: <20220824054744.77812-6-ZiyangZhang@linux.alibaba.com>
In-Reply-To: <20220824054744.77812-1-ZiyangZhang@linux.alibaba.com>
References: <20220824054744.77812-1-ZiyangZhang@linux.alibaba.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

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@linux.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