[PATCH v2] nvmet: auth: validate dhchap id list lengths

YunJe Shin posted 1 patch 3 weeks, 4 days ago
drivers/nvme/target/fabrics-cmd-auth.c | 11 ++++++++++-
include/linux/nvme.h                   |  2 ++
2 files changed, 12 insertions(+), 1 deletion(-)
[PATCH v2] nvmet: auth: validate dhchap id list lengths
Posted by YunJe Shin 3 weeks, 4 days ago
From: Yunje Shin <ioerts@kookmin.ac.kr>

The function nvmet_auth_negotiate() parses the idlist array in the
struct nvmf_auth_dhchap_protocol_descriptor payload. This array is 60
bytes and is logically divided into two 30-byte halves: the first half
for HMAC IDs and the second half for DH group IDs. The current code
uses a hardcoded +30 offset for the DH list, but does not validate
halen and dhlen against the per-half bounds. As a result, if a
malicious host sends halen or dhlen larger than 30, the loop can
read past the 60-byte array into adjacent slab memory, triggering a
KASAN slab-out-of-bounds read.

KASAN splat:
[    4.241646] BUG: KASAN: slab-out-of-bounds in nvmet_execute_auth_send+0x19b8/0x2090
[    4.242874] Read of size 1 at addr ffff8881045754e8 by task kworker/1:1H/41
[    4.265342] The buggy address belongs to the cache kmalloc-96 of size 96
[    4.266291]  allocated 72-byte region [ffff8881045754a0, ffff8881045754e8)
[    4.270337] page dumped because: kasan: bad access detected

This patch fixes the issue by introducing NVME_AUTH_DHCHAP_MAX_HASH_IDS
and NVME_AUTH_DHCHAP_MAX_DH_IDS defined as 30, which explicitly indicates
the maximum boundaries allowed per NVMe specification. The lengths halen
and dhlen are validated against these boundaries before processing,
preventing the out-of-bounds reads.

Fixes: db1312dd95488 ("nvmet: implement basic In-Band Authentication")
Cc: stable@kernel.org
Signed-off-by: Yunje Shin <ioerts@kookmin.ac.kr>
Reviewed-by: Hannes Reinecke <hare@suse.de>
---
v2:
    - Replaced the runtime 'sizeof' calculation (idlist_half) with explicit 
      NVME_AUTH_DHCHAP_MAX_HASH_IDS and NVME_AUTH_DHCHAP_MAX_DH_IDS macros
      to clearly reflect the 30:30 split limit per Chris Leech's feedback.

 drivers/nvme/target/fabrics-cmd-auth.c | 11 ++++++++++-
 include/linux/nvme.h                   |  2 ++
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/nvme/target/fabrics-cmd-auth.c b/drivers/nvme/target/fabrics-cmd-auth.c
index 5946681cb0e3..acba4878a873 100644
--- a/drivers/nvme/target/fabrics-cmd-auth.c
+++ b/drivers/nvme/target/fabrics-cmd-auth.c
@@ -72,6 +72,14 @@ static u8 nvmet_auth_negotiate(struct nvmet_req *req, void *d)
 	    NVME_AUTH_DHCHAP_AUTH_ID)
 		return NVME_AUTH_DHCHAP_FAILURE_INCORRECT_PAYLOAD;
 
+	/*
+	 * idlist[0..29]: hash IDs
+	 * idlist[30..59]: DH group IDs
+	 */
+	if (data->auth_protocol[0].dhchap.halen > NVME_AUTH_DHCHAP_MAX_HASH_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];
 
@@ -97,7 +105,8 @@ static u8 nvmet_auth_negotiate(struct nvmet_req *req, void *d)
 	dhgid = -1;
 	fallback_dhgid = -1;
 	for (i = 0; i < data->auth_protocol[0].dhchap.dhlen; i++) {
-		int tmp_dhgid = data->auth_protocol[0].dhchap.idlist[i + 30];
+		int tmp_dhgid =
+			data->auth_protocol[0].dhchap.idlist[i + NVME_AUTH_DHCHAP_MAX_HASH_IDS];
 
 		if (tmp_dhgid != ctrl->dh_gid) {
 			dhgid = tmp_dhgid;
diff --git a/include/linux/nvme.h b/include/linux/nvme.h
index b09dcaf5bcbc..ea0393ab16fc 100644
--- a/include/linux/nvme.h
+++ b/include/linux/nvme.h
@@ -1824,6 +1824,8 @@ struct nvmf_auth_dhchap_protocol_descriptor {
 	__u8		dhlen;
 	__u8		idlist[60];
 };
+#define NVME_AUTH_DHCHAP_MAX_HASH_IDS 30
+#define NVME_AUTH_DHCHAP_MAX_DH_IDS 30
 
 enum {
 	NVME_AUTH_DHCHAP_AUTH_ID	= 0x01,
-- 
2.43.0
Re: [PATCH v2] nvmet: auth: validate dhchap id list lengths
Posted by Christoph Hellwig 2 weeks, 6 days ago
On Fri, Mar 13, 2026 at 02:24:09PM +0900, YunJe Shin wrote:
> +	/*
> +	 * idlist[0..29]: hash IDs
> +	 * idlist[30..59]: DH group IDs
> +	 */
> +	if (data->auth_protocol[0].dhchap.halen > NVME_AUTH_DHCHAP_MAX_HASH_IDS ||
> +	    data->auth_protocol[0].dhchap.dhlen > NVME_AUTH_DHCHAP_MAX_DH_IDS)

Overly lone lines. A local variable for data->auth_protocol[0].dhchap
would really help with readability here.

> diff --git a/include/linux/nvme.h b/include/linux/nvme.h
> index b09dcaf5bcbc..ea0393ab16fc 100644
> --- a/include/linux/nvme.h
> +++ b/include/linux/nvme.h
> @@ -1824,6 +1824,8 @@ struct nvmf_auth_dhchap_protocol_descriptor {
>  	__u8		dhlen;
>  	__u8		idlist[60];
>  };
> +#define NVME_AUTH_DHCHAP_MAX_HASH_IDS 30
> +#define NVME_AUTH_DHCHAP_MAX_DH_IDS 30

Tabs before the values.  Bonus points for a reference to the relevant
spec.
Re: [PATCH v2] nvmet: auth: validate dhchap id list lengths
Posted by yunje shin 2 weeks, 6 days ago
On Tue, Mar 17, 2026 at 11:51 PM Christoph Hellwig <hch@lst.de> wrote:
> Overly lone lines. A local variable for data->auth_protocol[0].dhchap
> would really help with readability here.
...
> Tabs before the values.  Bonus points for a reference to the relevant
> spec.

I will fix the tab alignment for the macros and add a spec reference
in v3.

Regarding the local variable — I understand the readability concern,
but data->auth_protocol[0].dhchap is currently used without a local
variable in 8 places in target/fabrics-cmd-auth.c (lines 42-44, 71,
75-76, 100-101) and 12 places in host/auth.c (lines 145-156).
Adding one only for the bounds check felt inconsistent, so I kept
the existing style. Happy to hear your thoughts on this.
Re: [PATCH v2] nvmet: auth: validate dhchap id list lengths
Posted by Christoph Hellwig 2 weeks, 4 days ago
On Wed, Mar 18, 2026 at 01:55:13AM +0900, yunje shin wrote:
> Regarding the local variable — I understand the readability concern,
> but data->auth_protocol[0].dhchap is currently used without a local
> variable in 8 places in target/fabrics-cmd-auth.c (lines 42-44, 71,
> 75-76, 100-101) and 12 places in host/auth.c (lines 145-156).
> Adding one only for the bounds check felt inconsistent, so I kept
> the existing style. Happy to hear your thoughts on this.

Bonus points for fixing all of them up :)
Re: [PATCH v2] nvmet: auth: validate dhchap id list lengths
Posted by yunje shin 2 weeks, 4 days ago
On Tue, Mar 17, 2026 at 11:51 PM Christoph Hellwig <hch@lst.de> wrote:
>
> On Fri, Mar 13, 2026 at 02:24:09PM +0900, YunJe Shin wrote:
> > +#define NVME_AUTH_DHCHAP_MAX_HASH_IDS 30
> > +#define NVME_AUTH_DHCHAP_MAX_DH_IDS 30
>
> Tabs before the values.  Bonus points for a reference to the relevant
> spec.
>

 Thanks. Since Alistair has already sent the nvme.h macro patch with
the spec reference, I'll drop the header changes
  from my patch and send v3
Re: [PATCH v2] nvmet: auth: validate dhchap id list lengths
Posted by Chris Leech 3 weeks, 3 days ago
On Fri, Mar 13, 2026 at 02:24:09PM +0900, YunJe Shin wrote:
> From: Yunje Shin <ioerts@kookmin.ac.kr>
> 
> The function nvmet_auth_negotiate() parses the idlist array in the
> struct nvmf_auth_dhchap_protocol_descriptor payload. This array is 60
> bytes and is logically divided into two 30-byte halves: the first half
> for HMAC IDs and the second half for DH group IDs. The current code
> uses a hardcoded +30 offset for the DH list, but does not validate
> halen and dhlen against the per-half bounds. As a result, if a
> malicious host sends halen or dhlen larger than 30, the loop can
> read past the 60-byte array into adjacent slab memory, triggering a
> KASAN slab-out-of-bounds read.
> 
> KASAN splat:
> [    4.241646] BUG: KASAN: slab-out-of-bounds in nvmet_execute_auth_send+0x19b8/0x2090
> [    4.242874] Read of size 1 at addr ffff8881045754e8 by task kworker/1:1H/41
> [    4.265342] The buggy address belongs to the cache kmalloc-96 of size 96
> [    4.266291]  allocated 72-byte region [ffff8881045754a0, ffff8881045754e8)
> [    4.270337] page dumped because: kasan: bad access detected
> 
> This patch fixes the issue by introducing NVME_AUTH_DHCHAP_MAX_HASH_IDS
> and NVME_AUTH_DHCHAP_MAX_DH_IDS defined as 30, which explicitly indicates
> the maximum boundaries allowed per NVMe specification. The lengths halen
> and dhlen are validated against these boundaries before processing,
> preventing the out-of-bounds reads.
> 
> Fixes: db1312dd95488 ("nvmet: implement basic In-Band Authentication")
> Cc: stable@kernel.org
> Signed-off-by: Yunje Shin <ioerts@kookmin.ac.kr>
> Reviewed-by: Hannes Reinecke <hare@suse.de>
> ---
> v2:
>     - Replaced the runtime 'sizeof' calculation (idlist_half) with explicit 
>       NVME_AUTH_DHCHAP_MAX_HASH_IDS and NVME_AUTH_DHCHAP_MAX_DH_IDS macros
>       to clearly reflect the 30:30 split limit per Chris Leech's feedback.

Reviewed-by: Chris Leech <cleech@redhat.com>