[PATCH] nvme-core: warn and abort if shared ns with empty list

[Keith Busch]

On Tue, Jun 06, 2023 at 10:21:34AM +0200, Irvin Cote wrote:
> In nvme_find_ns_head if we find a shared ns but it has an empty
> list or its refcount is 0 we gloss over it and go to the next.
> Ignoring when such a case happens means that when nvme_find_ns_head
> returns NULL, we could create a new nshead with the same nsid. Hence
> we could find ourselves with two shared nsheads having the same nsid.
> Instead warn the user through dev_err and exit.

Let's say you're holding a reference on the old "head" and then hot
remove the drive. The kernel will tear down the device and its
namespace, but the subsystem and empty head will remain due to that
held reference.

Now plug the drive back in. Today, the controller will attach to the
previous subsystem, and make a new "head" for its namespace (because the
old one is empty), and you can continue from there, though the new
handle names may look a bit odd.

This change won't make the re-added namespace visible to the user, which
doesn't sound right.
 
> Signed-off-by: Irvin Cote <irvincoteg at gmail.com>
> ---
>  drivers/nvme/host/core.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index c8a471482421..175f1516083f 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -3341,6 +3341,11 @@ static struct nvme_ns_head *nvme_find_ns_head(struct nvme_ctrl *ctrl,
>  			continue;
>  		if (!list_empty(&h->list) && nvme_tryget_ns_head(h))
>  			return h;
> +		else {
> +			dev_err(ctrl->device,
> +				"Found shared namespace with empty list or 0 refcount\n");

I don't think we're guaranteed this path is using a "shared" namespace.

> +			return ERR_PTR(-ENOENT);
> +		}
>  	}
>  
>  	return NULL;