[PATCH 2/3] nvme-tcp: sanitize request list handling

[Hannes Reinecke]

On 4/14/25 01:00, Sagi Grimberg wrote:
> 
> 
> On 03/04/2025 9:55, Hannes Reinecke wrote:
>> Validate the request in nvme_tcp_handle_r2t() to ensure it's not
>> part of any list, otherwise a malicious R2T PDU might inject a
>> loop in request list processing.
> 
> Not clear what do you mean by "malicious R2T PDU".
> Can you please clarify what you have seen/observed in the commit msg 
> (i.e. what led you to this patch)?
> 
This is coming from code inspection only.
In nvme_tcp_handle_r2t() we are looking up a request by the 'command_id'
value in the pdu, and then add it to 'queue->req_list' without further
checking.
So if a malicious R2T packet is received containing the command_id of
a command currently on 'queue->req_list' we'll end up with duplicate
entries on the list.

[ .. ]

>> @@ -773,7 +785,8 @@ static int nvme_tcp_handle_r2t(struct 
>> nvme_tcp_queue *queue,
>>       nvme_tcp_setup_h2c_data_pdu(req);
>>       llist_add(&req->lentry, &queue->req_list);
>> -    queue_work_on(queue->io_cpu, nvme_tcp_wq, &queue->io_work);
>> +    if (list_empty(&queue->send_list))
>> +        queue_work_on(queue->io_cpu, nvme_tcp_wq, &queue->io_work);
> 
> Is this change mandatory? looks out of place.
> 
Arguably an optimisation. I can leave it out.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare at suse.de                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich