Re: [PATCH] nvme: avoid double free special payload

[Max Gurtovoy <mgurtovoy@nvidia.com>]

hi,

On 11/06/2024 13:53, Sagi Grimberg wrote:
> Looks reasonable.
>
> Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
>
>
> On 11/06/2024 13:02, brookxu.cn wrote:
>> From: Chunguang Xu <chunguang.xu@shopee.com>
>>
>> Now we may double free spacial payload for some requests, such as
>> discard. This will corrupt the memory and lead to kernel crash. Now we
>> will free special payload before retry it. If we disconnect device
>> before reconnect success, then we will fail request by
>> nvme_fail_nonready_command(), as a result we will double free
>> special payload. Here try to fix it, we may can clear RQF_SPECIAL_LOAD
>> bit after we cleanup command. This will not broken following clean
>> logic of blkmq, as nvme request will not be partial complete.
>>
>> Fixes: 16686f3a6c3c ("nvme: move common call to nvme_cleanup_cmd to 
>> core layer")
I'm not sure that this commit caused the bug. The nvme_cleanup_cmd() was 
called in this path also before this commit.
>> Signed-off-by: Chunguang Xu <chunguang.xu@shopee.com>

The fix looks fine to me, but the commit message can be improved a bit 
to be more clear about the scenario.

Reviewed-by: Max Gurtovoy <mgurtovoy@nvidia.com>

>> ---
>>   drivers/nvme/host/core.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
>> index f5d150c62955..c40930d10bd3 100644
>> --- a/drivers/nvme/host/core.c
>> +++ b/drivers/nvme/host/core.c
>> @@ -998,6 +998,7 @@ void nvme_cleanup_cmd(struct request *req)
>>               clear_bit_unlock(0, &ctrl->discard_page_busy);
>>           else
>>               kfree(bvec_virt(&req->special_vec));
>> +        req->rq_flags &= ~RQF_SPECIAL_PAYLOAD;
>>       }
>>   }
>>   EXPORT_SYMBOL_GPL(nvme_cleanup_cmd);
>