Re: [PATCH] nvmet: auth: validate dhchap id list lengths(KASAN: slab-out-of-bounds)

[yunje shin]

Thank you for the clarification regarding the 64-byte structural
constraints. If this approach looks good to you, I will format it
properly with an updated commit message and send out a formal v2
patch.

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,7 @@ 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

Thanks
Yunje Shin.

On Wed, Mar 11, 2026 at 3:07 AM Chris Leech <cleech@xxxxxxxxxx> wrote:
>
> On Wed, Mar 11, 2026 at 02:52:36AM +0900, yunje shin wrote:
> > Thanks for the review.
> >
> > Yes, I triggered the KASAN issue by injecting an invalid dhlen The
> > reproduction steps are:
> > 1. Connect to the NVMe/TCP target on port 4420 (ICReq + Fabrics CONNECT).
> > 2. Send AUTH_SEND with a crafted NEGOTIATE payload where dhlen=200. 3.
> > The kernel target code in nvmet_auth_negotiate() then iterates
> >
> >     for (i = 0; i < data->auth_protocol[0].dhchap.dhlen; i++) {
> >         int tmp_dhgid = data->auth_protocol[0].dhchap.idlist[i + 30];
> >
> > With dhlen=200, this reads idlist[30..229], but idlist[] is only 60
> > bytes (indices 0..59). The accesses at indices 60 and beyond read past
> > the kmalloc'd slab object into adjacent slab memory, which KASAN
> > catches as a slab-out-of-bounds read.
>
> Thank you, I appreciate understanding how this was triggered.
>
> > While the standard Linux NVMe host driver does use hardcoded halen=3
> > and dhlen=6, the NVMe target is network-facing and must validate all
> > fields from the wire. A malicious or non-standard host can send
> > arbitrary values. The same applies to halen — if halen > 30, the
> > first loop also reads out of bounds.
>
> Yes, this code absolutly should validate halen and dhlen bounds.
>
> > Regarding idlist_half — yes, idlist is currently a fixed 60-byte array
> > and the DH offset is always 30. I derived it from sizeof(idlist)
> > rather than hardcoding 30 so that the bounds check and the DH offset
> > stay consistent with the array definition. If the struct ever changes,
> > the validation adapts automatically instead of silently going stale.
>
> The 60-byte idlist (and 30:30 split) are part of the NVMe specification.
> It's the maximum amount of space while keeping to a 64-byte struct.
>
> I'd rather see this made clearer with a define for the limit, but not
> adding code that appears to calculate it at runtime.
>
> Thanks,
> - Chris
>