[PATCH] nvmet: Free the old negotiation state when restarting authentication

[PATCH] nvmet: Free the old negotiation state when restarting authentication

[Guixin Liu]

Restarting a DH-HMAC-CHAP negotiation with SEND(NEGOTIATE) resets
sq->dhchap_step but does not release the previous c1/c2/skey, which
are only freed in nvmet_auth_sq_free() at teardown.  The next
CHALLENGE/REPLY exchange then overwrites those pointers with fresh
allocations.

An unauthenticated host can repeat the negotiate message endlessly,
leaking a few dozen bytes per round and slowly exhausting target
memory.

Clear the old negotiation state with nvmet_auth_sq_free() before
setting up the new one.

Fixes: db1312dd95488 ("nvmet: implement basic In-Band Authentication")
Cc: stable@vger.kernel.org
Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
---
 drivers/nvme/target/fabrics-cmd-auth.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/nvme/target/fabrics-cmd-auth.c b/drivers/nvme/target/fabrics-cmd-auth.c
index bc49b48cdd63..3d9840d37a8b 100644
--- a/drivers/nvme/target/fabrics-cmd-auth.c
+++ b/drivers/nvme/target/fabrics-cmd-auth.c
@@ -330,6 +330,7 @@ void nvmet_execute_auth_send(struct nvmet_req *req)
 			/* Restart negotiation */
 			pr_debug("%s: ctrl %d qid %d reset negotiation\n",
 				 __func__, ctrl->cntlid, req->sq->qid);
+			nvmet_auth_sq_free(req->sq);
 			if (!req->sq->qid) {
 				dhchap_status = nvmet_setup_auth(ctrl, req->sq,
 								 true);
-- 
2.43.7

Re: [PATCH] nvmet: Free the old negotiation state when restarting authentication

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] nvmet-auth: NULL deref on restart due to stale dhchap_skey_len
--

commit b95a68b07cb2fd80ef9528e69f5467fefe0e42a1
Author: Guixin Liu <kanie@linux.alibaba.com>

nvmet: Free the old negotiation state when restarting authentication

This commit calls nvmet_auth_sq_free() when restarting DH-HMAC-CHAP
negotiation in nvmet_execute_auth_send() so that previously allocated
c1, c2, and skey buffers on the submission queue are freed before a new
round begins.

> diff --git a/drivers/nvme/target/fabrics-cmd-auth.c b/drivers/nvme/target/fabrics-cmd-auth.c
> index 1fe21f2672f10..e68b1c6f2ce5a 100644
> --- a/drivers/nvme/target/fabrics-cmd-auth.c
> +++ b/drivers/nvme/target/fabrics-cmd-auth.c
> @@ -330,6 +330,7 @@ void nvmet_execute_auth_send(struct nvmet_req *req)
>  			/* Restart negotiation */
>  			pr_debug("%s: ctrl %d qid %d reset negotiation\n",
>  				 __func__, ctrl->cntlid, req->sq->qid);
> +			nvmet_auth_sq_free(req->sq);

[Severity: Critical]
Can this trigger a NULL pointer dereference in nvmet_auth_host_hash() if
the host restarts negotiation after generating a DH session key and then
sends a reply with dhvlen == 0?

In the first round, when a DH group is configured (ctrl->dh_gid !=
NVME_AUTH_DHGROUP_NULL) and the host sends a reply with a non-zero dhvlen,
nvmet_auth_reply() -> nvmet_auth_ctrl_sesskey() allocates
req->sq->dhchap_skey and sets req->sq->dhchap_skey_len to the hash length.
If that reply fails authentication with NVME_AUTH_DHCHAP_MESSAGE_FAILURE1,
the host can send a new negotiate message to restart authentication.

Calling nvmet_auth_sq_free() here frees sq->dhchap_skey and sets it to
NULL, but leaves sq->dhchap_skey_len non-zero:

drivers/nvme/target/auth.c:nvmet_auth_sq_free() {
	...
	kfree(sq->dhchap_skey);
	sq->dhchap_skey = NULL;
}

If the host then completes the negotiate and challenge steps and sends a
reply with dhvlen == 0 in the second round, nvmet_auth_reply() skips
nvmet_auth_ctrl_sesskey():

drivers/nvme/target/fabrics-cmd-auth.c:nvmet_auth_reply() {
	...
	if (dhvlen) {
		if (!ctrl->dh_tfm)
			return NVME_AUTH_DHCHAP_FAILURE_INCORRECT_PAYLOAD;
		if (nvmet_auth_ctrl_sesskey(req, data->rval + 2 * data->hl,
					    dhvlen) < 0)
			return NVME_AUTH_DHCHAP_FAILURE_DHGROUP_UNUSABLE;
	}
	...
	if (nvmet_auth_host_hash(req, response, data->hl) < 0) {
}

nvmet_auth_host_hash() then passes the NULL req->sq->dhchap_skey along with
the stale non-zero req->sq->dhchap_skey_len to
nvme_auth_augmented_challenge():

drivers/nvme/target/auth.c:nvmet_auth_host_hash() {
	...
	if (ctrl->dh_gid != NVME_AUTH_DHGROUP_NULL) {
		challenge = kmalloc(shash_len, GFP_KERNEL);
		if (!challenge) {
			ret = -ENOMEM;
			goto out_free_response;
		}
		ret = nvme_auth_augmented_challenge(ctrl->shash_id,
						    req->sq->dhchap_skey,
						    req->sq->dhchap_skey_len,
						    req->sq->dhchap_c1,
						    challenge, shash_len);
}

Because skey_len is non-zero while skey is NULL,
nvme_auth_augmented_challenge() -> nvme_auth_hmac() ->
nvme_auth_hmac_init() dereferences NULL when preparing the HMAC key.

Should nvmet_auth_sq_free() also reset sq->dhchap_skey_len = 0 (or should
nvmet_auth_reply() require dhvlen != 0 when ctrl->dh_gid !=
NVME_AUTH_DHGROUP_NULL)?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261028064345.544504-1-kanie@linux.alibaba.com?part=1

Re: [PATCH] nvmet: Free the old negotiation state when restarting authentication

[Guixin Liu]

在 2026/10/28 14:59, sashiko-bot@kernel.org 写道:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Critical] nvmet-auth: NULL deref on restart due to stale dhchap_skey_len
> --
>
> commit b95a68b07cb2fd80ef9528e69f5467fefe0e42a1
> Author: Guixin Liu <kanie@linux.alibaba.com>
>
> nvmet: Free the old negotiation state when restarting authentication
>
> This commit calls nvmet_auth_sq_free() when restarting DH-HMAC-CHAP
> negotiation in nvmet_execute_auth_send() so that previously allocated
> c1, c2, and skey buffers on the submission queue are freed before a new
> round begins.
>
>> diff --git a/drivers/nvme/target/fabrics-cmd-auth.c b/drivers/nvme/target/fabrics-cmd-auth.c
>> index 1fe21f2672f10..e68b1c6f2ce5a 100644
>> --- a/drivers/nvme/target/fabrics-cmd-auth.c
>> +++ b/drivers/nvme/target/fabrics-cmd-auth.c
>> @@ -330,6 +330,7 @@ void nvmet_execute_auth_send(struct nvmet_req *req)
>>  			/* Restart negotiation */
>>  			pr_debug("%s: ctrl %d qid %d reset negotiation\n",
>>  				 __func__, ctrl->cntlid, req->sq->qid);
>> +			nvmet_auth_sq_free(req->sq);
> [Severity: Critical]
> Can this trigger a NULL pointer dereference in nvmet_auth_host_hash() if
> the host restarts negotiation after generating a DH session key and then
> sends a reply with dhvlen == 0?
>
> In the first round, when a DH group is configured (ctrl->dh_gid !=
> NVME_AUTH_DHGROUP_NULL) and the host sends a reply with a non-zero dhvlen,
> nvmet_auth_reply() -> nvmet_auth_ctrl_sesskey() allocates
> req->sq->dhchap_skey and sets req->sq->dhchap_skey_len to the hash length.
> If that reply fails authentication with NVME_AUTH_DHCHAP_MESSAGE_FAILURE1,
> the host can send a new negotiate message to restart authentication.
>
> Calling nvmet_auth_sq_free() here frees sq->dhchap_skey and sets it to
> NULL, but leaves sq->dhchap_skey_len non-zero:
>
> drivers/nvme/target/auth.c:nvmet_auth_sq_free() {
> 	...
> 	kfree(sq->dhchap_skey);
> 	sq->dhchap_skey = NULL;
> }
>
> If the host then completes the negotiate and challenge steps and sends a
> reply with dhvlen == 0 in the second round, nvmet_auth_reply() skips
> nvmet_auth_ctrl_sesskey():
>
> drivers/nvme/target/fabrics-cmd-auth.c:nvmet_auth_reply() {
> 	...
> 	if (dhvlen) {
> 		if (!ctrl->dh_tfm)
> 			return NVME_AUTH_DHCHAP_FAILURE_INCORRECT_PAYLOAD;
> 		if (nvmet_auth_ctrl_sesskey(req, data->rval + 2 * data->hl,
> 					    dhvlen) < 0)
> 			return NVME_AUTH_DHCHAP_FAILURE_DHGROUP_UNUSABLE;
> 	}
> 	...
> 	if (nvmet_auth_host_hash(req, response, data->hl) < 0) {
> }
>
> nvmet_auth_host_hash() then passes the NULL req->sq->dhchap_skey along with
> the stale non-zero req->sq->dhchap_skey_len to
> nvme_auth_augmented_challenge():
>
> drivers/nvme/target/auth.c:nvmet_auth_host_hash() {
> 	...
> 	if (ctrl->dh_gid != NVME_AUTH_DHGROUP_NULL) {
> 		challenge = kmalloc(shash_len, GFP_KERNEL);
> 		if (!challenge) {
> 			ret = -ENOMEM;
> 			goto out_free_response;
> 		}
> 		ret = nvme_auth_augmented_challenge(ctrl->shash_id,
> 						    req->sq->dhchap_skey,
> 						    req->sq->dhchap_skey_len,
> 						    req->sq->dhchap_c1,
> 						    challenge, shash_len);
> }
>
> Because skey_len is non-zero while skey is NULL,
> nvme_auth_augmented_challenge() -> nvme_auth_hmac() ->
> nvme_auth_hmac_init() dereferences NULL when preparing the HMAC key.
>
> Should nvmet_auth_sq_free() also reset sq->dhchap_skey_len = 0 (or should
> nvmet_auth_reply() require dhvlen != 0 when ctrl->dh_gid !=
> NVME_AUTH_DHGROUP_NULL)?
Sashiko is right, I should set dhchap_skey_len to zero in nvmet_auth_sq_free().
Changed in v2.
>