Re: [PATCH v3 2/3] scsi: libiscsi: validate the ITT reflected in a Reject PDU

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] scsi: libiscsi: TOCTOU race allows malicious target to corrupt SCSI task via Reject PDU
--

commit 0da359490f3ff756111324f1c5b68673b764be99
Author: Yehyeong Lee <yhlee@isslab.korea.ac.kr>

scsi: libiscsi: validate the ITT reflected in a Reject PDU

This commit adds validation to the ITT reflected in a Reject PDU to prevent
naming an unused or free task from the pool. It introduces checks to ensure
the task state is not ISCSI_TASK_FREE and that task->sc is NULL.

> diff --git a/drivers/scsi/libiscsi.c b/drivers/scsi/libiscsi.c
> index 7a74bc697d23b..ffd6446d07b28 100644
> --- a/drivers/scsi/libiscsi.c
> +++ b/drivers/scsi/libiscsi.c
> @@ -1142,13 +1142,15 @@ static int iscsi_handle_reject(struct iscsi_conn *conn, struct iscsi_hdr *hdr,
>  			spin_unlock(&conn->session->frwd_lock);
>  			spin_lock(&conn->session->back_lock);
>  		} else {
> -			struct iscsi_task *task;
> +			struct iscsi_task *task = NULL;
>  			/*
>  			 * Our nop as ping got dropped. We know the target
>  			 * and transport are ok so just clean up
>  			 */
> -			task = iscsi_itt_to_task(conn, rejected_pdu.itt);
> -			if (!task) {
> +			if (!iscsi_verify_itt(conn, rejected_pdu.itt))
> +				task = iscsi_itt_to_task(conn, rejected_pdu.itt);
> +			if (!task || task->state == ISCSI_TASK_FREE ||
> +			    task->sc) {

[Severity: High]
Does this introduce a Time-of-Check to Time-of-Use (TOCTOU) race with task
allocation?

The RX path executing iscsi_handle_reject() holds the session->back_lock,
while the TX path executing iscsi_alloc_task() holds the session->frwd_lock.

During task allocation, iscsi_alloc_task() performs these updates
sequentially:

iscsi_alloc_task() {
    ...
    task->state = ISCSI_TASK_PENDING;
    task->conn = conn;
    task->sc = sc;
    ...
}

Because these paths hold different locks, the RX path can observe an
incomplete task allocation where task->state is ISCSI_TASK_PENDING but
task->sc has not yet been assigned and is still NULL.

If a malicious target sends an ISCSI_OP_REJECT PDU matching the ITT of a task
currently being allocated, couldn't this check be completely bypassed?

This would falsely identify an in-flight SCSI task as a management task,
leading to premature completion via iscsi_nop_out_rsp().

This could eventually cause a refcount underflow and use-after-free when the
task actually completes.

>  				iscsi_conn_printk(KERN_ERR, conn,
>  						 "Invalid pdu reject. Could "
>  						 "not lookup rejected task.\n");

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261007015842.634893-1-yhlee@isslab.korea.ac.kr?part=2