Re: [PATCH] scsi: libiscsi_tcp: check that a read got the data the target claimed

[Mike Christie]

On 8/14/26 7:14 AM, Yehyeong Lee wrote:
> @@ -752,13 +795,25 @@ 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:
> +	case ISCSI_OP_SCSI_CMD_RSP: {
> +		struct iscsi_scsi_rsp *rsp = (struct iscsi_scsi_rsp *)hdr;
> +
> +		spin_lock(&conn->session->back_lock);
> +		task = iscsi_itt_to_ctask(conn, hdr->itt);
> +		if (task)
> +			rc = iscsi_tcp_check_data_in(task, rsp->flags,
> +						     be32_to_cpu(rsp->residual_count));
> +		spin_unlock(&conn->session->back_lock);
> +		if (rc)
> +			break;
> +
>  		if (tcp_conn->in.datalen) {
>  			iscsi_tcp_data_recv_prep(tcp_conn);
>  			return 0;
>  		}

For the normal case we will grab and drop the lock multiple times.

Could you switch it around where we do:

spin_lock(&conn->session->back_lock);
task = iscsi_itt_to_ctask(conn, hdr->itt);
if (!task) {
	spin_unlock(&conn->session->back_lock);
	return ISCSI_ERR_BAD_ITT;
}

rc = iscsi_tcp_check_data_in(task, rsp->flags,
			     be32_to_cpu(rsp->residual_count));
if (rc) {
	spin_unlock(&conn->session->back_lock);
	return rc;
}

if (tcp_conn->in.datalen) {
	spin_unlock(&conn->session->back_lock);
	iscsi_tcp_data_recv_prep(tcp_conn);
	return 0;
}

rc = __iscsi_complete_pdu(conn, hdr, NULL, 0);
spin_unlock(&conn->session->back_lock);
break;

>  		rc = iscsi_complete_pdu(conn, hdr, NULL, 0);
>  		break;
> +	}
>  	case ISCSI_OP_R2T:
>  		if (ahslen) {
>  			rc = ISCSI_ERR_AHSLEN;
> @@ -998,6 +1053,7 @@ int iscsi_tcp_task_init(struct iscsi_task *task)
>  
>  	BUG_ON(kfifo_len(&tcp_task->r2tqueue));
>  	tcp_task->exp_datasn = 0;
> +	tcp_task->data_in_bytes = 0;
>  
>  	/* Prepare PDU, optionally w/ immediate data */
>  	ISCSI_DBG_TCP(conn, "task deq [itt 0x%x imm %d unsol %d]\n",
> diff --git a/include/scsi/libiscsi_tcp.h b/include/scsi/libiscsi_tcp.h
> index ef53d4bea28a0..b38be23b9e10b 100644
> --- a/include/scsi/libiscsi_tcp.h
> +++ b/include/scsi/libiscsi_tcp.h
> @@ -66,6 +66,7 @@ struct iscsi_tcp_conn {
>  
>  struct iscsi_tcp_task {
>  	uint32_t		exp_datasn;	/* expected target's R2TSN/DataSN */
> +	u32			data_in_bytes;	/* Data-In payload received */

I know we are supposed to use u32, but could you do uint32_t to match the
rest of the code. It just makes it easier to search/review when the code
uses the same style.


>  	int			data_offset;
>  	struct iscsi_r2t_info	*r2t;		/* in progress solict R2T */
>  	struct iscsi_pool	r2tpool;
> -- 
> 2.43.0
>