Sagi, On 2/5/21 19:19, Yi Zhang wrote: > Hello > > We found this kernel NULL pointer issue with latest linux-block/for-next and it's 100% reproduced, let me know if you need more info/testing, thanks > > Kernel repo: https://git.kernel.org/pub/scm/linux/kernel/git/axboe/linux-block.git > Commit: 11f8b6fd0db9 - Merge branch 'for-5.12/io_uring' into for-next > > Reproducer: blktests nvme-tcp/012 > > > [ 124.458121] run blktests nvme/012 at 2021-02-05 21:53:34 > [ 125.525568] BUG: kernel NULL pointer dereference, address: 0000000000000008 > [ 125.532524] #PF: supervisor read access in kernel mode > [ 125.537665] #PF: error_code(0x0000) - not-present page > [ 125.542803] PGD 0 P4D 0 > [ 125.545343] Oops: 0000 [#1] SMP NOPTI > [ 125.549009] CPU: 15 PID: 12069 Comm: kworker/15:2H Tainted: G S I 5.11.0-rc6+ #1 > [ 125.557528] Hardware name: Dell Inc. PowerEdge R640/06NR82, BIOS 2.10.0 11/12/2020 > [ 125.565093] Workqueue: kblockd blk_mq_run_work_fn > [ 125.569797] RIP: 0010:nvme_tcp_init_iter+0x7d/0xd0 [nvme_tcp] > [ 125.575544] Code: 8b 75 68 44 8b 45 28 44 8b 7d 30 49 89 d4 48 c1 e2 04 4c 01 f2 45 89 fb 44 89 c7 85 ff 74 4d 44 89 e0 44 8b 55 10 48 c1 e0 04 <41> 8b 5c 06 08 45 0f b6 ca 89 d8 44 29 d8 39 f8 0f 47 c7 41 83 e9 > [ 125.594290] RSP: 0018:ffffbd084447bd18 EFLAGS: 00010246 > [ 125.599515] RAX: 0000000000000000 RBX: ffffa0bba9f3ce80 RCX: 0000000000000000 > [ 125.606648] RDX: 0000000000000000 RSI: 0000000000000001 RDI: 0000000002000000 > [ 125.613781] RBP: ffffa0ba8ac6fec0 R08: 0000000002000000 R09: 0000000000000000 > [ 125.620914] R10: 0000000002800809 R11: 0000000000000000 R12: 0000000000000000 > [ 125.628045] R13: ffffa0bba9f3cf90 R14: 0000000000000000 R15: 0000000000000000 > [ 125.635178] FS: 0000000000000000(0000) GS:ffffa0c9ff9c0000(0000) knlGS:0000000000000000 > [ 125.643264] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > [ 125.649009] CR2: 0000000000000008 CR3: 00000001c9c6c005 CR4: 00000000007706e0 > [ 125.656142] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 > [ 125.663274] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 > [ 125.670407] PKRU: 55555554 > [ 125.673119] Call Trace: > [ 125.675575] nvme_tcp_queue_rq+0xef/0x330 [nvme_tcp] > [ 125.680537] blk_mq_dispatch_rq_list+0x11c/0x7c0 > [ 125.685157] ? blk_mq_flush_busy_ctxs+0xf6/0x110 > [ 125.689775] __blk_mq_sched_dispatch_requests+0x12b/0x170 > [ 125.695175] blk_mq_sched_dispatch_requests+0x30/0x60 > [ 125.700227] __blk_mq_run_hw_queue+0x2b/0x60 > [ 125.704500] process_one_work+0x1cb/0x360 > [ 125.708513] ? process_one_work+0x360/0x360 > [ 125.712699] worker_thread+0x30/0x370 > [ 125.716365] ? process_one_work+0x360/0x360 > [ 125.720550] kthread+0x116/0x130 > [ 125.723782] ? kthread_park+0x80/0x80 > [ 125.727448] ret_from_fork+0x1f/0x30 The NVMe TCP does support merging for non-admin queues (nvme_tcp_alloc_tagset()). Based on the what is been done for the bvecs in other places in kernel especially when merging is enabled bio split case seems to be missing from tcp when building bvec. What I mean is following completely untested patch :- diff --git a/drivers/nvme/host/tcp.c b/drivers/nvme/host/tcp.c index 619b0d8f6e38..dabb2633b28c 100644 --- a/drivers/nvme/host/tcp.c +++ b/drivers/nvme/host/tcp.c @@ -222,7 +222,9 @@ static void nvme_tcp_init_iter(struct nvme_tcp_request *req, unsigned int dir) { struct request *rq = blk_mq_rq_from_pdu(req); - struct bio_vec *vec; + struct req_iterator rq_iter; + struct bio_vec *vec, *tvec; + struct bio_vec tmp; unsigned int size; int nr_bvec; size_t offset; @@ -233,17 +235,29 @@ static void nvme_tcp_init_iter(struct nvme_tcp_request *req, size = blk_rq_payload_bytes(rq); offset = 0; } else { - struct bio *bio = req->curr_bio; - struct bvec_iter bi; - struct bio_vec bv; - - vec = __bvec_iter_bvec(bio->bi_io_vec, bio->bi_iter); - nr_bvec = 0; - bio_for_each_bvec(bv, bio, bi) { + rq_for_each_bvec(tmp, rq, rq_iter) nr_bvec++; + + if (rq->bio != rq->biotail) { + vec = kmalloc_array(nr_bvec, sizeof(struct bio_vec), + GFP_NOIO); + if (!vec) + return; + + tvec = vec; + rq_for_each_bvec(tmp, rq, rq_iter) { + *vec = tmp; + vec++; + } + vec = tvec; + offset = 0; + } else { + struct bio *bio = req->curr_bio; + + vec = __bvec_iter_bvec(bio->bi_io_vec, bio->bi_iter); + size = bio->bi_iter.bi_size; + offset = bio->bi_iter.bi_bvec_done; } - size = bio->bi_iter.bi_size; - offset = bio->bi_iter.bi_bvec_done; } iov_iter_bvec(&req->iter, dir, vec, nr_bvec, size); @@ -2271,8 +2285,11 @@ static blk_status_t nvme_tcp_setup_cmd_pdu(struct nvme_ns *ns, req->data_len = blk_rq_nr_phys_segments(rq) ? blk_rq_payload_bytes(rq) : 0; req->curr_bio = rq->bio; - if (req->curr_bio) + if (req->curr_bio) { + if (rq->bio != rq->biotail) + printk(KERN_INFO"%s %d req->bio != req->biotail\n", __func__, __LINE__); nvme_tcp_init_iter(req, rq_data_dir(rq)); + } if (rq_data_dir(rq) == WRITE && req->data_len <= nvme_tcp_inline_data_size(queue)) -- 2.22.1 Feel free to ignore this as I might be *completely wrong* here since I don't have a reproducer, everything is working just fine on my machine.
