[PATCH v2] scsi: target: iscsi: Reject CDB size exceeding available buffer in iscsit_setup_scsi_cmd

[PATCH v2] scsi: target: iscsi: Reject CDB size exceeding available buffer in iscsit_setup_scsi_cmd

[ghuicao]

From: Cao Guanghui <caoguanghui@kylinos.cn>

In iscsit_setup_scsi_cmd(), the CDB is later re-parsed by
scsi_command_size() based on its SCSI opcode. For a VARIABLE_LENGTH_CMD
(0x7f) the returned size is cdb[7] + 8, where both cdb[0] and cdb[7]
come verbatim from the initiator-controlled PDU. The amount of CDB data
actually available is never cross-checked against this opcode-declared
length before the CDB is handed to target_cmd_init_cdb(), which does:

  memcpy(cmd->t_task_cdb, cdb, scsi_command_size(cdb));

An initiator can set cdb[0]=0x7f and cdb[7]=252 so that
scsi_command_size() returns 260, while the available CDB space is only
16 bytes (the basic header, when no Extended CDB AHS is present) or the
AHS-provided length. target_cmd_init_cdb() then reads up to 244 bytes
past the end of the CDB buffer, a heap out-of-bounds read. The leaked
bytes are later parsed as the CDB and can be indirectly observed by the
initiator through sense data and responses.

Reject the command when the opcode-declared CDB size exceeds the
available CDB space, before the CDB is passed on:

- Without an Extended CDB AHS the CDB is limited to ISCSI_CDB_SIZE (16)
  bytes in the basic header, so reject when scsi_command_size() >
  ISCSI_CDB_SIZE.
- With an Extended CDB AHS the buffer is allocated from the AHS-declared
  length, so reject when scsi_command_size() > cdb_length.

commit 2f3835771dff ("scsi: target: iscsi: reject invalid size Extended
CDB AHS") fixed the zero-length ahslength overflows and the AHS-buffer
overread, but did not cover the orthogonal "available CDB space vs.
opcode-declared size" path.

Fixes: 8f1f7d297bce ("scsi: target: iscsi: Add support for extended CDB AHS")
Signed-off-by: Cao Guanghui <caoguanghui@kylinos.cn>
---
Hi Martin,

Thanks for the review (via Sashiko). You are right that the original patch
only covered the Extended CDB AHS path and missed the standard command
path without an AHS, where the same out-of-bounds read is reachable. v2
adds the same bounds check for hdr->hlength == 0 and switches the two
paths to if/else for clarity.

v2:
  - Add the bounds check for the standard path without an AHS
    (hdr->hlength == 0) as well, fixing the same out-of-bounds read
    that was previously only covered for the Extended CDB AHS path.
  - Use if/else to make the two (mutually exclusive) paths explicit.

 drivers/target/iscsi/iscsi_target.c | 31 +++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c
index 62ada3a52210..56f8fd461192 100644
--- a/drivers/target/iscsi/iscsi_target.c
+++ b/drivers/target/iscsi/iscsi_target.c
@@ -1100,7 +1100,22 @@ int iscsit_setup_scsi_cmd(struct iscsit_conn *conn, struct iscsit_cmd *cmd,
 
 	cdb = hdr->cdb;
 
-	if (hdr->hlength) {
+	if (!hdr->hlength) {
+		/*
+		 * Without an Extended CDB AHS the CDB is limited to the 16
+		 * bytes in the basic header. The CDB is later re-parsed by
+		 * scsi_command_size() based on its opcode, which may claim a
+		 * larger length (e.g. VARIABLE_LENGTH_CMD with cdb[7]=252).
+		 * Reject such a mismatch before handing the CDB to
+		 * target_cmd_init_cdb() to avoid an out-of-bounds read.
+		 */
+		if (scsi_command_size(hdr->cdb) > ISCSI_CDB_SIZE) {
+			pr_err("SCSI command size %u exceeds CDB size %u, protocol error.\n",
+			       scsi_command_size(hdr->cdb), ISCSI_CDB_SIZE);
+			return iscsit_add_reject_cmd(cmd,
+				ISCSI_REASON_PROTOCOL_ERROR, buf);
+		}
+	} else {
 		ecdb_ahdr = (struct iscsi_ecdb_ahdr *) (hdr + 1);
 		if (ecdb_ahdr->ahstype != ISCSI_AHSTYPE_CDB) {
 			pr_err("Additional Header Segment type %d not supported!\n",
@@ -1124,6 +1139,20 @@ int iscsit_setup_scsi_cmd(struct iscsit_conn *conn, struct iscsit_cmd *cmd,
 
 		cdb_length = ahslength - 1 + ISCSI_CDB_SIZE;
 
+		/*
+		 * The CDB buffer is later re-parsed by scsi_command_size()
+		 * based on its opcode, which may claim a length larger than
+		 * the AHS provided. Reject such a mismatch before allocating
+		 * to avoid an out-of-bounds read of the CDB buffer in
+		 * target_cmd_init_cdb().
+		 */
+		if (scsi_command_size(hdr->cdb) > cdb_length) {
+			pr_err("Extended CDB AHS: SCSI command size %u exceeds AHS-provided CDB length %u, protocol error.\n",
+			       scsi_command_size(hdr->cdb), cdb_length);
+			return iscsit_add_reject_cmd(cmd,
+				ISCSI_REASON_PROTOCOL_ERROR, buf);
+		}
+
 		cdb = kmalloc(cdb_length, GFP_KERNEL);
 		if (cdb == NULL)
 			return iscsit_add_reject_cmd(cmd,
-- 
2.34.1

[PATCH v2] scsi: target: iscsi: Reject CDB size exceeding available buffer in iscsit_setup_scsi_cmd

[ghuicao]

From: Cao Guanghui <caoguanghui@kylinos.cn>

In iscsit_setup_scsi_cmd(), the CDB is later re-parsed by
scsi_command_size() based on its SCSI opcode. For a VARIABLE_LENGTH_CMD
(0x7f) the returned size is cdb[7] + 8, where both cdb[0] and cdb[7]
come verbatim from the initiator-controlled PDU. The amount of CDB data
actually available is never cross-checked against this opcode-declared
length before the CDB is handed to target_cmd_init_cdb(), which does:

  memcpy(cmd->t_task_cdb, cdb, scsi_command_size(cdb));

An initiator can set cdb[0]=0x7f and cdb[7]=252 so that
scsi_command_size() returns 260, while the available CDB space is only
16 bytes (the basic header, when no Extended CDB AHS is present) or the
AHS-provided length. target_cmd_init_cdb() then reads up to 244 bytes
past the end of the CDB buffer, a heap out-of-bounds read. The leaked
bytes are later parsed as the CDB and can be indirectly observed by the
initiator through sense data and responses.

Reject the command when the opcode-declared CDB size exceeds the
available CDB space, before the CDB is passed on:

- Without an Extended CDB AHS the CDB is limited to ISCSI_CDB_SIZE (16)
  bytes in the basic header, so reject when scsi_command_size() >
  ISCSI_CDB_SIZE.
- With an Extended CDB AHS the buffer is allocated from the AHS-declared
  length, so reject when scsi_command_size() > cdb_length.

commit 2f3835771dff ("scsi: target: iscsi: reject invalid size Extended
CDB AHS") fixed the zero-length ahslength overflows and the AHS-buffer
overread, but did not cover the orthogonal "available CDB space vs.
opcode-declared size" path.

Fixes: 8f1f7d297bce ("scsi: target: iscsi: Add support for extended CDB AHS")
Signed-off-by: Cao Guanghui <caoguanghui@kylinos.cn>
---
v2:
  - Add bounds check for standard path without AHS (hdr->hlength == 0)
  - Use if/else to make the two paths explicit
  
drivers/target/iscsi/iscsi_target.c | 31 +++++++++++++++++++++++++++++
1 file changed, 31 insertions(+)

diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c
index 62ada3a52210..56f8fd461192 100644
--- a/drivers/target/iscsi/iscsi_target.c
+++ b/drivers/target/iscsi/iscsi_target.c
@@ -1100,7 +1100,22 @@ int iscsit_setup_scsi_cmd(struct iscsit_conn *conn, struct iscsit_cmd *cmd,
 
 	cdb = hdr->cdb;
 
-	if (hdr->hlength) {
+	if (!hdr->hlength) {
+		/*
+		 * Without an Extended CDB AHS the CDB is limited to the 16
+		 * bytes in the basic header. The CDB is later re-parsed by
+		 * scsi_command_size() based on its opcode, which may claim a
+		 * larger length (e.g. VARIABLE_LENGTH_CMD with cdb[7]=252).
+		 * Reject such a mismatch before handing the CDB to
+		 * target_cmd_init_cdb() to avoid an out-of-bounds read.
+		 */
+		if (scsi_command_size(hdr->cdb) > ISCSI_CDB_SIZE) {
+			pr_err("SCSI command size %u exceeds CDB size %u, protocol error.\n",
+			       scsi_command_size(hdr->cdb), ISCSI_CDB_SIZE);
+			return iscsit_add_reject_cmd(cmd,
+				ISCSI_REASON_PROTOCOL_ERROR, buf);
+		}
+	} else {
 		ecdb_ahdr = (struct iscsi_ecdb_ahdr *) (hdr + 1);
 		if (ecdb_ahdr->ahstype != ISCSI_AHSTYPE_CDB) {
 			pr_err("Additional Header Segment type %d not supported!\n",
@@ -1124,6 +1139,20 @@ int iscsit_setup_scsi_cmd(struct iscsit_conn *conn, struct iscsit_cmd *cmd,
 
 		cdb_length = ahslength - 1 + ISCSI_CDB_SIZE;
 
+		/*
+		 * The CDB buffer is later re-parsed by scsi_command_size()
+		 * based on its opcode, which may claim a length larger than
+		 * the AHS provided. Reject such a mismatch before allocating
+		 * to avoid an out-of-bounds read of the CDB buffer in
+		 * target_cmd_init_cdb().
+		 */
+		if (scsi_command_size(hdr->cdb) > cdb_length) {
+			pr_err("Extended CDB AHS: SCSI command size %u exceeds AHS-provided CDB length %u, protocol error.\n",
+			       scsi_command_size(hdr->cdb), cdb_length);
+			return iscsit_add_reject_cmd(cmd,
+				ISCSI_REASON_PROTOCOL_ERROR, buf);
+		}
+
 		cdb = kmalloc(cdb_length, GFP_KERNEL);
 		if (cdb == NULL)
 			return iscsit_add_reject_cmd(cmd,
-- 
2.34.1

Re: [PATCH v2] scsi: target: iscsi: Reject CDB size exceeding available buffer in iscsit_setup_scsi_cmd

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] FORTIFY_SOURCE kernel panic due to out-of-bounds memcpy from fixed-size AHS ecdb array.
--

commit c7194730eff1d4be537c227966c11dc80b91b92b
Author: Cao Guanghui <caoguanghui@kylinos.cn>

scsi: target: iscsi: Reject CDB size exceeding available buffer in iscsit_setup_scsi_cmd

This commit fixes a heap out-of-bounds read in iscsit_setup_scsi_cmd()
caused by an initiator claiming a larger SCSI command size than the available
CDB space. It adds bounds checks to reject the command if the opcode-declared
CDB size exceeds either the basic header limit or the AHS-provided length.

Fixes: 8f1f7d297bce ("scsi: target: iscsi: Add support for extended CDB AHS")

> diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c
> index 62ada3a52210e..56f8fd461192d 100644
> --- a/drivers/target/iscsi/iscsi_target.c
> +++ b/drivers/target/iscsi/iscsi_target.c

[ ... ]

> @@ -1124,6 +1139,20 @@ int iscsit_setup_scsi_cmd(struct iscsit_conn *conn, struct iscsit_cmd *cmd,
>  
>  		cdb_length = ahslength - 1 + ISCSI_CDB_SIZE;
>  
> +		/*
> +		 * The CDB buffer is later re-parsed by scsi_command_size()
> +		 * based on its opcode, which may claim a length larger than
> +		 * the AHS provided. Reject such a mismatch before allocating
> +		 * to avoid an out-of-bounds read of the CDB buffer in
> +		 * target_cmd_init_cdb().
> +		 */
> +		if (scsi_command_size(hdr->cdb) > cdb_length) {
> +			pr_err("Extended CDB AHS: SCSI command size %u exceeds AHS-provided CDB length %u, protocol error.\n",
> +			       scsi_command_size(hdr->cdb), cdb_length);
> +			return iscsit_add_reject_cmd(cmd,
> +				ISCSI_REASON_PROTOCOL_ERROR, buf);
> +		}
> +
>  		cdb = kmalloc(cdb_length, GFP_KERNEL);
>  		if (cdb == NULL)
>  			return iscsit_add_reject_cmd(cmd,
>  				ISCSI_REASON_BOOKMARK_NO_RESOURCES, buf);
>  		memcpy(cdb, hdr->cdb, ISCSI_CDB_SIZE);
>  		memcpy(cdb + ISCSI_CDB_SIZE, ecdb_ahdr->ecdb, cdb_length - ISCSI_CDB_SIZE);
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

[Severity: High]
This is a pre-existing issue, but does this memcpy() risk triggering a
FORTIFY_SOURCE kernel panic?

If a remote attacker sends an iSCSI SCSI CMD PDU with an Extended CDB AHS
where ahslength is large (e.g., 1017), the target accepts it because it is
bounded by the total AHS buffer length.

However, when copying the AHS payload to the newly allocated CDB buffer,
memcpy() reads cdb_length - ISCSI_CDB_SIZE (up to 1016) bytes from
ecdb_ahdr->ecdb.

Since ecdb_ahdr->ecdb is declared as a fixed-size array of 244 bytes
(SCSI_MAX_VARLEN_CDB_SIZE - ISCSI_CDB_SIZE) in include/scsi/iscsi_proto.h:

struct iscsi_ecdb_ahdr {
	...
	/* 4-byte aligned extended CDB spillover */
	uint8_t ecdb[SCSI_MAX_VARLEN_CDB_SIZE - ISCSI_CDB_SIZE];
};

reading more than 244 bytes with CONFIG_FORTIFY_SOURCE=y will detect an
out-of-bounds read and panic the kernel, causing a system-wide Denial of
Service.

Should we restrict ahslength (or cdb_length) to not exceed the maximum allowed
Extended CDB length of 244 bytes?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260826100526.106011-1-ghuicao@163.com?part=1

[PATCH v3] scsi: target: iscsi: Reject CDB size exceeding available buffer

[ghuicao]

From: Cao Guanghui <caoguanghui@kylinos.cn>

In iscsit_setup_scsi_cmd(), the CDB is later re-parsed by
scsi_command_size() based on its SCSI opcode. For a VARIABLE_LENGTH_CMD
(0x7f) the returned size is cdb[7] + 8, where both cdb[0] and cdb[7]
come verbatim from the initiator-controlled PDU. The amount of CDB data
actually available is never cross-checked against this opcode-declared
length before the CDB is handed to target_cmd_init_cdb(), which does:

  memcpy(cmd->t_task_cdb, cdb, scsi_command_size(cdb));

An initiator can set cdb[0]=0x7f and cdb[7]=252 so that
scsi_command_size() returns 260, while the available CDB space is only
16 bytes (the basic header, when no Extended CDB AHS is present) or the
AHS-provided length. target_cmd_init_cdb() then reads up to 244 bytes
past the end of the CDB buffer, a heap out-of-bounds read. The leaked
bytes are later parsed as the CDB and can be indirectly observed by the
initiator through sense data and responses.

Additionally, when an Extended CDB AHS is present with a large
ahslength (up to 1017, bounded only by hlength), the code copies
cdb_length - ISCSI_CDB_SIZE bytes from ecdb_ahdr->ecdb, a fixed 244-byte
array declared in struct iscsi_ecdb_ahdr. When cdb_length exceeds
SCSI_MAX_VARLEN_CDB_SIZE (260) this reads past the array, and with
CONFIG_FORTIFY_SOURCE=y the runtime check in fortify_memcpy_chk()
detects the overflow and panics the kernel, causing a system-wide
Denial of Service.

Reject the command before either memcpy can occur:

- Without an Extended CDB AHS the CDB is limited to ISCSI_CDB_SIZE (16)
  bytes in the basic header, so reject when scsi_command_size() >
  ISCSI_CDB_SIZE.
- With an Extended CDB AHS reject when cdb_length exceeds
  SCSI_MAX_VARLEN_CDB_SIZE (260), which also bounds the read from the
  fixed-size ecdb array, and reject when scsi_command_size() >
  cdb_length.

commit 2f3835771dff ("scsi: target: iscsi: reject invalid size Extended
CDB AHS") fixed the zero-length ahslength overflows and the AHS-buffer
overread, but did not cover the orthogonal "available CDB space vs.
opcode-declared size" path or the ecdb array overread.

Fixes: 8f1f7d297bce ("scsi: target: iscsi: Add support for extended CDB AHS")
Cc: stable@vger.kernel.org
Signed-off-by: Cao Guanghui <caoguanghui@kylinos.cn>
---
v3:
  - Add cdb_length > SCSI_MAX_VARLEN_CDB_SIZE check in AHS path to
    prevent FORTIFY_SOURCE panic from ecdb array overread (Sashiko)
  - Shorten subject line to fit 75-char convention
  - Add Cc: stable@vger.kernel.org

v2:
  - Add bounds check for standard path without AHS (hdr->hlength == 0)
  - Use if/else to make the two paths explicit

 drivers/target/iscsi/iscsi_target.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 45 insertions(+), 1 deletion(-)

diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c
--- a/drivers/target/iscsi/iscsi_target.c
+++ b/drivers/target/iscsi/iscsi_target.c
@@ -1100,7 +1100,22 @@ int iscsit_setup_scsi_cmd(struct iscsit_conn *conn, struct iscsit_cmd *cmd,
 
 	cdb = hdr->cdb;
 
-	if (hdr->hlength) {
+	if (!hdr->hlength) {
+		/*
+		 * Without an Extended CDB AHS the CDB is limited to the 16
+		 * bytes in the basic header. The CDB is later re-parsed by
+		 * scsi_command_size() based on its opcode, which may claim a
+		 * larger length (e.g. VARIABLE_LENGTH_CMD with cdb[7]=252).
+		 * Reject such a mismatch before handing the CDB to
+		 * target_cmd_init_cdb() to avoid an out-of-bounds read.
+		 */
+		if (scsi_command_size(hdr->cdb) > ISCSI_CDB_SIZE) {
+			pr_err("SCSI command size %u exceeds CDB size %u, protocol error.\n",
+			       scsi_command_size(hdr->cdb), ISCSI_CDB_SIZE);
+			return iscsit_add_reject_cmd(cmd,
+				ISCSI_REASON_PROTOCOL_ERROR, buf);
+		}
+	} else {
 		ecdb_ahdr = (struct iscsi_ecdb_ahdr *) (hdr + 1);
 		if (ecdb_ahdr->ahstype != ISCSI_AHSTYPE_CDB) {
 			pr_err("Additional Header Segment type %d not supported!\n",
@@ -1124,7 +1139,36 @@ int iscsit_setup_scsi_cmd(struct iscsit_conn *conn, struct iscsit_cmd *cmd,
 
 		cdb_length = ahslength - 1 + ISCSI_CDB_SIZE;
 
+		/*
+		 * The Extended CDB AHS ecdb field is a fixed-size array of
+		 * SCSI_MAX_VARLEN_CDB_SIZE - ISCSI_CDB_SIZE (244) bytes. An
+		 * attacker-controlled ahslength can make cdb_length exceed
+		 * this, causing the memcpy below to read past the ecdb array
+		 * and triggering a FORTIFY_SOURCE runtime panic. Reject CDB
+		 * lengths exceeding the SCSI maximum before allocating.
+		 */
+		if (cdb_length > SCSI_MAX_VARLEN_CDB_SIZE) {
+			pr_err("Extended CDB length %u exceeds maximum %u, protocol error.\n",
+			       cdb_length, SCSI_MAX_VARLEN_CDB_SIZE);
+			return iscsit_add_reject_cmd(cmd,
+				ISCSI_REASON_PROTOCOL_ERROR, buf);
+		}
+
+		/*
+		 * The CDB buffer is later re-parsed by scsi_command_size()
+		 * based on its opcode, which may claim a length larger than
+		 * the AHS provided. Reject such a mismatch before allocating
+		 * to avoid an out-of-bounds read of the CDB buffer in
+		 * target_cmd_init_cdb().
+		 */
+		if (scsi_command_size(hdr->cdb) > cdb_length) {
+			pr_err("Extended CDB AHS: SCSI command size %u exceeds AHS-provided CDB length %u, protocol error.\n",
+			       scsi_command_size(hdr->cdb), cdb_length);
+			return iscsit_add_reject_cmd(cmd,
+				ISCSI_REASON_PROTOCOL_ERROR, buf);
+		}
+
 		cdb = kmalloc(cdb_length, GFP_KERNEL);
 		if (cdb == NULL)
 			return iscsit_add_reject_cmd(cmd,
 				ISCSI_REASON_BOOKMARK_NO_RESOURCES, buf);
-- 
2.34.1

Re: [PATCH v3] scsi: target: iscsi: Reject CDB size exceeding available buffer

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The iSCSI target RX path fails to include Additional Header Segments (AHS) when computing the received HeaderDigest, breaking CRC verification for valid PDUs with AHS.
--

commit 6cf5aacf20580909d513380590652bf27da173c3
Author: Cao Guanghui <caoguanghui@kylinos.cn>

scsi: target: iscsi: Reject CDB size exceeding available buffer

This commit adds validation to ensure the CDB size requested by
scsi_command_size() and the Extended CDB length do not exceed available
buffers. It prevents out-of-bounds reads during memcpy in
target_cmd_init_cdb() and fortify_source panics when an Extended CDB AHS
is present.

> diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c
> index 62ada3a52210e..f19f1c31a39b2 100644
> --- a/drivers/target/iscsi/iscsi_target.c
> +++ b/drivers/target/iscsi/iscsi_target.c

[ ... ]

[Severity: High]
This isn't a bug introduced by this patch, but since this code correctly
handles AHS structures, I noticed a pre-existing issue in the iSCSI target
RX path that might also need addressing.

In iscsit_get_rx_pdu(), does the header digest calculation need to be updated
to include the AHS length?

drivers/target/iscsi/iscsi_target.c:iscsit_get_rx_pdu() {
    ...
			checksum = iscsit_crc_buf(buffer, ISCSI_HDR_LEN, 0,
						  NULL);
    ...
}

Doesn't this calculate the CRC exclusively over the 48-byte basic header
(ISCSI_HDR_LEN)?

If the initiator negotiates HeaderDigest and sends an iSCSI PDU with an
Additional Header Segment (hdr->hlength > 0), wouldn't this break CRC
verification for valid PDUs with AHS since they will be falsely rejected
with a CRC error?

According to RFC 3720 Section 10.2.1.2, shouldn't the Header Digest be
calculated over the basic header AND any AHS, using
ISCSI_HDR_LEN + (hdr->hlength * 4) instead of just ISCSI_HDR_LEN?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260827013245.21289-1-ghuicao@163.com?part=1

[PATCH v4 1/2] scsi: target: iscsi: Reject CDB size exceeding available buffer

[ghuicao]

From: Cao Guanghui <caoguanghui@kylinos.cn>

In iscsit_setup_scsi_cmd(), the CDB is later re-parsed by
scsi_command_size() based on its SCSI opcode. For a VARIABLE_LENGTH_CMD
(0x7f) the returned size is cdb[7] + 8, where both cdb[0] and cdb[7]
come verbatim from the initiator-controlled PDU. The amount of CDB data
actually available is never cross-checked against this opcode-declared
length before the CDB is handed to target_cmd_init_cdb(), which does:

  memcpy(cmd->t_task_cdb, cdb, scsi_command_size(cdb));

An initiator can set cdb[0]=0x7f and cdb[7]=252 so that
scsi_command_size() returns 260, while the available CDB space is only
16 bytes (the basic header, when no Extended CDB AHS is present) or the
AHS-provided length. target_cmd_init_cdb() then reads up to 244 bytes
past the end of the CDB buffer, a heap out-of-bounds read. The leaked
bytes are later parsed as the CDB and can be indirectly observed by the
initiator through sense data and responses.

Additionally, when an Extended CDB AHS is present with a large
ahslength (up to 1017, bounded only by hlength), the code copies
cdb_length - ISCSI_CDB_SIZE bytes from ecdb_ahdr->ecdb, a fixed 244-byte
array declared in struct iscsi_ecdb_ahdr. When cdb_length exceeds
SCSI_MAX_VARLEN_CDB_SIZE (260) this reads past the array, and with
CONFIG_FORTIFY_SOURCE=y the runtime check in fortify_memcpy_chk()
detects the overflow and panics the kernel, causing a system-wide
Denial of Service.

Reject the command before either memcpy can occur:

- Without an Extended CDB AHS the CDB is limited to ISCSI_CDB_SIZE (16)
  bytes in the basic header, so reject when scsi_command_size() >
  ISCSI_CDB_SIZE.
- With an Extended CDB AHS reject when cdb_length exceeds
  SCSI_MAX_VARLEN_CDB_SIZE (260), which also bounds the read from the
  fixed-size ecdb array, and reject when scsi_command_size() >
  cdb_length.

commit 2f3835771dff ("scsi: target: iscsi: reject invalid size Extended
CDB AHS") fixed the zero-length ahslength overflows and the AHS-buffer
overread, but did not cover the orthogonal "available CDB space vs.
opcode-declared size" path or the ecdb array overread.

Fixes: 8f1f7d297bce ("scsi: target: iscsi: Add support for extended CDB AHS")
Cc: stable@vger.kernel.org
Signed-off-by: Cao Guanghui <caoguanghui@kylinos.cn>
---
v3:
  - Add cdb_length > SCSI_MAX_VARLEN_CDB_SIZE check in AHS path to
    prevent FORTIFY_SOURCE panic from ecdb array overread (Sashiko)
  - Shorten subject line to fit 75-char convention
  - Add Cc: stable@vger.kernel.org

v2:
  - Add bounds check for standard path without AHS (hdr->hlength == 0)
  - Use if/else to make the two paths explicit

 drivers/target/iscsi/iscsi_target.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 45 insertions(+), 1 deletion(-)

diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c
--- a/drivers/target/iscsi/iscsi_target.c
+++ b/drivers/target/iscsi/iscsi_target.c
@@ -1100,7 +1100,22 @@ int iscsit_setup_scsi_cmd(struct iscsit_conn *conn, struct iscsit_cmd *cmd,
 
 	cdb = hdr->cdb;
 
-	if (hdr->hlength) {
+	if (!hdr->hlength) {
+		/*
+		 * Without an Extended CDB AHS the CDB is limited to the 16
+		 * bytes in the basic header. The CDB is later re-parsed by
+		 * scsi_command_size() based on its opcode, which may claim a
+		 * larger length (e.g. VARIABLE_LENGTH_CMD with cdb[7]=252).
+		 * Reject such a mismatch before handing the CDB to
+		 * target_cmd_init_cdb() to avoid an out-of-bounds read.
+		 */
+		if (scsi_command_size(hdr->cdb) > ISCSI_CDB_SIZE) {
+			pr_err("SCSI command size %u exceeds CDB size %u, protocol error.\n",
+			       scsi_command_size(hdr->cdb), ISCSI_CDB_SIZE);
+			return iscsit_add_reject_cmd(cmd,
+				ISCSI_REASON_PROTOCOL_ERROR, buf);
+		}
+	} else {
 		ecdb_ahdr = (struct iscsi_ecdb_ahdr *) (hdr + 1);
 		if (ecdb_ahdr->ahstype != ISCSI_AHSTYPE_CDB) {
 			pr_err("Additional Header Segment type %d not supported!\n",
@@ -1124,7 +1139,36 @@ int iscsit_setup_scsi_cmd(struct iscsit_conn *conn, struct iscsit_cmd *cmd,
 
 		cdb_length = ahslength - 1 + ISCSI_CDB_SIZE;
 
+		/*
+		 * The Extended CDB AHS ecdb field is a fixed-size array of
+		 * SCSI_MAX_VARLEN_CDB_SIZE - ISCSI_CDB_SIZE (244) bytes. An
+		 * attacker-controlled ahslength can make cdb_length exceed
+		 * this, causing the memcpy below to read past the ecdb array
+		 * and triggering a FORTIFY_SOURCE runtime panic. Reject CDB
+		 * lengths exceeding the SCSI maximum before allocating.
+		 */
+		if (cdb_length > SCSI_MAX_VARLEN_CDB_SIZE) {
+			pr_err("Extended CDB length %u exceeds maximum %u, protocol error.\n",
+			       cdb_length, SCSI_MAX_VARLEN_CDB_SIZE);
+			return iscsit_add_reject_cmd(cmd,
+				ISCSI_REASON_PROTOCOL_ERROR, buf);
+		}
+
+		/*
+		 * The CDB buffer is later re-parsed by scsi_command_size()
+		 * based on its opcode, which may claim a length larger than
+		 * the AHS provided. Reject such a mismatch before allocating
+		 * to avoid an out-of-bounds read of the CDB buffer in
+		 * target_cmd_init_cdb().
+		 */
+		if (scsi_command_size(hdr->cdb) > cdb_length) {
+			pr_err("Extended CDB AHS: SCSI command size %u exceeds AHS-provided CDB length %u, protocol error.\n",
+			       scsi_command_size(hdr->cdb), cdb_length);
+			return iscsit_add_reject_cmd(cmd,
+				ISCSI_REASON_PROTOCOL_ERROR, buf);
+		}
+
 		cdb = kmalloc(cdb_length, GFP_KERNEL);
 		if (cdb == NULL)
 			return iscsit_add_reject_cmd(cmd,
 				ISCSI_REASON_BOOKMARK_NO_RESOURCES, buf);
-- 
2.34.1

[PATCH v4 2/2] scsi: target: iscsi: Fix HeaderDigest to cover AHS data

[ghuicao]

From: Cao Guanghui <caoguanghui@kylinos.cn>

In iscsit_get_rx_pdu(), the HeaderDigest CRC32C is computed over only
the 48-byte basic header (ISCSI_HDR_LEN), excluding any Additional
Header Segments.  However, RFC 3720 Section 10.2.1.2 specifies that
the header digest covers "all the header segments of a PDU, i.e., BHS
and AHS."

The iSCSI initiator (libiscsi) correctly computes the digest over
BHS + AHS (task->hdr_len = 48 + hlength * 4), so any valid PDU with
both HeaderDigest negotiated and AHS present (e.g., SCSI commands with
Extended CDB) would fail CRC verification on the target side and be
falsely rejected.

Track the total header length in a local variable and use it for the
CRC computation so that the digest covers the full header as required
by the RFC.

Fixes: e48354ce078c ("iscsi-target: Add iSCSI fabric support for target v4.1")
Cc: stable@vger.kernel.org
Signed-off-by: Cao Guanghui <caoguanghui@kylinos.cn>
---
 drivers/target/iscsi/iscsi_target.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c
--- a/drivers/target/iscsi/iscsi_target.c
+++ b/drivers/target/iscsi/iscsi_target.c
@@ -4026,6 +4026,6 @@ static void iscsit_get_rx_pdu(struct iscsit_conn *conn)
 {
 	int ret;
 	u8 *buffer, *tmp_buf, opcode;
-	u32 checksum = 0, digest = 0;
+	u32 checksum = 0, digest = 0, header_length;
 	struct iscsi_hdr *hdr;
 	struct kvec iov;
@@ -4053,8 +4053,10 @@ static void iscsit_get_rx_pdu(struct iscsit_conn *conn)
 		}
 
 		hdr = (struct iscsi_hdr *) buffer;
+		header_length = ISCSI_HDR_LEN;
 		if (hdr->hlength) {
 			iov.iov_len = hdr->hlength * 4;
+			header_length = ISCSI_HDR_LEN + iov.iov_len;
 			tmp_buf = krealloc(buffer,
 					  ISCSI_HDR_LEN + iov.iov_len,
 					  GFP_KERNEL);
@@ -4082,5 +4084,5 @@ static void iscsit_get_rx_pdu(struct iscsit_conn *conn)
 			}
 
-			checksum = iscsit_crc_buf(buffer, ISCSI_HDR_LEN, 0,
-						  NULL);
+			checksum = iscsit_crc_buf(buffer, header_length, 0,
+						  NULL);
 			if (digest != checksum) {
-- 
2.34.1