Skip to content

Commit 4082326

Browse files
Hannes Reineckekeithbusch
Hannes Reinecke
authored andcommitted
nvmet: Fix crash when a namespace is disabled
The namespace percpu counter protects pending I/O, and we can only safely diable the namespace once the counter drop to zero. Otherwise we end up with a crash when running blktests/nvme/058 (eg for loop transport): [ 2352.930426] [ T53909] Oops: general protection fault, probably for non-canonical address 0xdffffc0000000005: 0000 [#1] PREEMPT SMP KASAN PTI [ 2352.930431] [ T53909] KASAN: null-ptr-deref in range [0x0000000000000028-0x000000000000002f] [ 2352.930434] [ T53909] CPU: 3 UID: 0 PID: 53909 Comm: kworker/u16:5 Tainted: G W 6.13.0-rc6 torvalds#232 [ 2352.930438] [ T53909] Tainted: [W]=WARN [ 2352.930440] [ T53909] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.3-3.fc41 04/01/2014 [ 2352.930443] [ T53909] Workqueue: nvmet-wq nvme_loop_execute_work [nvme_loop] [ 2352.930449] [ T53909] RIP: 0010:blkcg_set_ioprio+0x44/0x180 as the queue is already torn down when calling submit_bio(); So we need to init the percpu counter in nvmet_ns_enable(), and wait for it to drop to zero in nvmet_ns_disable() to avoid having I/O pending after the namespace has been disabled. Fixes: 74d1696 ("nvmet-loop: avoid using mutex in IO hotpath") Signed-off-by: Hannes Reinecke <hare@kernel.org> Reviewed-by: Nilay Shroff <nilay@linux.ibm.com> Reviewed-by: Sagi Grimberg <sagi@grimberg.me> Reviewed-by: Christoph Hellwig <hch@lst.de> Reviewed-by: Chaitanya Kulkarni <kch@nvidia.com> Tested-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@wdc.com> Signed-off-by: Keith Busch <kbusch@kernel.org>
1 parent 84e0090 commit 4082326

File tree

1 file changed

+19
-21
lines changed

1 file changed

+19
-21
lines changed

drivers/nvme/target/core.c

+19-21
Original file line numberDiff line numberDiff line change
@@ -606,13 +606,19 @@ int nvmet_ns_enable(struct nvmet_ns *ns)
606606
goto out_dev_put;
607607
}
608608

609+
if (percpu_ref_init(&ns->ref, nvmet_destroy_namespace, 0, GFP_KERNEL))
610+
goto out_pr_exit;
611+
609612
nvmet_ns_changed(subsys, ns->nsid);
610613
ns->enabled = true;
611614
xa_set_mark(&subsys->namespaces, ns->nsid, NVMET_NS_ENABLED);
612615
ret = 0;
613616
out_unlock:
614617
mutex_unlock(&subsys->lock);
615618
return ret;
619+
out_pr_exit:
620+
if (ns->pr.enable)
621+
nvmet_pr_exit_ns(ns);
616622
out_dev_put:
617623
list_for_each_entry(ctrl, &subsys->ctrls, subsys_entry)
618624
pci_dev_put(radix_tree_delete(&ctrl->p2p_ns_map, ns->nsid));
@@ -638,6 +644,19 @@ void nvmet_ns_disable(struct nvmet_ns *ns)
638644

639645
mutex_unlock(&subsys->lock);
640646

647+
/*
648+
* Now that we removed the namespaces from the lookup list, we
649+
* can kill the per_cpu ref and wait for any remaining references
650+
* to be dropped, as well as a RCU grace period for anyone only
651+
* using the namepace under rcu_read_lock(). Note that we can't
652+
* use call_rcu here as we need to ensure the namespaces have
653+
* been fully destroyed before unloading the module.
654+
*/
655+
percpu_ref_kill(&ns->ref);
656+
synchronize_rcu();
657+
wait_for_completion(&ns->disable_done);
658+
percpu_ref_exit(&ns->ref);
659+
641660
if (ns->pr.enable)
642661
nvmet_pr_exit_ns(ns);
643662

@@ -660,22 +679,6 @@ void nvmet_ns_free(struct nvmet_ns *ns)
660679
if (ns->nsid == subsys->max_nsid)
661680
subsys->max_nsid = nvmet_max_nsid(subsys);
662681

663-
mutex_unlock(&subsys->lock);
664-
665-
/*
666-
* Now that we removed the namespaces from the lookup list, we
667-
* can kill the per_cpu ref and wait for any remaining references
668-
* to be dropped, as well as a RCU grace period for anyone only
669-
* using the namepace under rcu_read_lock(). Note that we can't
670-
* use call_rcu here as we need to ensure the namespaces have
671-
* been fully destroyed before unloading the module.
672-
*/
673-
percpu_ref_kill(&ns->ref);
674-
synchronize_rcu();
675-
wait_for_completion(&ns->disable_done);
676-
percpu_ref_exit(&ns->ref);
677-
678-
mutex_lock(&subsys->lock);
679682
subsys->nr_namespaces--;
680683
mutex_unlock(&subsys->lock);
681684

@@ -705,9 +708,6 @@ struct nvmet_ns *nvmet_ns_alloc(struct nvmet_subsys *subsys, u32 nsid)
705708
ns->nsid = nsid;
706709
ns->subsys = subsys;
707710

708-
if (percpu_ref_init(&ns->ref, nvmet_destroy_namespace, 0, GFP_KERNEL))
709-
goto out_free;
710-
711711
if (ns->nsid > subsys->max_nsid)
712712
subsys->max_nsid = nsid;
713713

@@ -730,8 +730,6 @@ struct nvmet_ns *nvmet_ns_alloc(struct nvmet_subsys *subsys, u32 nsid)
730730
return ns;
731731
out_exit:
732732
subsys->max_nsid = nvmet_max_nsid(subsys);
733-
percpu_ref_exit(&ns->ref);
734-
out_free:
735733
kfree(ns);
736734
out_unlock:
737735
mutex_unlock(&subsys->lock);

0 commit comments

Comments
 (0)