nvmet-auth: validate reply message payload bounds against transfer length

[PATCH] nvmet-auth: validate reply message payload bounds against transfer length

Posted by Tianchu Chen 1 week, 2 days ago

From: Tianchu Chen <flynnnchen@tencent.com>

nvmet_auth_reply() accesses the variable-length rval[] array using
attacker-controlled hl (hash length) and dhvlen (DH value length) fields
without verifying they fit within the allocated buffer of tl bytes.

A malicious NVMe-oF initiator can craft a DHCHAP_REPLY message with a
small transfer length but large hl/dhvlen values, causing out-of-bounds
heap reads when the target processes the DH public key (rval + 2*hl) or
performs the host response memcmp.

With DH authentication configured, the OOB pointer is passed directly to
sg_init_one() and read by crypto_kpp_compute_shared_secret(), reaching
up to 526 bytes past the buffer. This is exploitable pre-authentication.

Add bounds validation ensuring sizeof(*data) + 2*hl + dhvlen <= tl before
any access to the variable-length fields.

Discovered by Atuin - Automated Vulnerability Discovery Engine.

Fixes: db1312dd9548 ("nvmet: implement basic In-Band Authentication")
Cc: stable@vger.kernel.org
Signed-off-by: Tianchu Chen <flynnnchen@tencent.com>
---
 drivers/nvme/target/fabrics-cmd-auth.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/nvme/target/fabrics-cmd-auth.c b/drivers/nvme/target/fabrics-cmd-auth.c
index f1e613e7c..0a85acf1e 100644
--- a/drivers/nvme/target/fabrics-cmd-auth.c
+++ b/drivers/nvme/target/fabrics-cmd-auth.c
@@ -132,13 +132,22 @@ static u8 nvmet_auth_negotiate(struct nvmet_req *req, void *d)
 	return 0;
 }
 
-static u8 nvmet_auth_reply(struct nvmet_req *req, void *d)
+static u8 nvmet_auth_reply(struct nvmet_req *req, void *d, u32 tl)
 {
 	struct nvmet_ctrl *ctrl = req->sq->ctrl;
 	struct nvmf_auth_dhchap_reply_data *data = d;
-	u16 dhvlen = le16_to_cpu(data->dhvlen);
+	u16 dhvlen;
 	u8 *response;
 
+	if (tl < sizeof(*data))
+		return NVME_AUTH_DHCHAP_FAILURE_INCORRECT_PAYLOAD;
+
+	dhvlen = le16_to_cpu(data->dhvlen);
+
+	/* Validate that hl and dhvlen fit within the transfer length */
+	if (sizeof(*data) + 2 * (size_t)data->hl + dhvlen > tl)
+		return NVME_AUTH_DHCHAP_FAILURE_INCORRECT_PAYLOAD;
+
 	pr_debug("%s: ctrl %d qid %d: data hl %d cvalid %d dhvlen %u\n",
 		 __func__, ctrl->cntlid, req->sq->qid,
 		 data->hl, data->cvalid, dhvlen);
@@ -338,7 +347,7 @@ void nvmet_execute_auth_send(struct nvmet_req *req)
 
 	switch (data->auth_id) {
 	case NVME_AUTH_DHCHAP_MESSAGE_REPLY:
-		dhchap_status = nvmet_auth_reply(req, d);
+		dhchap_status = nvmet_auth_reply(req, d, tl);
 		if (dhchap_status == 0)
 			req->sq->dhchap_step =
 				NVME_AUTH_DHCHAP_MESSAGE_SUCCESS1;
-- 
2.51.0

Re: [PATCH] nvmet-auth: validate reply message payload bounds against transfer length

Posted by Keith Busch 5 days ago

On Fri, May 29, 2026 at 02:18:39PM +0000, Tianchu Chen wrote:
> From: Tianchu Chen <flynnnchen@tencent.com>
> 
> nvmet_auth_reply() accesses the variable-length rval[] array using
> attacker-controlled hl (hash length) and dhvlen (DH value length) fields
> without verifying they fit within the allocated buffer of tl bytes.

Thanks, applied to nvme-7.2.

Re: [PATCH] nvmet-auth: validate reply message payload bounds against transfer length

Posted by Christoph Hellwig 1 week ago

On Fri, May 29, 2026 at 02:18:39PM +0000, Tianchu Chen wrote:
> From: Tianchu Chen <flynnnchen@tencent.com>
> 
> nvmet_auth_reply() accesses the variable-length rval[] array using
> attacker-controlled hl (hash length) and dhvlen (DH value length) fields
> without verifying they fit within the allocated buffer of tl bytes.
> 
> A malicious NVMe-oF initiator can craft a DHCHAP_REPLY message with a
> small transfer length but large hl/dhvlen values, causing out-of-bounds
> heap reads when the target processes the DH public key (rval + 2*hl) or
> performs the host response memcmp.
> 
> With DH authentication configured, the OOB pointer is passed directly to
> sg_init_one() and read by crypto_kpp_compute_shared_secret(), reaching
> up to 526 bytes past the buffer. This is exploitable pre-authentication.
> 
> Add bounds validation ensuring sizeof(*data) + 2*hl + dhvlen <= tl before
> any access to the variable-length fields.
> 
> Discovered by Atuin - Automated Vulnerability Discovery Engine.
> 
> Fixes: db1312dd9548 ("nvmet: implement basic In-Band Authentication")
> Cc: stable@vger.kernel.org
> Signed-off-by: Tianchu Chen <flynnnchen@tencent.com>
> ---
>  drivers/nvme/target/fabrics-cmd-auth.c | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/nvme/target/fabrics-cmd-auth.c b/drivers/nvme/target/fabrics-cmd-auth.c
> index f1e613e7c..0a85acf1e 100644
> --- a/drivers/nvme/target/fabrics-cmd-auth.c
> +++ b/drivers/nvme/target/fabrics-cmd-auth.c
> @@ -132,13 +132,22 @@ static u8 nvmet_auth_negotiate(struct nvmet_req *req, void *d)
>  	return 0;
>  }
>  
> -static u8 nvmet_auth_reply(struct nvmet_req *req, void *d)
> +static u8 nvmet_auth_reply(struct nvmet_req *req, void *d, u32 tl)
>  {
>  	struct nvmet_ctrl *ctrl = req->sq->ctrl;
>  	struct nvmf_auth_dhchap_reply_data *data = d;
> -	u16 dhvlen = le16_to_cpu(data->dhvlen);
> +	u16 dhvlen;
>  	u8 *response;
>  
> +	if (tl < sizeof(*data))
> +		return NVME_AUTH_DHCHAP_FAILURE_INCORRECT_PAYLOAD;
> +
> +	dhvlen = le16_to_cpu(data->dhvlen);
> +
> +	/* Validate that hl and dhvlen fit within the transfer length */
> +	if (sizeof(*data) + 2 * (size_t)data->hl + dhvlen > tl)

Can't still still overflow?  This should probably use struct_size
to get the size up to and including the rval array, then
use use checked subtractions from tl.

Re: [PATCH] nvmet-auth: validate reply message payload bounds against transfer length

Posted by Tianchu Chen 1 week ago

June 1, 2026 at 3:19 PM, "Christoph Hellwig" <hch@lst.de mailto:hch@lst.de?to=%22Christoph%20Hellwig%22%20%3Chch%40lst.de%3E > wrote:


> 
> On Fri, May 29, 2026 at 02:18:39PM +0000, Tianchu Chen wrote:
> 
> > 
> > From: Tianchu Chen <flynnnchen@tencent.com>
> >  
> >  nvmet_auth_reply() accesses the variable-length rval[] array using
> >  attacker-controlled hl (hash length) and dhvlen (DH value length) fields
> >  without verifying they fit within the allocated buffer of tl bytes.
> >  
> >  A malicious NVMe-oF initiator can craft a DHCHAP_REPLY message with a
> >  small transfer length but large hl/dhvlen values, causing out-of-bounds
> >  heap reads when the target processes the DH public key (rval + 2*hl) or
> >  performs the host response memcmp.
> >  
> >  With DH authentication configured, the OOB pointer is passed directly to
> >  sg_init_one() and read by crypto_kpp_compute_shared_secret(), reaching
> >  up to 526 bytes past the buffer. This is exploitable pre-authentication.
> >  
> >  Add bounds validation ensuring sizeof(*data) + 2*hl + dhvlen <= tl before
> >  any access to the variable-length fields.
> >  
> >  Discovered by Atuin - Automated Vulnerability Discovery Engine.
> >  
> >  Fixes: db1312dd9548 ("nvmet: implement basic In-Band Authentication")
> >  Cc: stable@vger.kernel.org
> >  Signed-off-by: Tianchu Chen <flynnnchen@tencent.com>
> >  ---
> >  drivers/nvme/target/fabrics-cmd-auth.c | 15 ++++++++++++---
> >  1 file changed, 12 insertions(+), 3 deletions(-)
> >  
> >  diff --git a/drivers/nvme/target/fabrics-cmd-auth.c b/drivers/nvme/target/fabrics-cmd-auth.c
> >  index f1e613e7c..0a85acf1e 100644
> >  --- a/drivers/nvme/target/fabrics-cmd-auth.c
> >  +++ b/drivers/nvme/target/fabrics-cmd-auth.c
> >  @@ -132,13 +132,22 @@ static u8 nvmet_auth_negotiate(struct nvmet_req *req, void *d)
> >  return 0;
> >  }
> >  
> >  -static u8 nvmet_auth_reply(struct nvmet_req *req, void *d)
> >  +static u8 nvmet_auth_reply(struct nvmet_req *req, void *d, u32 tl)
> >  {
> >  struct nvmet_ctrl *ctrl = req->sq->ctrl;
> >  struct nvmf_auth_dhchap_reply_data *data = d;
> >  - u16 dhvlen = le16_to_cpu(data->dhvlen);
> >  + u16 dhvlen;
> >  u8 *response;
> >  
> >  + if (tl < sizeof(*data))
> >  + return NVME_AUTH_DHCHAP_FAILURE_INCORRECT_PAYLOAD;
> >  +
> >  + dhvlen = le16_to_cpu(data->dhvlen);
> >  +
> >  + /* Validate that hl and dhvlen fit within the transfer length */
> >  + if (sizeof(*data) + 2 * (size_t)data->hl + dhvlen > tl)
> > 
> Can't still still overflow? This should probably use struct_size
> to get the size up to and including the rval array, then
> use use checked subtractions from tl.
>

Thanks for the review.

I think the current patch doesn't actually allows overflow.

hl is a __u8 and dhvlen is a __le16. So the maximum value of
sizeof(*data) + 2 * (size_t)data->hl + dhvlen
is 66061, which is far below SIZE_MAX. It can't wrap.

About rewritting using struct_size: we still need to check
tl >= struct_size(data, rval, 0) first, which is exactly the same
as just using sizeof(*data) in current patch. Because we have to
confirm the header is fully present before we can even read
hl / dhvlen, which are the values that determine the minimum
required rval[] length.

So I believe the current patch already achieves the intended
goal of validating the payload against the buffer size,
using struct_size won't change the behavior.

That said, I'm happy to send a v2 that uses struct_size(data, rval, 0)
as the header anchor and then peels off the variable parts 
with checked subtractions, if you prefer that style:

size_t size = struct_size(data, rval, 0); /* check the header is fully present */
if (tl < size)
	return NVME_AUTH_DHCHAP_FAILURE_INCORRECT_PAYLOAD;
tl -= size;
/* rval[] carries: response (hl) + challenge (hl) + DH value (dhvlen) */
if (tl < 2 * (u32)data->hl)
	return NVME_AUTH_DHCHAP_FAILURE_INCORRECT_PAYLOAD;
tl -= 2 * (u32)data->hl;
dhvlen = le16_to_cpu(data->dhvlen);
if (tl < dhvlen)
	return NVME_AUTH_DHCHAP_FAILURE_INCORRECT_PAYLOAD;

Which one do you prefer? Just let me know which you'd prefer and I can send 
a v2 if needed.


Best regards,
Tianchu

Re: [PATCH] nvmet-auth: validate reply message payload bounds against transfer length

Posted by Christoph Hellwig 6 days, 18 hours ago

On Mon, Jun 01, 2026 at 08:58:18AM +0000, Tianchu Chen wrote:
> I think the current patch doesn't actually allows overflow.
> 
> hl is a __u8 and dhvlen is a __le16. So the maximum value of
> sizeof(*data) + 2 * (size_t)data->hl + dhvlen
> is 66061, which is far below SIZE_MAX. It can't wrap.

True.

> 
> About rewritting using struct_size: we still need to check
> tl >= struct_size(data, rval, 0) first,

Yes, or the existing sizeof.

> which is exactly the same
> as just using sizeof(*data) in current patch. Because we have to
> confirm the header is fully present before we can even read
> hl / dhvlen, which are the values that determine the minimum
> required rval[] length.
> 
> So I believe the current patch already achieves the intended
> goal of validating the payload against the buffer size,
> using struct_size won't change the behavior.

I find untangling magic numbers a bit hard.  Now unlike more
conventional cases we're stuck with some of them, but trying
to make the expression as self-describing as possible would
still be nice.

> That said, I'm happy to send a v2 that uses struct_size(data, rval, 0)

For the anchor that's not all that useful.

Anyway, let's keep the original version for now.  I noticed there
is more of that 2 * magic in surrounding code, so we'll need to
address that separately instead of burdening it on you.

Re: [PATCH] nvmet-auth: validate reply message payload bounds against transfer length

Posted by Hannes Reinecke 1 week ago

On 5/29/26 16:18, Tianchu Chen wrote:
> From: Tianchu Chen <flynnnchen@tencent.com>
> 
> nvmet_auth_reply() accesses the variable-length rval[] array using
> attacker-controlled hl (hash length) and dhvlen (DH value length) fields
> without verifying they fit within the allocated buffer of tl bytes.
> 
> A malicious NVMe-oF initiator can craft a DHCHAP_REPLY message with a
> small transfer length but large hl/dhvlen values, causing out-of-bounds
> heap reads when the target processes the DH public key (rval + 2*hl) or
> performs the host response memcmp.
> 
> With DH authentication configured, the OOB pointer is passed directly to
> sg_init_one() and read by crypto_kpp_compute_shared_secret(), reaching
> up to 526 bytes past the buffer. This is exploitable pre-authentication.
> 
> Add bounds validation ensuring sizeof(*data) + 2*hl + dhvlen <= tl before
> any access to the variable-length fields.
> 
> Discovered by Atuin - Automated Vulnerability Discovery Engine.
> 
> Fixes: db1312dd9548 ("nvmet: implement basic In-Band Authentication")
> Cc: stable@vger.kernel.org
> Signed-off-by: Tianchu Chen <flynnnchen@tencent.com>
> ---
>   drivers/nvme/target/fabrics-cmd-auth.c | 15 ++++++++++++---
>   1 file changed, 12 insertions(+), 3 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