[PATCH 1/1] nvme: correctly account for namespace head reference counter

[Nilay Shroff]

On 6/23/25 1:48 PM, Daniel Wagner wrote:
> Hi Nilay,
> 
> On Fri, Jun 20, 2025 at 01:32:43PM +0530, Nilay Shroff wrote:
>> --- a/drivers/nvme/host/core.c
>> +++ b/drivers/nvme/host/core.c
>> @@ -4089,6 +4089,7 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl, struct nvme_ns_info *info)
>>  	struct nvme_ns *ns;
>>  	struct gendisk *disk;
>>  	int node = ctrl->numa_node;
>> +	bool last_path = false;
>>  
>>  	ns = kzalloc_node(sizeof(*ns), GFP_KERNEL, node);
>>  	if (!ns)
>> @@ -4181,9 +4182,13 @@ static void nvme_alloc_ns(struct nvme_ctrl *ctrl, struct nvme_ns_info *info)
>>   out_unlink_ns:
>>  	mutex_lock(&ctrl->subsys->lock);
>>  	list_del_rcu(&ns->siblings);
>> -	if (list_empty(&ns->head->list))
>> +	if (list_empty(&ns->head->list)) {
>>  		list_del_init(&ns->head->entry);
>> +		last_path = true;
>> +	}
>>  	mutex_unlock(&ctrl->subsys->lock);
>> +	if (last_path)
>> +		nvme_put_ns_head(ns->head);
>>  	nvme_put_ns_head(ns->head);
> 
> Could the put moved in front of 'goto out_unlink_ns'?

I believe you would want to move the second nvme_put_ns_head()
above as the first statement under out_unlink_ns label as shown
below:

out_unlink_ns:
	nvme_put_ns_head(ns->head);
	mutex_lock(&ctrl->subsys->lock);
	list_del_rcu(&ns->siblings);
	if (list_empty(&ns->head->list)) {
		list_del_init(&ns->head->entry);
                last_path = true;
        }
	mutex_unlock(&ctrl->subsys->lock);
        if (last_path)
                nvme_put_ns_head(ns->head);

If that's the case, and this is indeed the last reference to ns->head,
then nvme_put_ns_head() could potentially free the ns->head object. 
If that happens, any access to ns->head afterward — such as ns->head->list
— would result in a use-after-free and potentially trigger a kernel oops,
correct?

Thanks,
--Nilay