From b600de2d7d3a16f9007fad1bdae82a3951a26af2 Mon Sep 17 00:00:00 2001 From: Yu Kuai Date: Mon, 30 Jan 2023 09:41:36 +0800 Subject: [PATCH 1/8] block, bfq: fix uaf for bfqq in bic_set_bfqq() After commit 64dc8c732f5c ("block, bfq: fix possible uaf for 'bfqq->bic'"), bic->bfqq will be accessed in bic_set_bfqq(), however, in some context bic->bfqq will be freed, and bic_set_bfqq() is called with the freed bic->bfqq. Fix the problem by always freeing bfqq after bic_set_bfqq(). Fixes: 64dc8c732f5c ("block, bfq: fix possible uaf for 'bfqq->bic'") Reported-and-tested-by: Shinichiro Kawasaki Signed-off-by: Yu Kuai Reviewed-by: Jan Kara Link: https://lore.kernel.org/r/20230130014136.591038-1-yukuai1@huaweicloud.com Signed-off-by: Jens Axboe --- block/bfq-cgroup.c | 2 +- block/bfq-iosched.c | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/block/bfq-cgroup.c b/block/bfq-cgroup.c index 7d9b15f0dbd5..0fbde0fc0628 100644 --- a/block/bfq-cgroup.c +++ b/block/bfq-cgroup.c @@ -769,8 +769,8 @@ static void __bfq_bic_change_cgroup(struct bfq_data *bfqd, * request from the old cgroup. */ bfq_put_cooperator(sync_bfqq); - bfq_release_process_ref(bfqd, sync_bfqq); bic_set_bfqq(bic, NULL, true); + bfq_release_process_ref(bfqd, sync_bfqq); } } } diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c index ccf2204477a5..380e9bda2e57 100644 --- a/block/bfq-iosched.c +++ b/block/bfq-iosched.c @@ -5425,9 +5425,11 @@ static void bfq_check_ioprio_change(struct bfq_io_cq *bic, struct bio *bio) bfqq = bic_to_bfqq(bic, false); if (bfqq) { - bfq_release_process_ref(bfqd, bfqq); + struct bfq_queue *old_bfqq = bfqq; + bfqq = bfq_get_queue(bfqd, bio, false, bic, true); bic_set_bfqq(bic, bfqq, false); + bfq_release_process_ref(bfqd, old_bfqq); } bfqq = bic_to_bfqq(bic, true); From 29baef789c838bd5c02f50c88adbbc6b955aaf61 Mon Sep 17 00:00:00 2001 From: Liu Xiaodong Date: Tue, 31 Jan 2023 02:05:52 -0500 Subject: [PATCH 2/8] block: ublk: extending queue_size to fix overflow When validating drafted SPDK ublk target, in a case that assigning large queue depth to multiqueue ublk device, ublk target would run into a weird incorrect state. During rounds of review and debug, An overflow bug was found in ublk driver. In ublk_cmd.h, UBLK_MAX_QUEUE_DEPTH is 4096 which means each ublk queue depth can be set as large as 4096. But when setting qd for a ublk device, sizeof(struct ublk_queue) + depth * sizeof(struct ublk_io) will be larger than 65535 if qd is larger than 2728. Then queue_size is overflowed, and ublk_get_queue() references a wrong pointer position. The wrong content of ublk_queue elements will lead to out-of-bounds memory access. Extend queue_size in ublk_device as "unsigned int". Signed-off-by: Liu Xiaodong Fixes: 71f28f3136af ("ublk_drv: add io_uring based userspace block driver") Reviewed-by: Ming Lei Link: https://lore.kernel.org/r/20230131070552.115067-1-xiaodong.liu@intel.com Signed-off-by: Jens Axboe --- drivers/block/ublk_drv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c index e54693204630..6368b56eacf1 100644 --- a/drivers/block/ublk_drv.c +++ b/drivers/block/ublk_drv.c @@ -137,7 +137,7 @@ struct ublk_device { char *__queues; - unsigned short queue_size; + unsigned int queue_size; struct ublksrv_ctrl_dev_info dev_info; struct blk_mq_tag_set tag_set; From 81ea42b9c3d61ea34d82d900ed93f4b4851f13b0 Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Mon, 30 Jan 2023 13:12:33 -0800 Subject: [PATCH 3/8] block: Fix the blk_mq_destroy_queue() documentation Commit 2b3f056f72e5 moved a blk_put_queue() call from blk_mq_destroy_queue() into its callers. Reflect this change in the documentation block above blk_mq_destroy_queue(). Cc: Christoph Hellwig Cc: Sagi Grimberg Cc: Chaitanya Kulkarni Cc: Keith Busch Fixes: 2b3f056f72e5 ("blk-mq: move the call to blk_put_queue out of blk_mq_destroy_queue") Signed-off-by: Bart Van Assche Reviewed-by: Christoph Hellwig Link: https://lore.kernel.org/r/20230130211233.831613-1-bvanassche@acm.org Signed-off-by: Jens Axboe --- block/blk-mq.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/block/blk-mq.c b/block/blk-mq.c index 9d463f7563bc..9c8dc70020bc 100644 --- a/block/blk-mq.c +++ b/block/blk-mq.c @@ -4069,8 +4069,9 @@ EXPORT_SYMBOL(blk_mq_init_queue); * blk_mq_destroy_queue - shutdown a request queue * @q: request queue to shutdown * - * This shuts down a request queue allocated by blk_mq_init_queue() and drops - * the initial reference. All future requests will failed with -ENODEV. + * This shuts down a request queue allocated by blk_mq_init_queue(). All future + * requests will be failed with -ENODEV. The caller is responsible for dropping + * the reference from blk_mq_init_queue() by calling blk_put_queue(). * * Context: can sleep */ From 0cab4404874f2de52617de8400c844891c6ea1ce Mon Sep 17 00:00:00 2001 From: Amit Engel Date: Mon, 23 Jan 2023 14:37:28 +0200 Subject: [PATCH 4/8] nvme-fc: fix a missing queue put in nvmet_fc_ls_create_association As part of nvmet_fc_ls_create_association there is a case where nvmet_fc_alloc_target_queue fails right after a new association with an admin queue is created. In this case, no one releases the get taken in nvmet_fc_alloc_target_assoc. This fix is adding the missing put. Signed-off-by: Amit Engel Reviewed-by: James Smart Signed-off-by: Christoph Hellwig --- drivers/nvme/target/fc.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/nvme/target/fc.c b/drivers/nvme/target/fc.c index ab2627e17bb9..1ab6601fdd5c 100644 --- a/drivers/nvme/target/fc.c +++ b/drivers/nvme/target/fc.c @@ -1685,8 +1685,10 @@ nvmet_fc_ls_create_association(struct nvmet_fc_tgtport *tgtport, else { queue = nvmet_fc_alloc_target_queue(iod->assoc, 0, be16_to_cpu(rqst->assoc_cmd.sqsize)); - if (!queue) + if (!queue) { ret = VERR_QUEUE_ALLOC_FAIL; + nvmet_fc_tgt_a_put(iod->assoc); + } } } From fd62678ab55cb01e11a404d302cdade222bf4022 Mon Sep 17 00:00:00 2001 From: Maurizio Lombardi Date: Fri, 27 Jan 2023 16:42:37 +0100 Subject: [PATCH 5/8] nvme: clear the request_queue pointers on failure in nvme_alloc_admin_tag_set If nvme_alloc_admin_tag_set() fails, the admin_q and fabrics_q pointers are left with an invalid, non-NULL value. Other functions may then check the pointers and dereference them, e.g. in nvme_probe() -> out_disable: -> nvme_dev_remove_admin(). Fix the bug by setting admin_q and fabrics_q to NULL in case of error. Also use the set variable to free the tag_set as ctrl->admin_tagset isn't initialized yet. Signed-off-by: Maurizio Lombardi Reviewed-by: Keith Busch Reviewed-by: Chaitanya Kulkarni Signed-off-by: Christoph Hellwig --- drivers/nvme/host/core.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c index 2990ed331a1a..1c333ce64bd4 100644 --- a/drivers/nvme/host/core.c +++ b/drivers/nvme/host/core.c @@ -4892,7 +4892,9 @@ out_cleanup_admin_q: blk_mq_destroy_queue(ctrl->admin_q); blk_put_queue(ctrl->admin_q); out_free_tagset: - blk_mq_free_tag_set(ctrl->admin_tagset); + blk_mq_free_tag_set(set); + ctrl->admin_q = NULL; + ctrl->fabrics_q = NULL; return ret; } EXPORT_SYMBOL_GPL(nvme_alloc_admin_tag_set); From 6fbf13c0e24fd86ab2e4477cd8484a485b687421 Mon Sep 17 00:00:00 2001 From: Maurizio Lombardi Date: Tue, 31 Jan 2023 17:38:42 +0100 Subject: [PATCH 6/8] nvme: clear the request_queue pointers on failure in nvme_alloc_io_tag_set In nvme_alloc_io_tag_set(), the connect_q pointer should be set to NULL in case of error to avoid potential invalid pointer dereferences. Signed-off-by: Maurizio Lombardi Reviewed-by: Chaitanya Kulkarni Signed-off-by: Christoph Hellwig --- drivers/nvme/host/core.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c index 1c333ce64bd4..c777f56a9e74 100644 --- a/drivers/nvme/host/core.c +++ b/drivers/nvme/host/core.c @@ -4956,6 +4956,7 @@ int nvme_alloc_io_tag_set(struct nvme_ctrl *ctrl, struct blk_mq_tag_set *set, out_free_tag_set: blk_mq_free_tag_set(set); + ctrl->connect_q = NULL; return ret; } EXPORT_SYMBOL_GPL(nvme_alloc_io_tag_set); From bd97a59da6a866e3dee5d2a2d582ec71dbbc84cd Mon Sep 17 00:00:00 2001 From: Shin'ichiro Kawasaki Date: Tue, 31 Jan 2023 18:26:44 +0900 Subject: [PATCH 7/8] nvme-auth: use workqueue dedicated to authentication NVMe In-Band authentication uses two kinds of works: chap->auth_work and ctrl->dhchap_auth_work. The latter work flushes or cancels the former work. However, the both works are queued to the same workqueue nvme-wq. It results in the lockdep WARNING as follows: WARNING: possible recursive locking detected 6.2.0-rc4+ #1 Not tainted -------------------------------------------- kworker/u16:7/69 is trying to acquire lock: ffff902d52e65548 ((wq_completion)nvme-wq){+.+.}-{0:0}, at: start_flush_work+0x2c5/0x380 but task is already holding lock: ffff902d52e65548 ((wq_completion)nvme-wq){+.+.}-{0:0}, at: process_one_work+0x210/0x410 To avoid the WARNING, introduce a new workqueue nvme-auth-wq dedicated to chap->auth_work. Reported-by: Daniel Wagner Link: https://lore.kernel.org/linux-nvme/20230130110802.paafkiipmitwtnwr@carbon.lan/ Fixes: f50fff73d620 ("nvme: implement In-Band authentication") Signed-off-by: Shin'ichiro Kawasaki Tested-by: Daniel Wagner Reviewed-by: Hannes Reinecke Signed-off-by: Christoph Hellwig --- drivers/nvme/host/auth.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/drivers/nvme/host/auth.c b/drivers/nvme/host/auth.c index 4424f53a8a0a..b57630d1d3b8 100644 --- a/drivers/nvme/host/auth.c +++ b/drivers/nvme/host/auth.c @@ -45,6 +45,8 @@ struct nvme_dhchap_queue_context { int sess_key_len; }; +struct workqueue_struct *nvme_auth_wq; + #define nvme_auth_flags_from_qid(qid) \ (qid == 0) ? 0 : BLK_MQ_REQ_NOWAIT | BLK_MQ_REQ_RESERVED #define nvme_auth_queue_from_qid(ctrl, qid) \ @@ -866,7 +868,7 @@ int nvme_auth_negotiate(struct nvme_ctrl *ctrl, int qid) chap = &ctrl->dhchap_ctxs[qid]; cancel_work_sync(&chap->auth_work); - queue_work(nvme_wq, &chap->auth_work); + queue_work(nvme_auth_wq, &chap->auth_work); return 0; } EXPORT_SYMBOL_GPL(nvme_auth_negotiate); @@ -1008,10 +1010,15 @@ EXPORT_SYMBOL_GPL(nvme_auth_free); int __init nvme_init_auth(void) { + nvme_auth_wq = alloc_workqueue("nvme-auth-wq", + WQ_UNBOUND | WQ_MEM_RECLAIM | WQ_SYSFS, 0); + if (!nvme_auth_wq) + return -ENOMEM; + nvme_chap_buf_cache = kmem_cache_create("nvme-chap-buf-cache", CHAP_BUF_SIZE, 0, SLAB_HWCACHE_ALIGN, NULL); if (!nvme_chap_buf_cache) - return -ENOMEM; + goto err_destroy_workqueue; nvme_chap_buf_pool = mempool_create(16, mempool_alloc_slab, mempool_free_slab, nvme_chap_buf_cache); @@ -1021,6 +1028,8 @@ int __init nvme_init_auth(void) return 0; err_destroy_chap_buf_cache: kmem_cache_destroy(nvme_chap_buf_cache); +err_destroy_workqueue: + destroy_workqueue(nvme_auth_wq); return -ENOMEM; } @@ -1028,4 +1037,5 @@ void __exit nvme_exit_auth(void) { mempool_destroy(nvme_chap_buf_pool); kmem_cache_destroy(nvme_chap_buf_cache); + destroy_workqueue(nvme_auth_wq); } From 0416f3be58c6b1ea066cd52e354b857693feb01e Mon Sep 17 00:00:00 2001 From: Ming Lei Date: Thu, 2 Feb 2023 10:18:04 +0800 Subject: [PATCH 8/8] blk-cgroup: don't update io stat for root cgroup We source root cgroup stats from the system-wide stats, see blkcg_print_stat and blkcg_rstat_flush, so don't update io state for root cgroup. Fixes blkg leak issue introduced in commit 3b8cc6298724 ("blk-cgroup: Optimize blkcg_rstat_flush()") which starts to grab blkg's reference when adding iostat_cpu into percpu blkcg list, but this state won't be consumed by blkcg_rstat_flush() where the blkg reference is dropped. Tested-by: Bart van Assche Reported-by: Bart van Assche Fixes: 3b8cc6298724 ("blk-cgroup: Optimize blkcg_rstat_flush()") Cc: Tejun Heo Cc: Waiman Long Signed-off-by: Ming Lei Link: https://lore.kernel.org/r/20230202021804.278582-1-ming.lei@redhat.com Signed-off-by: Jens Axboe --- block/blk-cgroup.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c index 4c94a6560f62..9ac1efb053e0 100644 --- a/block/blk-cgroup.c +++ b/block/blk-cgroup.c @@ -2001,6 +2001,10 @@ void blk_cgroup_bio_start(struct bio *bio) struct blkg_iostat_set *bis; unsigned long flags; + /* Root-level stats are sourced from system-wide IO stats */ + if (!cgroup_parent(blkcg->css.cgroup)) + return; + cpu = get_cpu(); bis = per_cpu_ptr(bio->bi_blkg->iostat_cpu, cpu); flags = u64_stats_update_begin_irqsave(&bis->sync);