Re: [PATCH v3] zfcp: Fix integer underflow in status read buffer payload length

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] scsi: zfcp: payload capture artificially truncated in zfcp_dbf_san_in_els()
--

commit 041b21a7756415603296020fe0feb75f0060af3c
Author: Ajaykumar Rajappa <ajaykr@linux.ibm.com>

zfcp: Fix integer underflow in status read buffer payload length

This commit prevents integer underflows when processing the status read
buffer payload length in zfcp_dbf_hba_fsf_uss() and zfcp_dbf_san_in_els().
It validates that the buffer length reaches the payload area before
subtraction and skips scatterlist setup when the payload is empty.

> diff --git a/drivers/s390/scsi/zfcp_dbf.c b/drivers/s390/scsi/zfcp_dbf.c
> --- a/drivers/s390/scsi/zfcp_dbf.c
> +++ b/drivers/s390/scsi/zfcp_dbf.c

[ ... ]

> @@ -716,15 +717,20 @@ void zfcp_dbf_san_in_els(char *tag, struct zfcp_fsf_req *fsf)
>  		(struct fsf_status_read_buffer *) fsf->data;
>  	u16 length;
>  	struct scatterlist sg;
> +	const u32 pay_offset = offsetof(struct fsf_status_read_buffer, payload);
>  
>  	if (unlikely(!debug_level_enabled(dbf->san, ZFCP_DBF_SAN_LEVEL)))
>  		return;
>  
> -	length = (u16)(srb->length -
> -			offsetof(struct fsf_status_read_buffer, payload));
> -	sg_init_one(&sg, srb->payload.data, length);
> -	zfcp_dbf_san(tag, dbf, "san_els", &sg, ZFCP_DBF_SAN_ELS, length,
> -		     fsf->req_id, ntoh24(srb->d_id), length);
> +	length = (srb->length < pay_offset) ? 0 :
> +		 (u16)(srb->length - pay_offset);
> +
> +	if (length)
> +		sg_init_one(&sg, srb->payload.data, length);
> +
> +	zfcp_dbf_san(tag, dbf, "san_els", length ? &sg : NULL, ZFCP_DBF_SAN_ELS,
> +		     length, fsf->req_id, ntoh24(srb->d_id),
> +		     min_t(u16, length, ZFCP_DBF_PAY_MAX_REC));

[Severity: Medium]
Does this artificially truncate the debugging trace payload for incoming ELS
frames?

When an ELS frame larger than 256 bytes is received while tracing is active,
passing min_t(u16, length, ZFCP_DBF_PAY_MAX_REC) here limits the capture to
the first 256 bytes, discarding the remainder of the payload and potentially
losing important debugging information.

The commit message states:

"Payload capture remains bounded by ZFCP_DBF_PAY_MAX_REC, preserving
existing trace behavior for valid status read buffers."

However, the previous code passed length directly as the capture length
argument, allowing zfcp_dbf_san() to write multiple 256-byte trace records
in a loop to capture the entire payload. Could this be updated to preserve
the original behavior of capturing the full payload length?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260924115204.1090452-1-ajaykr@linux.ibm.com?part=1