[PATCH V3 2/3] nvmet_auth: use common helper for buffer alloc

Max Gurtovoy mgurtovoy at nvidia.com
Thu Jun 8 05:41:13 PDT 2023



On 06/06/2023 1:02, Sagi Grimberg wrote:
> 
> 
> On 6/5/23 12:19, Chaitanya Kulkarni wrote:
>> Add a common helper to factor out buffer allocation in
>> nvmet_execute_auth_send() and nvmet_execute_auth_receive() and call it
>> from nvmet_auth_common_prep() once we done with the secp/spsp0/spsp1
>> check.
>>
>> Only functional change in this patch is transfer buffer allocation is
>> moved before nvmet_check_transfer_len() and it is freed if when
>> nvmet_check_transfer_len() fails. But similar allocation and free is
>> used in error unwind path in nvme code and it is not in fast path, so
>> it shuold be fine.
> 
> This is asking for a future memory leak. It kinda makes sense to
> allocate after we run sanity checks on the request...
> 

Also I don't understand why do we need helper function for malloc ?
Allocating using nvmet_auth_alloc_transfer_buffer() and freeing using 
free() is just make the driver maintenance harder.

Another problem in this patch is that the len in the recv buffer is 
le32_to_cpu(req->cmd->auth_receive.al) and in the send is 
le32_to_cpu(req->cmd->auth_send.tl) so it's logically wrong to unify 
them. Same for error_loc.

maybe just don't include this patch 2/3 in V4 ?



More information about the Linux-nvme mailing list