public inbox for linux-nvme@lists.infradead.org
 help / color / mirror / Atom feed
* [PATCH] NVMe/TCP: Fixed the out of order H2C PDU Crash
       [not found] <BL3PR01MB684908447A2F3F7D2AFC57DAC8A3A@BL3PR01MB6849.prod.exchangelabs.com>
@ 2025-12-12 19:31 ` Shivam
  2025-12-12 20:11   ` Jens Axboe
  0 siblings, 1 reply; 8+ messages in thread
From: Shivam @ 2025-12-12 19:31 UTC (permalink / raw)
  To: linux-nvme; +Cc: sagi, kch, hch, kbusch, gregkh, Shivam

From: Shivam <skumar47@syr.edu>

Signed-off-by: Shivam Kumar <skumar47@syr.edu>
---
 drivers/nvme/target/tcp.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/nvme/target/tcp.c b/drivers/nvme/target/tcp.c
index 15416ff0eac4..cbd9286eabf4 100644
--- a/drivers/nvme/target/tcp.c
+++ b/drivers/nvme/target/tcp.c
@@ -982,6 +982,18 @@ static int nvmet_tcp_handle_h2c_data_pdu(struct nvmet_tcp_queue *queue)
 		pr_err("H2CData PDU len %u is invalid\n", cmd->pdu_len);
 		goto err_proto;
 	}
+	/*
+	* Ensure command data structures are initialized. we must check both
+	* cmd->req.sg and cmd->iov because they can have different NULL state:
+	*- Unintialized commands: both NULL
+	*- READ commands: cmd->req.sg allocated, cmd->iov NULL
+	*- WRITE commands: both allocated
+	*/
+	if (unlikely(!cmd->req.sg || !cmd->iov)) {
+		pr_err("queue %d: H2CData PDU received for invalid command state (ttag %u)\n",
+			queue->idx, data->ttag);
+		goto err_proto;
+	}
 	cmd->pdu_recv = 0;
 	nvmet_tcp_build_pdu_iovec(cmd);
 	queue->cmd = cmd;
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] NVMe/TCP: Fixed the out of order H2C PDU Crash
  2025-12-12 19:31 ` [PATCH] NVMe/TCP: Fixed the out of order H2C PDU Crash Shivam
@ 2025-12-12 20:11   ` Jens Axboe
  2025-12-12 21:08     ` [PATCH v2] nvme-tcp: fix NULL pointer dereferences in nvmet_tcp_build_pdu_iovec Shivam
  2025-12-13 18:57     ` [PATCH v3] " Shivam Kumar
  0 siblings, 2 replies; 8+ messages in thread
From: Jens Axboe @ 2025-12-12 20:11 UTC (permalink / raw)
  To: Shivam, linux-nvme; +Cc: sagi, kch, hch, kbusch, gregkh, Shivam

On 12/12/25 12:31 PM, Shivam wrote:
> From: Shivam <skumar47@syr.edu>
> 
> Signed-off-by: Shivam Kumar <skumar47@syr.edu>

You forgot to write an actual commit message.

-- 
Jens Axboe



^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2] nvme-tcp: fix NULL pointer dereferences in nvmet_tcp_build_pdu_iovec
  2025-12-12 20:11   ` Jens Axboe
@ 2025-12-12 21:08     ` Shivam
  2025-12-12 21:15       ` Jens Axboe
                         ` (2 more replies)
  2025-12-13 18:57     ` [PATCH v3] " Shivam Kumar
  1 sibling, 3 replies; 8+ messages in thread
From: Shivam @ 2025-12-12 21:08 UTC (permalink / raw)
  To: linux-nvme; +Cc: axboe, sagi, kch, hch, kbusch, gregkh, Shivam

From: Shivam <skumar47@syr.edu>

The CVE-2023-6356 patch added ttag bounds checking and data_offset
validation in nvmet_tcp_handle_h2c_data_pdu(), but it did not validate
whether the command's data structures (cmd->req.sg and cmd->iov) have
been properly initialized before processing H2C_DATA PDUs.

The nvmet_tcp_build_pdu_iovec() function dereferences these pointers
without NULL checks. This can be triggered by sending an H2C_DATA PDU
immediately after the ICREQ/ICRESP handshake, before sending a CONNECT
command or NVMe write command.

Attack vectors that trigger NULL pointer dereferences:
1. H2C_DATA PDU sent before CONNECT → both pointers NULL
2. H2C_DATA PDU for READ command → cmd->req.sg allocated, cmd->iov NULL
3. H2C_DATA PDU for uninitialized command slot → both pointers NULL

The fix validates both cmd->req.sg and cmd->iov before calling
nvmet_tcp_build_pdu_iovec(). Both checks are required because:
- Uninitialized commands: both NULL
- READ commands: cmd->req.sg allocated, cmd->iov NULL
- WRITE commands: both allocated

Reported-by: Shivam Kumar <skumar47@syr.edu>
Signed-off-by: Shivam Kumar <skumar47@syr.edu>
---
 drivers/nvme/target/tcp.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/nvme/target/tcp.c b/drivers/nvme/target/tcp.c
index 15416ff0eac4..d5966d007ba3 100644
--- a/drivers/nvme/target/tcp.c
+++ b/drivers/nvme/target/tcp.c
@@ -982,6 +982,18 @@ static int nvmet_tcp_handle_h2c_data_pdu(struct nvmet_tcp_queue *queue)
 		pr_err("H2CData PDU len %u is invalid\n", cmd->pdu_len);
 		goto err_proto;
 	}
+       /*
+	* Ensure command data structures are initialized. We must check both
+	* cmd->req.sg and cmd->iov because they can have different NULL states:
+	* - Uninitialized commands: both NULL
+	* - READ commands: cmd->req.sg allocated, cmd->iov NULL
+	* - WRITE commands: both allocated
+	*/
+	if (unlikely(!cmd->req.sg || !cmd->iov)) {
+		pr_err("queue %d: H2CData PDU received for invalid command state (ttag %u)\n",
+			queue->idx, data->ttag);
+		goto err_proto;
+	}
 	cmd->pdu_recv = 0;
 	nvmet_tcp_build_pdu_iovec(cmd);
 	queue->cmd = cmd;
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] nvme-tcp: fix NULL pointer dereferences in nvmet_tcp_build_pdu_iovec
  2025-12-12 21:08     ` [PATCH v2] nvme-tcp: fix NULL pointer dereferences in nvmet_tcp_build_pdu_iovec Shivam
@ 2025-12-12 21:15       ` Jens Axboe
  2025-12-13 13:13       ` Sagi Grimberg
  2025-12-13 13:22       ` Greg KH
  2 siblings, 0 replies; 8+ messages in thread
From: Jens Axboe @ 2025-12-12 21:15 UTC (permalink / raw)
  To: Shivam, linux-nvme; +Cc: sagi, kch, hch, kbusch, gregkh, Shivam

On 12/12/25 2:08 PM, Shivam wrote:
> From: Shivam <skumar47@syr.edu>
> 
> The CVE-2023-6356 patch added ttag bounds checking and data_offset

Please just replace that with an actual sha, and add it as a Fixes
tag too. No need to resend for that, I'm assuming whoever applies
this can sort that out. Some random CVE number doesn't mean anything
to me, a sha is something that is easily identifiable.

Rest of commit message looks good.

> validation in nvmet_tcp_handle_h2c_data_pdu(), but it did not validate
> whether the command's data structures (cmd->req.sg and cmd->iov) have
> been properly initialized before processing H2C_DATA PDUs.
> 
> The nvmet_tcp_build_pdu_iovec() function dereferences these pointers
> without NULL checks. This can be triggered by sending an H2C_DATA PDU
> immediately after the ICREQ/ICRESP handshake, before sending a CONNECT
> command or NVMe write command.
> 
> Attack vectors that trigger NULL pointer dereferences:
> 1. H2C_DATA PDU sent before CONNECT → both pointers NULL
> 2. H2C_DATA PDU for READ command → cmd->req.sg allocated, cmd->iov NULL
> 3. H2C_DATA PDU for uninitialized command slot → both pointers NULL
> 
> The fix validates both cmd->req.sg and cmd->iov before calling
> nvmet_tcp_build_pdu_iovec(). Both checks are required because:
> - Uninitialized commands: both NULL
> - READ commands: cmd->req.sg allocated, cmd->iov NULL
> - WRITE commands: both allocated
> 
> Reported-by: Shivam Kumar <skumar47@syr.edu>
> Signed-off-by: Shivam Kumar <skumar47@syr.edu>

No need for the Reported-by tag, you already have your SOB there.

-- 
Jens Axboe



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] nvme-tcp: fix NULL pointer dereferences in nvmet_tcp_build_pdu_iovec
  2025-12-12 21:08     ` [PATCH v2] nvme-tcp: fix NULL pointer dereferences in nvmet_tcp_build_pdu_iovec Shivam
  2025-12-12 21:15       ` Jens Axboe
@ 2025-12-13 13:13       ` Sagi Grimberg
  2025-12-13 13:22       ` Greg KH
  2 siblings, 0 replies; 8+ messages in thread
From: Sagi Grimberg @ 2025-12-13 13:13 UTC (permalink / raw)
  To: Shivam, linux-nvme; +Cc: axboe, kch, hch, kbusch, gregkh, Shivam



On 12/12/2025 23:08, Shivam wrote:
> From: Shivam <skumar47@syr.edu>
>
> The CVE-2023-6356 patch

Can you provide the commit hash here?

>   added ttag bounds checking and data_offset
> validation in nvmet_tcp_handle_h2c_data_pdu(), but it did not validate
> whether the command's data structures (cmd->req.sg and cmd->iov) have
> been properly initialized before processing H2C_DATA PDUs.
>
> The nvmet_tcp_build_pdu_iovec() function dereferences these pointers
> without NULL checks. This can be triggered by sending an H2C_DATA PDU
> immediately after the ICREQ/ICRESP handshake, before sending a CONNECT
> command or NVMe write command.
>
> Attack vectors that trigger NULL pointer dereferences:
> 1. H2C_DATA PDU sent before CONNECT → both pointers NULL
> 2. H2C_DATA PDU for READ command → cmd->req.sg allocated, cmd->iov NULL
> 3. H2C_DATA PDU for uninitialized command slot → both pointers NULL
>
> The fix validates both cmd->req.sg and cmd->iov before calling
> nvmet_tcp_build_pdu_iovec(). Both checks are required because:
> - Uninitialized commands: both NULL
> - READ commands: cmd->req.sg allocated, cmd->iov NULL
> - WRITE commands: both allocated

Needs a Fixes tag.

Other than that, you can add my:
Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
on your v3.


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2] nvme-tcp: fix NULL pointer dereferences in nvmet_tcp_build_pdu_iovec
  2025-12-12 21:08     ` [PATCH v2] nvme-tcp: fix NULL pointer dereferences in nvmet_tcp_build_pdu_iovec Shivam
  2025-12-12 21:15       ` Jens Axboe
  2025-12-13 13:13       ` Sagi Grimberg
@ 2025-12-13 13:22       ` Greg KH
  2 siblings, 0 replies; 8+ messages in thread
From: Greg KH @ 2025-12-13 13:22 UTC (permalink / raw)
  To: Shivam; +Cc: linux-nvme, axboe, sagi, kch, hch, kbusch, Shivam

On Fri, Dec 12, 2025 at 04:08:50PM -0500, Shivam wrote:
> From: Shivam <skumar47@syr.edu>

Does not match your signed-off-by line :(


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v3] nvme-tcp: fix NULL pointer dereferences in nvmet_tcp_build_pdu_iovec
  2025-12-12 20:11   ` Jens Axboe
  2025-12-12 21:08     ` [PATCH v2] nvme-tcp: fix NULL pointer dereferences in nvmet_tcp_build_pdu_iovec Shivam
@ 2025-12-13 18:57     ` Shivam Kumar
  2026-01-09 14:54       ` Keith Busch
  1 sibling, 1 reply; 8+ messages in thread
From: Shivam Kumar @ 2025-12-13 18:57 UTC (permalink / raw)
  To: linux-nvme; +Cc: axboe, sagi, kch, hch, kbusch, gregkh, Shivam Kumar

Commit efa56305908b ("nvmet-tcp: Fix a kernel panic when host sends an invalid H2C PDU length")
added ttag bounds checking and data_offset
validation in nvmet_tcp_handle_h2c_data_pdu(), but it did not validate
whether the command's data structures (cmd->req.sg and cmd->iov) have
been properly initialized before processing H2C_DATA PDUs.

The nvmet_tcp_build_pdu_iovec() function dereferences these pointers
without NULL checks. This can be triggered by sending H2C_DATA PDU
immediately after the ICREQ/ICRESP handshake, before
sending a CONNECT command or NVMe write command.

Attack vectors that trigger NULL pointer dereferences:
1. H2C_DATA PDU sent before CONNECT → both pointers NULL
2. H2C_DATA PDU for READ command → cmd->req.sg allocated, cmd->iov NULL
3. H2C_DATA PDU for uninitialized command slot → both pointers NULL

The fix validates both cmd->req.sg and cmd->iov before calling
nvmet_tcp_build_pdu_iovec(). Both checks are required because:
- Uninitialized commands: both NULL
- READ commands: cmd->req.sg allocated, cmd->iov NULL
- WRITE commands: both allocated

Fixes: efa56305908b ("nvmet-tcp: Fix a kernel panic when host sends an invalid H2C PDU length")
Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
Signed-off-by: Shivam Kumar <kumar.shivam43666@gmail.com>
---
 drivers/nvme/target/tcp.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/nvme/target/tcp.c b/drivers/nvme/target/tcp.c
index 15416ff0eac4..d5966d007ba3 100644
--- a/drivers/nvme/target/tcp.c
+++ b/drivers/nvme/target/tcp.c
@@ -982,6 +982,18 @@ static int nvmet_tcp_handle_h2c_data_pdu(struct nvmet_tcp_queue *queue)
 		pr_err("H2CData PDU len %u is invalid\n", cmd->pdu_len);
 		goto err_proto;
 	}
+       /*
+	* Ensure command data structures are initialized. We must check both
+	* cmd->req.sg and cmd->iov because they can have different NULL states:
+	* - Uninitialized commands: both NULL
+	* - READ commands: cmd->req.sg allocated, cmd->iov NULL
+	* - WRITE commands: both allocated
+	*/
+	if (unlikely(!cmd->req.sg || !cmd->iov)) {
+		pr_err("queue %d: H2CData PDU received for invalid command state (ttag %u)\n",
+			queue->idx, data->ttag);
+		goto err_proto;
+	}
 	cmd->pdu_recv = 0;
 	nvmet_tcp_build_pdu_iovec(cmd);
 	queue->cmd = cmd;
-- 
2.34.1



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v3] nvme-tcp: fix NULL pointer dereferences in nvmet_tcp_build_pdu_iovec
  2025-12-13 18:57     ` [PATCH v3] " Shivam Kumar
@ 2026-01-09 14:54       ` Keith Busch
  0 siblings, 0 replies; 8+ messages in thread
From: Keith Busch @ 2026-01-09 14:54 UTC (permalink / raw)
  To: Shivam Kumar; +Cc: linux-nvme, axboe, sagi, kch, hch, gregkh

On Sat, Dec 13, 2025 at 01:57:48PM -0500, Shivam Kumar wrote:
> Commit efa56305908b ("nvmet-tcp: Fix a kernel panic when host sends an invalid H2C PDU length")
> added ttag bounds checking and data_offset
> validation in nvmet_tcp_handle_h2c_data_pdu(), but it did not validate
> whether the command's data structures (cmd->req.sg and cmd->iov) have
> been properly initialized before processing H2C_DATA PDUs.
> 
> The nvmet_tcp_build_pdu_iovec() function dereferences these pointers
> without NULL checks. This can be triggered by sending H2C_DATA PDU
> immediately after the ICREQ/ICRESP handshake, before
> sending a CONNECT command or NVMe write command.
> 
> Attack vectors that trigger NULL pointer dereferences:
> 1. H2C_DATA PDU sent before CONNECT -> both pointers NULL
> 2. H2C_DATA PDU for READ command -> cmd->req.sg allocated, cmd->iov NULL
> 3. H2C_DATA PDU for uninitialized command slot -> both pointers NULL
> 
> The fix validates both cmd->req.sg and cmd->iov before calling
> nvmet_tcp_build_pdu_iovec(). Both checks are required because:
> - Uninitialized commands: both NULL
> - READ commands: cmd->req.sg allocated, cmd->iov NULL
> - WRITE commands: both allocated

Thanks, applied.


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2026-01-09 14:55 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <BL3PR01MB684908447A2F3F7D2AFC57DAC8A3A@BL3PR01MB6849.prod.exchangelabs.com>
2025-12-12 19:31 ` [PATCH] NVMe/TCP: Fixed the out of order H2C PDU Crash Shivam
2025-12-12 20:11   ` Jens Axboe
2025-12-12 21:08     ` [PATCH v2] nvme-tcp: fix NULL pointer dereferences in nvmet_tcp_build_pdu_iovec Shivam
2025-12-12 21:15       ` Jens Axboe
2025-12-13 13:13       ` Sagi Grimberg
2025-12-13 13:22       ` Greg KH
2025-12-13 18:57     ` [PATCH v3] " Shivam Kumar
2026-01-09 14:54       ` Keith Busch

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox