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

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 <[email protected]>

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/[email protected]?part=1