RE: [PATCH] nvmet_tcp: fix header digest verification

["Engel, Amit" <Amit.Engel@Dell.com>]

One of our tests found that 
With the current code,
recv_digest == exp_digest even if the pdu header digest is being modified (synthetically)
For example:
In case that pdu header digest is changed to 0xffffffff, recv == exp == 0xfffffff, and nvmet_tcp_verify_hdgst returns 0 - which is wrong


Internal Use - Confidential

-----Original Message-----
From: Sagi Grimberg <sagi@grimberg.me> 
Sent: Tuesday, October 19, 2021 3:19 PM
To: Engel, Amit; linux-nvme@lists.infradead.org; hch@lst.de
Subject: Re: [PATCH] nvmet_tcp: fix header digest verification


[EXTERNAL EMAIL] 


> pass the correct length to nvmet_tcp_hdgst.
> when verify_hdgst, need to pass pdu header len.
> This fixes a wrong behaviour where
> header digest verification pass although digest is wrong

Amit, can you describe what you mean here?
what is wrong about the digest?

What is currently passed in len is queue->offset, which is the temporary offset from the pdu, which is equal to hdr->hlen because we verify the digest right after we finish reading the PDU header.

> 
> Signed-off-by: Amit Engel <amit.engel@dell.com>
> ---
>   drivers/nvme/target/tcp.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/nvme/target/tcp.c b/drivers/nvme/target/tcp.c 
> index 6eb0b3153477..954c27a94561 100644
> --- a/drivers/nvme/target/tcp.c
> +++ b/drivers/nvme/target/tcp.c
> @@ -268,7 +268,7 @@ static int nvmet_tcp_verify_hdgst(struct nvmet_tcp_queue *queue,
>   	}
>   
>   	recv_digest = *(__le32 *)(pdu + hdr->hlen);
> -	nvmet_tcp_hdgst(queue->rcv_hash, pdu, len);
> +	nvmet_tcp_hdgst(queue->rcv_hash, pdu, hdr->hlen);

If at all, the caller should pass hdr->hlen and keep this generic.