[PATCH] nvmet: fix heap out-of-bounds read in nvmet_auth_negotiate()

[PATCH] nvmet: fix heap out-of-bounds read in nvmet_auth_negotiate()

[Guixin Liu]

nvmet_execute_auth_send() allocates the DH-HMAC-CHAP message buffer with
the host-supplied transfer length (tl) and hands it to
nvmet_auth_negotiate() without passing tl along. nvmet_auth_negotiate()
then reads the negotiate header and, for each of the halen hash
identifiers and dhlen DH group identifiers, indexes into the fixed
idlist[60] array (hashes at idlist[0..halen), groups at idlist[30..]).

Neither the transfer length nor halen/dhlen is validated. A malicious or
non-conformant host can report a tl smaller than the negotiate structure,
or a halen/dhlen larger than the array (both are u8, up to 255), making
the loops read past the end of the allocated buffer (heap out-of-bounds
read). The sibling nvmet_auth_reply() already validates tl against the
structure size; the negotiate path did not.

Pass tl into nvmet_auth_negotiate(), reject a tl that does not cover the
negotiate data plus one full protocol descriptor, and reject halen/dhlen
larger than NVME_AUTH_DHCHAP_MAX_DH_IDS.

Fixes: db1312dd9548 ("nvmet: implement basic In-Band Authentication")
Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
---
 drivers/nvme/target/fabrics-cmd-auth.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/nvme/target/fabrics-cmd-auth.c b/drivers/nvme/target/fabrics-cmd-auth.c
index d1b39e64d877..d015968bae2d 100644
--- a/drivers/nvme/target/fabrics-cmd-auth.c
+++ b/drivers/nvme/target/fabrics-cmd-auth.c
@@ -31,12 +31,16 @@ void nvmet_auth_sq_init(struct nvmet_sq *sq)
 	sq->dhchap_step = NVME_AUTH_DHCHAP_MESSAGE_NEGOTIATE;
 }
 
-static u8 nvmet_auth_negotiate(struct nvmet_req *req, void *d)
+static u8 nvmet_auth_negotiate(struct nvmet_req *req, void *d, u32 tl)
 {
 	struct nvmet_ctrl *ctrl = req->sq->ctrl;
 	struct nvmf_auth_dhchap_negotiate_data *data = d;
 	int i, hash_id = 0, fallback_hash_id = 0, dhgid, fallback_dhgid;
 
+	if (tl < sizeof(*data) +
+		 sizeof(struct nvmf_auth_dhchap_protocol_descriptor))
+		return NVME_AUTH_DHCHAP_FAILURE_INCORRECT_PAYLOAD;
+
 	pr_debug("%s: ctrl %d qid %d: data sc_d %d napd %d authid %d halen %d dhlen %d\n",
 		 __func__, ctrl->cntlid, req->sq->qid,
 		 data->sc_c, data->napd, data->auth_protocol[0].dhchap.authid,
@@ -72,6 +76,10 @@ static u8 nvmet_auth_negotiate(struct nvmet_req *req, void *d)
 	    NVME_AUTH_DHCHAP_AUTH_ID)
 		return NVME_AUTH_DHCHAP_FAILURE_INCORRECT_PAYLOAD;
 
+	if (data->auth_protocol[0].dhchap.halen > NVME_AUTH_DHCHAP_MAX_DH_IDS ||
+	    data->auth_protocol[0].dhchap.dhlen > NVME_AUTH_DHCHAP_MAX_DH_IDS)
+		return NVME_AUTH_DHCHAP_FAILURE_INCORRECT_PAYLOAD;
+
 	for (i = 0; i < data->auth_protocol[0].dhchap.halen; i++) {
 		u8 host_hmac_id = data->auth_protocol[0].dhchap.idlist[i];
 
@@ -317,7 +325,7 @@ void nvmet_execute_auth_send(struct nvmet_req *req)
 		} else if (data->auth_id != req->sq->dhchap_step)
 			goto done_failure1;
 		/* Validate negotiation parameters */
-		dhchap_status = nvmet_auth_negotiate(req, d);
+		dhchap_status = nvmet_auth_negotiate(req, d, tl);
 		if (dhchap_status == 0)
 			req->sq->dhchap_step =
 				NVME_AUTH_DHCHAP_MESSAGE_CHALLENGE;
-- 
2.43.7

Re: [PATCH] nvmet: fix heap out-of-bounds read in nvmet_auth_negotiate()

[Hannes Reinecke]

On 8/4/26 5:38 AM, Guixin Liu wrote:
> nvmet_execute_auth_send() allocates the DH-HMAC-CHAP message buffer with
> the host-supplied transfer length (tl) and hands it to
> nvmet_auth_negotiate() without passing tl along. nvmet_auth_negotiate()
> then reads the negotiate header and, for each of the halen hash
> identifiers and dhlen DH group identifiers, indexes into the fixed
> idlist[60] array (hashes at idlist[0..halen), groups at idlist[30..]).
> 
> Neither the transfer length nor halen/dhlen is validated. A malicious or
> non-conformant host can report a tl smaller than the negotiate structure,
> or a halen/dhlen larger than the array (both are u8, up to 255), making
> the loops read past the end of the allocated buffer (heap out-of-bounds
> read). The sibling nvmet_auth_reply() already validates tl against the
> structure size; the negotiate path did not.
> 
> Pass tl into nvmet_auth_negotiate(), reject a tl that does not cover the
> negotiate data plus one full protocol descriptor, and reject halen/dhlen
> larger than NVME_AUTH_DHCHAP_MAX_DH_IDS.
> 
> Fixes: db1312dd9548 ("nvmet: implement basic In-Band Authentication")
> Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
> ---
>   drivers/nvme/target/fabrics-cmd-auth.c | 12 ++++++++++--
>   1 file changed, 10 insertions(+), 2 deletions(-)
> 
Reviewed-by: Hannes Reinecke <hare@kernel.org>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@suse.de                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich

Re: [PATCH] nvmet: fix heap out-of-bounds read in nvmet_auth_negotiate()

[Christoph Hellwig]

On Tue, Aug 04, 2026 at 11:38:00AM +0800, Guixin Liu wrote:
> nvmet_execute_auth_send() allocates the DH-HMAC-CHAP message buffer with
> the host-supplied transfer length (tl) and hands it to
> nvmet_auth_negotiate() without passing tl along. nvmet_auth_negotiate()
> then reads the negotiate header and, for each of the halen hash
> identifiers and dhlen DH group identifiers, indexes into the fixed
> idlist[60] array (hashes at idlist[0..halen), groups at idlist[30..]).
> 
> Neither the transfer length nor halen/dhlen is validated. A malicious or
> non-conformant host can report a tl smaller than the negotiate structure,
> or a halen/dhlen larger than the array (both are u8, up to 255), making
> the loops read past the end of the allocated buffer (heap out-of-bounds
> read). The sibling nvmet_auth_reply() already validates tl against the
> structure size; the negotiate path did not.
> 
> Pass tl into nvmet_auth_negotiate(), reject a tl that does not cover the
> negotiate data plus one full protocol descriptor, and reject halen/dhlen
> larger than NVME_AUTH_DHCHAP_MAX_DH_IDS.
> 
> Fixes: db1312dd9548 ("nvmet: implement basic In-Band Authentication")
> Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
> ---
>  drivers/nvme/target/fabrics-cmd-auth.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/nvme/target/fabrics-cmd-auth.c b/drivers/nvme/target/fabrics-cmd-auth.c
> index d1b39e64d877..d015968bae2d 100644
> --- a/drivers/nvme/target/fabrics-cmd-auth.c
> +++ b/drivers/nvme/target/fabrics-cmd-auth.c
> @@ -31,12 +31,16 @@ void nvmet_auth_sq_init(struct nvmet_sq *sq)
>  	sq->dhchap_step = NVME_AUTH_DHCHAP_MESSAGE_NEGOTIATE;
>  }
>  
> -static u8 nvmet_auth_negotiate(struct nvmet_req *req, void *d)
> +static u8 nvmet_auth_negotiate(struct nvmet_req *req, void *d, u32 tl)
>  {
>  	struct nvmet_ctrl *ctrl = req->sq->ctrl;
>  	struct nvmf_auth_dhchap_negotiate_data *data = d;
>  	int i, hash_id = 0, fallback_hash_id = 0, dhgid, fallback_dhgid;
>  
> +	if (tl < sizeof(*data) +
> +		 sizeof(struct nvmf_auth_dhchap_protocol_descriptor))

Odd formatting here.  Condition continuations either get two-tab indents
or after the brace:

	if (tl < sizeof(*data) +
			sizeof(struct nvmf_auth_dhchap_protocol_descriptor))

	if (tl <
	   sizeof(*data) + sizeof(struct nvmf_auth_dhchap_protocol_descriptor))

Otherwise looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH] nvmet: fix heap out-of-bounds read in nvmet_auth_negotiate()

[Guixin Liu]

在 2026/8/5 00:19, Christoph Hellwig 写道:
> On Tue, Aug 04, 2026 at 11:38:00AM +0800, Guixin Liu wrote:
>> nvmet_execute_auth_send() allocates the DH-HMAC-CHAP message buffer with
>> the host-supplied transfer length (tl) and hands it to
>> nvmet_auth_negotiate() without passing tl along. nvmet_auth_negotiate()
>> then reads the negotiate header and, for each of the halen hash
>> identifiers and dhlen DH group identifiers, indexes into the fixed
>> idlist[60] array (hashes at idlist[0..halen), groups at idlist[30..]).
>>
>> Neither the transfer length nor halen/dhlen is validated. A malicious or
>> non-conformant host can report a tl smaller than the negotiate structure,
>> or a halen/dhlen larger than the array (both are u8, up to 255), making
>> the loops read past the end of the allocated buffer (heap out-of-bounds
>> read). The sibling nvmet_auth_reply() already validates tl against the
>> structure size; the negotiate path did not.
>>
>> Pass tl into nvmet_auth_negotiate(), reject a tl that does not cover the
>> negotiate data plus one full protocol descriptor, and reject halen/dhlen
>> larger than NVME_AUTH_DHCHAP_MAX_DH_IDS.
>>
>> Fixes: db1312dd9548 ("nvmet: implement basic In-Band Authentication")
>> Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
>> ---
>>   drivers/nvme/target/fabrics-cmd-auth.c | 12 ++++++++++--
>>   1 file changed, 10 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/nvme/target/fabrics-cmd-auth.c b/drivers/nvme/target/fabrics-cmd-auth.c
>> index d1b39e64d877..d015968bae2d 100644
>> --- a/drivers/nvme/target/fabrics-cmd-auth.c
>> +++ b/drivers/nvme/target/fabrics-cmd-auth.c
>> @@ -31,12 +31,16 @@ void nvmet_auth_sq_init(struct nvmet_sq *sq)
>>   	sq->dhchap_step = NVME_AUTH_DHCHAP_MESSAGE_NEGOTIATE;
>>   }
>>   
>> -static u8 nvmet_auth_negotiate(struct nvmet_req *req, void *d)
>> +static u8 nvmet_auth_negotiate(struct nvmet_req *req, void *d, u32 tl)
>>   {
>>   	struct nvmet_ctrl *ctrl = req->sq->ctrl;
>>   	struct nvmf_auth_dhchap_negotiate_data *data = d;
>>   	int i, hash_id = 0, fallback_hash_id = 0, dhgid, fallback_dhgid;
>>   
>> +	if (tl < sizeof(*data) +
>> +		 sizeof(struct nvmf_auth_dhchap_protocol_descriptor))
> Odd formatting here.  Condition continuations either get two-tab indents
> or after the brace:
>
> 	if (tl < sizeof(*data) +
> 			sizeof(struct nvmf_auth_dhchap_protocol_descriptor))
>
> 	if (tl <
> 	   sizeof(*data) + sizeof(struct nvmf_auth_dhchap_protocol_descriptor))
Well, this is an oversight.

Hi Keith, could you please fix this when you apply?  Or I can send a v2 
patch.

Best Regards,
Guixin Liu
> Otherwise looks good:
>
> Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH] nvmet: fix heap out-of-bounds read in nvmet_auth_negotiate()

[Keith Busch]

On Tue, Aug 04, 2026 at 11:38:00AM +0800, Guixin Liu wrote:
> nvmet_execute_auth_send() allocates the DH-HMAC-CHAP message buffer with
> the host-supplied transfer length (tl) and hands it to
> nvmet_auth_negotiate() without passing tl along. nvmet_auth_negotiate()
> then reads the negotiate header and, for each of the halen hash
> identifiers and dhlen DH group identifiers, indexes into the fixed
> idlist[60] array (hashes at idlist[0..halen), groups at idlist[30..]).

Thanks, applied to nvme-7.3 with the formatting updated.