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

[Maurizio Lombardi]

On Wed Mar 12, 2025 at 9:16 AM CET, Hannes Reinecke wrote:
> On 3/11/25 19:53, Chris Leech wrote:
>>> @@ -759,8 +762,12 @@ static int nvme_tcp_handle_r2t(struct nvme_tcp_queue *queue,
>>>   
>>>   	nvme_tcp_setup_h2c_data_pdu(req);
>>>   
>>> +	WARN_ON(queue->request == req);
>>> +	WARN_ON(llist_on_list(&req->lentry));
>>> +	WARN_ON(!list_empty(&req->entry));
>>>   	llist_add(&req->lentry, &queue->req_list);
>> 
>> Are we happy with a WARN here, or should this be handled as an error?
>> The idea of an duplicate R2Ts creating a loop in req_list is
>> frightening.
>> 
> It is, but wanted to check if others see this as an issue, too.
> We actually should bail out and reset the connection; BUG_ON()
> is pretty harsh, and not really appropriate as this isn't an error
> on our side.

So a malicious or buggy target could trigger WARNs host-side? Did I understand it
correctly?

This doesn't sound ok to me, I think the host should print an error
message and reset the connection.

Maurizio