[PATCH 2/3] nvmet-tcp: Don't free SQ on authentication success

alistair23@gmail.com posted 3 patches 1 month, 2 weeks ago
[PATCH 2/3] nvmet-tcp: Don't free SQ on authentication success
Posted by alistair23@gmail.com 1 month, 2 weeks ago
From: Alistair Francis <alistair.francis@wdc.com>

Curently after the host sends a REPLACETLSPSK we free the TLS keys as
part of calling nvmet_auth_sq_free() on success. This means when the
host sends a follow up REPLACETLSPSK we return CONCAT_MISMATCH as the
check for !nvmet_queue_tls_keyid(req->sq) fails.

This patch ensures we don't free the TLS key on success as we might need
it again in the future.

Signed-off-by: Alistair Francis <alistair.francis@wdc.com>
---
 drivers/nvme/target/fabrics-cmd-auth.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/nvme/target/fabrics-cmd-auth.c b/drivers/nvme/target/fabrics-cmd-auth.c
index f71456a94b66..5cb857d21dfd 100644
--- a/drivers/nvme/target/fabrics-cmd-auth.c
+++ b/drivers/nvme/target/fabrics-cmd-auth.c
@@ -574,9 +574,7 @@ void nvmet_execute_auth_receive(struct nvmet_req *req)
 	status = nvmet_copy_to_sgl(req, 0, d, al);
 	kfree(d);
 done:
-	if (req->sq->dhchap_step == NVME_AUTH_DHCHAP_MESSAGE_SUCCESS2)
-		nvmet_auth_sq_free(req->sq);
-	else if (req->sq->dhchap_step == NVME_AUTH_DHCHAP_MESSAGE_FAILURE1) {
+	if (req->sq->dhchap_step == NVME_AUTH_DHCHAP_MESSAGE_FAILURE1) {
 		nvmet_auth_sq_free(req->sq);
 		nvmet_ctrl_fatal_error(ctrl);
 	}
-- 
2.51.0
Re: [PATCH 2/3] nvmet-tcp: Don't free SQ on authentication success
Posted by Christoph Hellwig 1 month, 1 week ago
On Thu, Oct 30, 2025 at 01:51:13PM +1000, alistair23@gmail.com wrote:
> Curently after the host sends a REPLACETLSPSK we free the TLS keys as
> part of calling nvmet_auth_sq_free() on success. This means when the
> host sends a follow up REPLACETLSPSK we return CONCAT_MISMATCH as the
> check for !nvmet_queue_tls_keyid(req->sq) fails.
> 
> This patch ensures we don't free the TLS key on success as we might need
> it again in the future.

I initially though we'd now keep it around for the lifetime of the
queue, but I think we'll still free it from nvmet_execute_auth_send,
right?
Re: [PATCH 2/3] nvmet-tcp: Don't free SQ on authentication success
Posted by Alistair Francis 1 month, 1 week ago
On Sat, Nov 1, 2025 at 12:03 AM Christoph Hellwig <hch@lst.de> wrote:
>
> On Thu, Oct 30, 2025 at 01:51:13PM +1000, alistair23@gmail.com wrote:
> > Curently after the host sends a REPLACETLSPSK we free the TLS keys as
> > part of calling nvmet_auth_sq_free() on success. This means when the
> > host sends a follow up REPLACETLSPSK we return CONCAT_MISMATCH as the
> > check for !nvmet_queue_tls_keyid(req->sq) fails.
> >
> > This patch ensures we don't free the TLS key on success as we might need
> > it again in the future.
>
> I initially though we'd now keep it around for the lifetime of the
> queue, but I think we'll still free it from nvmet_execute_auth_send,
> right?

Good point. In theory nvmet_execute_auth_send() can free it, I don't
see that actually happening as `req->sq->dhchap_step` is always
`NVME_AUTH_DHCHAP_MESSAGE_NEGOTIATE` for me.

I'll add another patch to remove the call to nvmet_auth_sq_free() on success.

Alistair

>