[PATCH] nvmet: Avoid potential UAF in nvmet_req_complete()

[Damien Le Moal]

On 3/5/23 23:42, Bart Van Assche wrote:
> On 3/4/23 21:07, Damien Le Moal wrote:
>> A host ->queue_response() operation implementation may free the request
>> passed as argument. Such implementation potentially could result
>> in use after free of the request pointer when percpu_ref_put() is called
>> in nvmet_req_complete().
>>
>> Avoid such problem by using a local variable to save the sq pointer
>> before calling __nvmet_req_complete(), thus avoiding dereferencing the
>> req pointer.
>>
>> Signed-off-by: Damien Le Moal <damien.lemoal at opensource.wdc.com>
> 
> How about adding a Fixes: tag?

OK. Will find that out and add one.

> 
>> diff --git a/drivers/nvme/target/core.c b/drivers/nvme/target/core.c
>> index 597d1b2c0d52..0f9193323a02 100644
>> --- a/drivers/nvme/target/core.c
>> +++ b/drivers/nvme/target/core.c
>> @@ -758,8 +758,10 @@ static void __nvmet_req_complete(struct nvmet_req *req, u16 status)
>>   
>>   void nvmet_req_complete(struct nvmet_req *req, u16 status)
>>   {
>> +	struct nvmet_sq *sq = req->sq;
>> +
>>   	__nvmet_req_complete(req, status);
>> -	percpu_ref_put(&req->sq->ref);
>> +	percpu_ref_put(&sq->ref);
>>   }
>>   EXPORT_SYMBOL_GPL(nvmet_req_complete);
> 
> Interesting. The above patch looks similar to "[PATCH] nvmet: Fix a 
> use-after-free" 
> (https://lore.kernel.org/linux-nvme/20220812210317.2252051-1-bvanassche@acm.org/). 
> One could wonder whether there are more occurrences of this pattern?

I hope not :)
With this patch, the PCIe endpoint nvme driver I am working on has stopped
randomly crashing.


-- 
Damien Le Moal
Western Digital Research