Skip to content

Commit 0460e09

Browse files
Ming Leigregkh
authored andcommitted
ublk: fix deadlock when reading partition table
[ Upstream commit c258f5c ] When one process(such as udev) opens ublk block device (e.g., to read the partition table via bdev_open()), a deadlock[1] can occur: 1. bdev_open() grabs disk->open_mutex 2. The process issues read I/O to ublk backend to read partition table 3. In __ublk_complete_rq(), blk_update_request() or blk_mq_end_request() runs bio->bi_end_io() callbacks 4. If this triggers fput() on file descriptor of ublk block device, the work may be deferred to current task's task work (see fput() implementation) 5. This eventually calls blkdev_release() from the same context 6. blkdev_release() tries to grab disk->open_mutex again 7. Deadlock: same task waiting for a mutex it already holds The fix is to run blk_update_request() and blk_mq_end_request() with bottom halves disabled. This forces blkdev_release() to run in kernel work-queue context instead of current task work context, and allows ublk server to make forward progress, and avoids the deadlock. Fixes: 71f28f3 ("ublk_drv: add io_uring based userspace block driver") Link: ublk-org/ublksrv#170 [1] Signed-off-by: Ming Lei <ming.lei@redhat.com> Reviewed-by: Caleb Sander Mateos <csander@purestorage.com> [axboe: rewrite comment in ublk] Signed-off-by: Jens Axboe <axboe@kernel.dk> Signed-off-by: Sasha Levin <sashal@kernel.org>
1 parent 474cf21 commit 0460e09

File tree

1 file changed

+28
-4
lines changed

1 file changed

+28
-4
lines changed

drivers/block/ublk_drv.c

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1146,12 +1146,20 @@ static inline struct ublk_uring_cmd_pdu *ublk_get_uring_cmd_pdu(
11461146
return io_uring_cmd_to_pdu(ioucmd, struct ublk_uring_cmd_pdu);
11471147
}
11481148

1149+
static void ublk_end_request(struct request *req, blk_status_t error)
1150+
{
1151+
local_bh_disable();
1152+
blk_mq_end_request(req, error);
1153+
local_bh_enable();
1154+
}
1155+
11491156
/* todo: handle partial completion */
11501157
static inline void __ublk_complete_rq(struct request *req, struct ublk_io *io,
11511158
bool need_map)
11521159
{
11531160
unsigned int unmapped_bytes;
11541161
blk_status_t res = BLK_STS_OK;
1162+
bool requeue;
11551163

11561164
/* failed read IO if nothing is read */
11571165
if (!io->res && req_op(req) == REQ_OP_READ)
@@ -1183,14 +1191,30 @@ static inline void __ublk_complete_rq(struct request *req, struct ublk_io *io,
11831191
if (unlikely(unmapped_bytes < io->res))
11841192
io->res = unmapped_bytes;
11851193

1186-
if (blk_update_request(req, BLK_STS_OK, io->res))
1194+
/*
1195+
* Run bio->bi_end_io() with softirqs disabled. If the final fput
1196+
* happens off this path, then that will prevent ublk's blkdev_release()
1197+
* from being called on current's task work, see fput() implementation.
1198+
*
1199+
* Otherwise, ublk server may not provide forward progress in case of
1200+
* reading the partition table from bdev_open() with disk->open_mutex
1201+
* held, and causes dead lock as we could already be holding
1202+
* disk->open_mutex here.
1203+
*
1204+
* Preferably we would not be doing IO with a mutex held that is also
1205+
* used for release, but this work-around will suffice for now.
1206+
*/
1207+
local_bh_disable();
1208+
requeue = blk_update_request(req, BLK_STS_OK, io->res);
1209+
local_bh_enable();
1210+
if (requeue)
11871211
blk_mq_requeue_request(req, true);
11881212
else if (likely(!blk_should_fake_timeout(req->q)))
11891213
__blk_mq_end_request(req, BLK_STS_OK);
11901214

11911215
return;
11921216
exit:
1193-
blk_mq_end_request(req, res);
1217+
ublk_end_request(req, res);
11941218
}
11951219

11961220
static struct io_uring_cmd *__ublk_prep_compl_io_cmd(struct ublk_io *io,
@@ -1230,7 +1254,7 @@ static inline void __ublk_abort_rq(struct ublk_queue *ubq,
12301254
if (ublk_nosrv_dev_should_queue_io(ubq->dev))
12311255
blk_mq_requeue_request(rq, false);
12321256
else
1233-
blk_mq_end_request(rq, BLK_STS_IOERR);
1257+
ublk_end_request(rq, BLK_STS_IOERR);
12341258
}
12351259

12361260
static void
@@ -1275,7 +1299,7 @@ __ublk_do_auto_buf_reg(const struct ublk_queue *ubq, struct request *req,
12751299
ublk_auto_buf_reg_fallback(ubq, req->tag);
12761300
return AUTO_BUF_REG_FALLBACK;
12771301
}
1278-
blk_mq_end_request(req, BLK_STS_IOERR);
1302+
ublk_end_request(req, BLK_STS_IOERR);
12791303
return AUTO_BUF_REG_FAIL;
12801304
}
12811305

0 commit comments

Comments
 (0)