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 <sammi...@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
>