Skip to content

Commit de3510e

Browse files
damien-lemoalaxboe
authored andcommitted
null_blk: fix command timeout completion handling
Memory backed or zoned null block devices may generate actual request timeout errors due to the submission path being blocked on memory allocation or zone locking. Unlike fake timeouts or injected timeouts, the request submission path will call blk_mq_complete_request() or blk_mq_end_request() for these real timeout errors, causing a double completion and use after free situation as the block layer timeout handler executes blk_mq_rq_timed_out() and __blk_mq_free_request() in blk_mq_check_expired(). This problem often triggers a NULL pointer dereference such as: BUG: kernel NULL pointer dereference, address: 0000000000000050 RIP: 0010:blk_mq_sched_mark_restart_hctx+0x5/0x20 ... Call Trace: dd_finish_request+0x56/0x80 blk_mq_free_request+0x37/0x130 null_handle_cmd+0xbf/0x250 [null_blk] ? null_queue_rq+0x67/0xd0 [null_blk] blk_mq_dispatch_rq_list+0x122/0x850 __blk_mq_do_dispatch_sched+0xbb/0x2c0 __blk_mq_sched_dispatch_requests+0x13d/0x190 blk_mq_sched_dispatch_requests+0x30/0x60 __blk_mq_run_hw_queue+0x49/0x90 process_one_work+0x26c/0x580 worker_thread+0x55/0x3c0 ? process_one_work+0x580/0x580 kthread+0x134/0x150 ? kthread_create_worker_on_cpu+0x70/0x70 ret_from_fork+0x1f/0x30 This problem very often triggers when running the full btrfs xfstests on a memory-backed zoned null block device in a VM with limited amount of memory. Avoid this by executing blk_mq_complete_request() in null_timeout_rq() only for commands that are marked for a fake timeout completion using the fake_timeout boolean in struct null_cmd. For timeout errors injected through debugfs, the timeout handler will execute blk_mq_complete_request()i as before. This is safe as the submission path does not execute complete requests in this case. In null_timeout_rq(), also make sure to set the command error field to BLK_STS_TIMEOUT and to propagate this error through to the request completion. Reported-by: Johannes Thumshirn <Johannes.Thumshirn@wdc.com> Signed-off-by: Damien Le Moal <damien.lemoal@wdc.com> Tested-by: Johannes Thumshirn <Johannes.Thumshirn@wdc.com> Reviewed-by: Johannes Thumshirn <Johannes.Thumshirn@wdc.com> Link: https://lore.kernel.org/r/20210331225244.126426-1-damien.lemoal@wdc.com Signed-off-by: Jens Axboe <axboe@kernel.dk>
1 parent 3edf534 commit de3510e

2 files changed

Lines changed: 22 additions & 5 deletions

File tree

drivers/block/null_blk/main.c

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1369,10 +1369,13 @@ static blk_status_t null_handle_cmd(struct nullb_cmd *cmd, sector_t sector,
13691369
}
13701370

13711371
if (dev->zoned)
1372-
cmd->error = null_process_zoned_cmd(cmd, op,
1373-
sector, nr_sectors);
1372+
sts = null_process_zoned_cmd(cmd, op, sector, nr_sectors);
13741373
else
1375-
cmd->error = null_process_cmd(cmd, op, sector, nr_sectors);
1374+
sts = null_process_cmd(cmd, op, sector, nr_sectors);
1375+
1376+
/* Do not overwrite errors (e.g. timeout errors) */
1377+
if (cmd->error == BLK_STS_OK)
1378+
cmd->error = sts;
13761379

13771380
out:
13781381
nullb_complete_cmd(cmd);
@@ -1451,8 +1454,20 @@ static bool should_requeue_request(struct request *rq)
14511454

14521455
static enum blk_eh_timer_return null_timeout_rq(struct request *rq, bool res)
14531456
{
1457+
struct nullb_cmd *cmd = blk_mq_rq_to_pdu(rq);
1458+
14541459
pr_info("rq %p timed out\n", rq);
1455-
blk_mq_complete_request(rq);
1460+
1461+
/*
1462+
* If the device is marked as blocking (i.e. memory backed or zoned
1463+
* device), the submission path may be blocked waiting for resources
1464+
* and cause real timeouts. For these real timeouts, the submission
1465+
* path will complete the request using blk_mq_complete_request().
1466+
* Only fake timeouts need to execute blk_mq_complete_request() here.
1467+
*/
1468+
cmd->error = BLK_STS_TIMEOUT;
1469+
if (cmd->fake_timeout)
1470+
blk_mq_complete_request(rq);
14561471
return BLK_EH_DONE;
14571472
}
14581473

@@ -1473,6 +1488,7 @@ static blk_status_t null_queue_rq(struct blk_mq_hw_ctx *hctx,
14731488
cmd->rq = bd->rq;
14741489
cmd->error = BLK_STS_OK;
14751490
cmd->nq = nq;
1491+
cmd->fake_timeout = should_timeout_request(bd->rq);
14761492

14771493
blk_mq_start_request(bd->rq);
14781494

@@ -1489,7 +1505,7 @@ static blk_status_t null_queue_rq(struct blk_mq_hw_ctx *hctx,
14891505
return BLK_STS_OK;
14901506
}
14911507
}
1492-
if (should_timeout_request(bd->rq))
1508+
if (cmd->fake_timeout)
14931509
return BLK_STS_OK;
14941510

14951511
return null_handle_cmd(cmd, sector, nr_sectors, req_op(bd->rq));

drivers/block/null_blk/null_blk.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ struct nullb_cmd {
2222
blk_status_t error;
2323
struct nullb_queue *nq;
2424
struct hrtimer timer;
25+
bool fake_timeout;
2526
};
2627

2728
struct nullb_queue {

0 commit comments

Comments
 (0)