[PATCH] scsi: libiscsi_tcp: bound SCSI Response data segment to the connection buffer

[PATCH] scsi: libiscsi_tcp: bound SCSI Response data segment to the connection buffer

[HyeongJun An]

iscsi_tcp_hdr_dissect() receives the data segment of several PDU types
into the fixed-size conn->data buffer, which is allocated for
ISCSI_DEF_MAX_RECV_SEG_LEN (8192) bytes.  For the LOGIN_RSP, TEXT_RSP,
REJECT and ASYNC_EVENT opcodes the dissect path already rejects a PDU
whose DataSegmentLength exceeds that buffer.

The SCSI Command Response (ISCSI_OP_SCSI_CMD_RSP) path also copies its
data segment (sense/response data) into conn->data via
iscsi_tcp_data_recv_prep(), but it does so without the same check.  The
only upstream bound on in.datalen is conn->max_recv_dlength, the
initiator's advertised MaxRecvDataSegmentLength, which is commonly
negotiated well above 8192 (open-iscsi defaults to 262144).  A target
that returns a SCSI Response with a DataSegmentLength between 8193 and
max_recv_dlength therefore overflows the 8192-byte conn->data buffer.

Apply the same bound used by the sibling opcodes before handing the
data segment to conn->data.

Fixes: a081c13e39b5 ("[SCSI] iscsi_tcp: split module into lib and lld")
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: HyeongJun An <sammiee5311@gmail.com>
---
 drivers/scsi/libiscsi_tcp.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/scsi/libiscsi_tcp.c b/drivers/scsi/libiscsi_tcp.c
index e90805ba868f..790d11ec00bc 100644
--- a/drivers/scsi/libiscsi_tcp.c
+++ b/drivers/scsi/libiscsi_tcp.c
@@ -753,6 +753,18 @@ iscsi_tcp_hdr_dissect(struct iscsi_conn *conn, struct iscsi_hdr *hdr)
 		spin_unlock(&conn->session->back_lock);
 		break;
 	case ISCSI_OP_SCSI_CMD_RSP:
+		/*
+		 * Sense/response data is received into conn->data, so bound
+		 * it to that buffer like the responses handled below.
+		 */
+		if (tcp_conn->in.datalen > ISCSI_DEF_MAX_RECV_SEG_LEN) {
+			iscsi_conn_printk(KERN_ERR, conn,
+					  "iscsi_tcp: received buffer of len %u but conn buffer is only %u (opcode %0x)\n",
+					  tcp_conn->in.datalen,
+					  ISCSI_DEF_MAX_RECV_SEG_LEN, opcode);
+			rc = ISCSI_ERR_PROTO;
+			break;
+		}
 		if (tcp_conn->in.datalen) {
 			iscsi_tcp_data_recv_prep(tcp_conn);
 			return 0;
-- 
2.43.0

Re: [PATCH] scsi: libiscsi_tcp: bound SCSI Response data segment to the connection buffer

[sashiko-bot]

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

Pre-existing issues:
- [Medium] In `iscsi_scsi_cmd_rsp()`, an incorrect bounds check allows a malicious or compromised iSCSI target to leak up to 2 bytes of uninitialized kernel memory (or previous packet data) into the SCSI sense buffer via a crafted `CHECK_CONDITION` response.
--

--- Patch [1]: [PATCH] scsi: libiscsi_tcp: bound SCSI Response data segment to the connection buffer ---
commit 997ca9836cebc0e8267dfdbdc7d5ae21d710b9fd
Author: HyeongJun An <sammiee5311@gmail.com>

scsi: libiscsi_tcp: bound SCSI Response data segment to the connection buffer

iscsi_tcp_hdr_dissect() receives the data segment of several PDU types
into the fixed-size conn->data buffer. This commit applies bounds
checking for the SCSI Command Response path before copying the data
segment to prevent overflowing the 8192-byte buffer.

> diff --git a/drivers/scsi/libiscsi_tcp.c b/drivers/scsi/libiscsi_tcp.c
> index 60cb6dc3c6f09..ef28543def9e8 100644
> --- a/drivers/scsi/libiscsi_tcp.c
> +++ b/drivers/scsi/libiscsi_tcp.c
> @@ -720,6 +720,18 @@ iscsi_tcp_hdr_dissect(struct iscsi_conn *conn, struct iscsi_hdr *hdr)
>  		spin_unlock(&conn->session->back_lock);
>  		break;
>  	case ISCSI_OP_SCSI_CMD_RSP:
> +		/*
> +		 * Sense/response data is received into conn->data, so bound
> +		 * it to that buffer like the responses handled below.
> +		 */
> +		if (tcp_conn->in.datalen > ISCSI_DEF_MAX_RECV_SEG_LEN) {

[Severity: Medium]
This is a pre-existing issue, but while looking at how sense data is read
from conn->data, I noticed a potential out-of-bounds read in
drivers/scsi/libiscsi.c:iscsi_scsi_cmd_rsp():

        senselen = get_unaligned_be16(data);
        if (datalen < senselen)
                goto invalid_datalen;

        memcpy(sc->sense_buffer, data + 2,
               min_t(uint16_t, senselen, SCSI_SENSE_BUFFERSIZE));

Does this code read past the end of the received data? If datalen is exactly
equal to senselen, the memcpy copies from data + 2, which could read 2 bytes
past the valid data in the buffer and expose uninitialized or stale memory
into the SCSI sense buffer. Should the bounds check be datalen < senselen + 2?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260710050645.1194212-1-sammiee5311@gmail.com?part=1

Re: [PATCH] scsi: libiscsi_tcp: bound SCSI Response data segment to the connection buffer

[Chris Leech]

On Fri, Jul 10, 2026 at 02:06:45PM +0900, HyeongJun An wrote:
> iscsi_tcp_hdr_dissect() receives the data segment of several PDU types
> into the fixed-size conn->data buffer, which is allocated for
> ISCSI_DEF_MAX_RECV_SEG_LEN (8192) bytes.  For the LOGIN_RSP, TEXT_RSP,
> REJECT and ASYNC_EVENT opcodes the dissect path already rejects a PDU
> whose DataSegmentLength exceeds that buffer.
> 
> The SCSI Command Response (ISCSI_OP_SCSI_CMD_RSP) path also copies its
> data segment (sense/response data) into conn->data via
> iscsi_tcp_data_recv_prep(), but it does so without the same check.  The
> only upstream bound on in.datalen is conn->max_recv_dlength, the
> initiator's advertised MaxRecvDataSegmentLength, which is commonly
> negotiated well above 8192 (open-iscsi defaults to 262144).  A target
> that returns a SCSI Response with a DataSegmentLength between 8193 and
> max_recv_dlength therefore overflows the 8192-byte conn->data buffer.
> 
> Apply the same bound used by the sibling opcodes before handing the
> data segment to conn->data.
> 
> Fixes: a081c13e39b5 ("[SCSI] iscsi_tcp: split module into lib and lld")
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: HyeongJun An <sammiee5311@gmail.com>
> ---

Thanks, this looks correct to me.

However, with this applying the same bounds check the SCSI_CMD_RSP
handling is now identical to the LOGIN_RSP, TEXT_RSP, REJECT and
ASYNC_EVENT cases. I think it would be better to simply combine them
instead of duplicating.

I've run some iSCSI regression testing with this change, without issue.

- Chris Leech

>  drivers/scsi/libiscsi_tcp.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/scsi/libiscsi_tcp.c b/drivers/scsi/libiscsi_tcp.c
> index e90805ba868f..790d11ec00bc 100644
> --- a/drivers/scsi/libiscsi_tcp.c
> +++ b/drivers/scsi/libiscsi_tcp.c
> @@ -753,6 +753,18 @@ iscsi_tcp_hdr_dissect(struct iscsi_conn *conn, struct iscsi_hdr *hdr)
>  		spin_unlock(&conn->session->back_lock);
>  		break;
>  	case ISCSI_OP_SCSI_CMD_RSP:
> +		/*
> +		 * Sense/response data is received into conn->data, so bound
> +		 * it to that buffer like the responses handled below.
> +		 */
> +		if (tcp_conn->in.datalen > ISCSI_DEF_MAX_RECV_SEG_LEN) {
> +			iscsi_conn_printk(KERN_ERR, conn,
> +					  "iscsi_tcp: received buffer of len %u but conn buffer is only %u (opcode %0x)\n",
> +					  tcp_conn->in.datalen,
> +					  ISCSI_DEF_MAX_RECV_SEG_LEN, opcode);
> +			rc = ISCSI_ERR_PROTO;
> +			break;
> +		}
>  		if (tcp_conn->in.datalen) {
>  			iscsi_tcp_data_recv_prep(tcp_conn);
>  			return 0;
> -- 
> 2.43.0
> 

[PATCH v2] scsi: libiscsi_tcp: bound SCSI Response data segment to the connection buffer

[HyeongJun An]

iscsi_tcp_hdr_dissect() receives the data segment of several PDU types
into the fixed-size conn->data buffer, which is allocated for
ISCSI_DEF_MAX_RECV_SEG_LEN (8192) bytes.  For the LOGIN_RSP, TEXT_RSP,
REJECT and ASYNC_EVENT opcodes the dissect path already rejects a PDU
whose DataSegmentLength exceeds that buffer.

The SCSI Command Response (ISCSI_OP_SCSI_CMD_RSP) path also copies its
data segment (sense/response data) into conn->data via
iscsi_tcp_data_recv_prep(), but it does so without the same check.  The
only upstream bound on in.datalen is conn->max_recv_dlength, the
initiator's advertised MaxRecvDataSegmentLength, which is commonly
negotiated well above 8192 (open-iscsi defaults to 262144).  A target
that returns a SCSI Response with a DataSegmentLength between 8193 and
max_recv_dlength therefore overflows the 8192-byte conn->data buffer.

Once the same bound applies, ISCSI_OP_SCSI_CMD_RSP is handled exactly
like those responses: bound the data segment, receive it into conn->data
when present, and otherwise complete the PDU with no data.  Fold the
opcode into that case group rather than duplicating the check.

Fixes: a081c13e39b5 ("[SCSI] iscsi_tcp: split module into lib and lld")
Suggested-by: Chris Leech <cleech@redhat.com>
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: HyeongJun An <sammiee5311@gmail.com>
---
v2: Fold ISCSI_OP_SCSI_CMD_RSP into the LOGIN_RSP/TEXT_RSP/REJECT/
    ASYNC_EVENT case group instead of duplicating the bounds check, as
    suggested by Chris Leech.  The handling is identical: both bound the
    data segment to conn->data, receive it when present, and otherwise
    complete the PDU with no data (the latter via the LOGOUT_RSP
    fallthrough).  No functional change from v1.

v1: https://lore.kernel.org/all/20260710050645.1194212-1-sammiee5311@gmail.com/

 drivers/scsi/libiscsi_tcp.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/drivers/scsi/libiscsi_tcp.c b/drivers/scsi/libiscsi_tcp.c
index e90805ba868f..7223bb18b048 100644
--- a/drivers/scsi/libiscsi_tcp.c
+++ b/drivers/scsi/libiscsi_tcp.c
@@ -752,13 +752,6 @@ iscsi_tcp_hdr_dissect(struct iscsi_conn *conn, struct iscsi_hdr *hdr)
 		rc = __iscsi_complete_pdu(conn, hdr, NULL, 0);
 		spin_unlock(&conn->session->back_lock);
 		break;
-	case ISCSI_OP_SCSI_CMD_RSP:
-		if (tcp_conn->in.datalen) {
-			iscsi_tcp_data_recv_prep(tcp_conn);
-			return 0;
-		}
-		rc = iscsi_complete_pdu(conn, hdr, NULL, 0);
-		break;
 	case ISCSI_OP_R2T:
 		if (ahslen) {
 			rc = ISCSI_ERR_AHSLEN;
@@ -766,6 +759,7 @@ iscsi_tcp_hdr_dissect(struct iscsi_conn *conn, struct iscsi_hdr *hdr)
 		}
 		rc = iscsi_tcp_r2t_rsp(conn, hdr);
 		break;
+	case ISCSI_OP_SCSI_CMD_RSP:
 	case ISCSI_OP_LOGIN_RSP:
 	case ISCSI_OP_TEXT_RSP:
 	case ISCSI_OP_REJECT:
-- 
2.43.0

Re: [PATCH v2] scsi: libiscsi_tcp: bound SCSI Response data segment to the connection buffer

[sashiko-bot]

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

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but the iSCSI TCP receive path acquires `session->back_lock` using plain `spin_lock()`, which can deadlock if a softirq preempts the workqueue.
--

--- Patch [1]: [PATCH v2] scsi: libiscsi_tcp: bound SCSI Response data segment to the connection buffer ---
commit 03e41264108be30b9b653aa5ed7bc2fa484238e9
Author: HyeongJun An <sammiee5311@gmail.com>

scsi: libiscsi_tcp: bound SCSI Response data segment to the connection buffer
    
This commit bounds the data segment of SCSI Command Responses to the connection
buffer in iscsi_tcp_hdr_dissect(). It groups the opcode with LOGIN_RSP and
other response types to apply the existing bounds check.

> diff --git a/drivers/scsi/libiscsi_tcp.c b/drivers/scsi/libiscsi_tcp.c
> index e90805ba868fb..7223bb18b0480 100644
> --- a/drivers/scsi/libiscsi_tcp.c
> +++ b/drivers/scsi/libiscsi_tcp.c
> @@ -752,13 +752,6 @@ iscsi_tcp_hdr_dissect(struct iscsi_conn *conn, struct iscsi_hdr *hdr)
>  		rc = __iscsi_complete_pdu(conn, hdr, NULL, 0);
>  		spin_unlock(&conn->session->back_lock);
>  		break;
> -	case ISCSI_OP_SCSI_CMD_RSP:
> -		if (tcp_conn->in.datalen) {
> -			iscsi_tcp_data_recv_prep(tcp_conn);
> -			return 0;
> -		}
> -		rc = iscsi_complete_pdu(conn, hdr, NULL, 0);

[Severity: High]
This isn't a bug introduced by this patch, but can the plain spin_lock()
used here and in the receive path cause a self-deadlock?

If the iscsi_recv_from_iscsi_q module parameter is enabled, packet reception
is routed to the workqueue which runs in process context:

drivers/scsi/iscsi_tcp.c:iscsi_sw_tcp_recv_data_work() {
    ...
    lock_sock(sk);
    iscsi_sw_tcp_recv_data(conn);
    release_sock(sk);
}

When iscsi_tcp_hdr_dissect() handles ISCSI_OP_SCSI_DATA_IN, it takes
the back_lock using a plain spin_lock():

drivers/scsi/libiscsi_tcp.c:iscsi_tcp_hdr_dissect() {
    ...
    case ISCSI_OP_SCSI_DATA_IN:
        spin_lock(&conn->session->back_lock);
    ...
}

Similarly, the iscsi_complete_pdu() function being removed from this specific
case statement, but still called for these operations, takes back_lock with
a plain spin_lock():

drivers/scsi/libiscsi.c:iscsi_complete_pdu() {
    ...
    spin_lock(&conn->session->back_lock);
    rc = __iscsi_complete_pdu(conn, hdr, data, datalen);
    spin_unlock(&conn->session->back_lock);
    ...
}

If a softirq (like a block completion or timer) interrupts the workqueue
thread while this lock is held, and the softirq attempts to acquire the same
lock using spin_lock_bh() (e.g., in the iscsi_queuecommand error path),
would it spin forever?

Does the workqueue handler need to disable bottom halves before entering the
receive path?

> -		break;
>  	case ISCSI_OP_R2T:
>  		if (ahslen) {
>  			rc = ISCSI_ERR_AHSLEN;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260716065848.1653431-1-sammiee5311@gmail.com?part=1