Skip to contentSkip to conversation

nvme: ns_head vs namespace mismatch fixes

PATCH In review Anton Eidelman 3 participants 5 messages Mar 14, 2021 – Mar 15, 2021

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

Please, apply the following two upstream commits (attached) (in this order):
d567572906d9 nvme: unlink head after removing last namespace ac262508daa8 nvme: release namespace head reference on error

TO: v5.4, v5.5, v5.6, v5.7
These commits are present in v5.8
and apply cleanly to the above.

Reason:
These fix a potential crash or malfunction when an nvme namespace is deleted
and then a new namespace with the same nsid is created before the old ns_head for this nsid is gone.

The first commit prevents the new namespace
from being matched by nvme_init_ns_head()
with the old ns_head causing ID mismatch
and consequently a failure to initialize the new namespace.

The second commit prevents ns_head refcount imbalance in case nvme_init_ns_head() detects ID mismatch, and consequently a crash later.

Signature · 19 lines
*Lightbits Labs**
*Lead the cloud-native data center
transformation by 
delivering *scalable *and *efficient *software
defined storage that is 
*easy *to consume.



*This message is sent in confidence for the addressee 
only.  It
may contain legally privileged information. The contents are not 
to be
disclosed to anyone other than the addressee. Unauthorized recipients 
are
requested to preserve this confidentiality, advise the sender 
immediately of
any error in transmission and delete the email from their 
systems.*
#1 lore raw

PATCH 1/3 nvme: unlink head after removing last namespace

From: Keith Busch <[email protected]>

The driver had been unlinking the namespace head from the subsystem's list only after the last reference was released, and outside of the list's subsys->lock protection.

There is no reason to track an empty head, so unlink the entry from the subsystem's list when the last namespace using that head is removed and with the mutex lock protecting the list update. The next namespace to attach reusing the previous NSID will allocate a new head rather than find the old head with mismatched identifiers.

---
 drivers/nvme/host/core.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)
drivers/nvme/host/core.c +5 -2
index d0f65322156c..15b9e60be204 100644
@@ -433,7 +433,6 @@ static void nvme_free_ns_head(struct kref *ref)
 
 	nvme_mpath_remove_disk(head);
 	ida_simple_remove(&head->subsys->ns_ida, head->instance);
-	list_del_init(&head->entry);
 	cleanup_srcu_struct(&head->srcu);
 	nvme_put_subsystem(head->subsys);
 	kfree(head);
@@ -3414,7 +3413,6 @@ static int __nvme_check_ids(struct nvme_subsystem *subsys,
 
 	list_for_each_entry(h, &subsys->nsheads, entry) {
 		if (nvme_ns_ids_valid(&new->ids) &&
-		    !list_empty(&h->list) &&
 		    nvme_ns_ids_equal(&new->ids, &h->ids))
 			return -EINVAL;
 	}
@@ -3660,6 +3658,8 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl, unsigned nsid)
  out_unlink_ns:
 	mutex_lock(&ctrl->subsys->lock);
 	list_del_rcu(&ns->siblings);
+	if (list_empty(&ns->head->list))
+		list_del_init(&ns->head->entry);
 	mutex_unlock(&ctrl->subsys->lock);
 	nvme_put_ns_head(ns->head);
  out_free_id:
@@ -3679,7 +3679,10 @@ static void nvme_ns_remove(struct nvme_ns *ns)
 
 	mutex_lock(&ns->ctrl->subsys->lock);
 	list_del_rcu(&ns->siblings);
+	if (list_empty(&ns->head->list))
+		list_del_init(&ns->head->entry);
 	mutex_unlock(&ns->ctrl->subsys->lock);
+
 	synchronize_rcu(); /* guarantee not available in head->list */
 	nvme_mpath_clear_current_path(ns);
 	synchronize_srcu(&ns->head->srcu); /* wait for concurrent submissions */
Signature · 25 lines
2.25.1


-- 


*Lightbits Labs**
*Lead the cloud-native data center
transformation by 
delivering *scalable *and *efficient *software
defined storage that is 
*easy *to consume.



*This message is sent in confidence for the addressee 
only.  It
may contain legally privileged information. The contents are not 
to be
disclosed to anyone other than the addressee. Unauthorized recipients 
are
requested to preserve this confidentiality, advise the sender 
immediately of
any error in transmission and delete the email from their 
systems.*
maintainer
#2 lore raw

Re: [PATCH 1/3] nvme: unlink head after removing last namespace

Quoted 11 lines · Anton Eidelman

*This message is sent in confidence for the addressee only. It
may contain legally privileged information. The contents are not to be
disclosed to anyone other than the addressee. Unauthorized recipients are
requested to preserve this confidentiality, advise the sender immediately of
any error in transmission and delete the email from their systems.*

email deleted.

maintainer
#3 lore raw
In reply to Anton Eidelman
Quoted 10 lines · Anton Eidelman

*This message is sent in confidence for the addressee only. It
may contain legally privileged information. The contents are not to be
disclosed to anyone other than the addressee. Unauthorized recipients are
requested to preserve this confidentiality, advise the sender immediately of
any error in transmission and delete the email from their systems.*

This text is not compatible with kernel development in public, email is now deleted.

maintainer
#4 lore raw
Quoted 3 lines

This text is not compatible with kernel development in public, email is now deleted.

Sorry for that Greg, this was an oversight as this is an auto-added footer to the corporate email... obviously not the intention here.