Re: [PATCH] target: iscsi: reject ImmediateData on non-WRITE commands

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] iscsi: PDU injection via unconsumed immediate data on reject
--

commit 8092717f025cfe28cadafc17843da9c3d45dd5dc
Author: sung byeongchan <tjdqudcks0424@naver.com>

target: iscsi: reject ImmediateData on non-WRITE commands

This commit modifies iscsit_setup_scsi_cmd() to reject non-WRITE commands
that contain ImmediateData.

> diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c
> index 124ff269b8e75..b8e26e00cd462 100644
> --- a/drivers/target/iscsi/iscsi_target.c
> +++ b/drivers/target/iscsi/iscsi_target.c
> @@ -1098,6 +1098,12 @@ int iscsit_setup_scsi_cmd(struct iscsit_conn *conn, struct iscsit_cmd *cmd,
>  					     ISCSI_REASON_BOOKMARK_INVALID, buf);
>  	}
>  
> +	if (payload_length && !(hdr->flags & ISCSI_FLAG_CMD_WRITE)) {
> +		pr_err("Immediate data for non-WRITE command\n");
> +		return iscsit_add_reject_cmd(cmd,
> +					     ISCSI_REASON_PROTOCOL_ERROR, buf);
> +	}
> +
>  	cdb = hdr->cdb;

[Severity: High]
Does rejecting the command here leave attacker-controlled data in the receive
buffer?

When iscsit_setup_scsi_cmd() returns the rejection, the caller
iscsit_handle_scsi_cmd() handles the error by returning 0:

iscsit_handle_scsi_cmd() {
    ...
	rc = iscsit_setup_scsi_cmd(conn, cmd, buf);
	if (rc < 0)
		return 0;
    ...
}

This fails to consume the payload_length bytes from the TCP socket and leaves
it open for further reading.

Because iscsi_target_rx_opcode() returns 0, the receive loop in
iscsit_get_rx_pdu() continues and reads the next ISCSI_HDR_LEN bytes from the
unconsumed immediate data:

iscsit_get_rx_pdu() {
    ...
		ret = rx_data(conn, &iov, 1, ISCSI_HDR_LEN);
		if (ret != ISCSI_HDR_LEN) {
    ...
		ret = iscsi_target_rx_opcode(conn, buffer);
		if (ret < 0)
			break;
    ...
}

Could an attacker use this unconsumed immediate data to inject arbitrary iSCSI
PDUs that bypass deep packet inspection?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261008060347.175260-1-tjdqudcks0424@naver.com?part=1