Thread (6 messages) 6 messages, 2 authors, 2021-10-27

RE: [PATCH] nvmet_tcp: fix header digest verification

From: Engel, Amit <hidden>
Date: 2021-10-20 15:43:47

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.
quoted hunk ↗ jump to hunk
Signed-off-by: Amit Engel <redacted>
---
  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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help