Re: [PATCH] nvmet: Fix a use-after-free

[Bart Van Assche <bvanassche@acm.org>]

On 8/14/22 04:45, Sagi Grimberg wrote:
> On 8/13/22 00:03, Bart Van Assche wrote:
>> @@ -745,9 +747,9 @@ static void __nvmet_req_complete(struct nvmet_req 
>> *req, u16 status)
>>       trace_nvmet_req_complete(req);
>> -    if (req->ns)
>> -        nvmet_put_namespace(req->ns);
>>       req->ops->queue_response(req);
>> +    if (ns)
>> +        nvmet_put_namespace(ns);
> 
> Why did the put change position?
> I'm not exactly clear what was used-after-free here..

Hi Sagi,

Is my understanding correct that the NVMe target namespace owns the 
block device `req` is associated with and hence that the namespace 
reference count must only be dropped after dereferencing the `req` 
pointer has finished?

This is what I found in the NVMe target code:
* nvmet_put_namespace() decreases ns->ref.
* Dropping the last ns->ref causes nvmet_destroy_namespace() to be
   called. That function completes ns->disable_done.
* nvmet_ns_disable() waits on that completion and calls
   nvmet_ns_dev_disable().
* For a block device, nvmet_ns_dev_disable() calls blkdev_put().
* The last blkdev_put() call calls disk_release().
* disk_release() calls blk_put_queue().
* The last blk_put_queue() call calls blk_release_queue().
* blk_release_queue() frees struct request_queue.
* blk_mq_complete_request_remote() dereferences req->q.

Thanks,

Bart.