Re: [PATCH 1/3] zfcp: Enhance fsf status read buffer tracing

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Potential out-of-bounds read and information leak due to an unvalidated hardware-provided payload length causing an integer underflow.
--

commit 2a80a7c2006e6e0fd3b5316bfecd4d7662992064
Author: Chinmaya Kajagar <chinmayk@linux.ibm.com>

zfcp: Enhance fsf status read buffer tracing

This commit enhances the status read buffer trace records by appending the
remaining FSF status read buffer fields to the trace structure and extracting
the 3-byte source ID.

> diff --git a/drivers/s390/scsi/zfcp_dbf.c b/drivers/s390/scsi/zfcp_dbf.c
> index 71f625926ae19..89b859176b8bd 100644
> --- a/drivers/s390/scsi/zfcp_dbf.c
> +++ b/drivers/s390/scsi/zfcp_dbf.c
> @@ -220,6 +220,13 @@ void zfcp_dbf_hba_fsf_uss(char *tag, struct zfcp_fsf_req *req)
>  	rec->u.uss.lun = srb->fcp_lun;
>  	memcpy(&rec->u.uss.queue_designator, &srb->queue_designator,
>  	       sizeof(rec->u.uss.queue_designator));
> +	rec->u.uss.length = srb->length;
> +	rec->u.uss.res1 = srb->res1;
> +	rec->u.uss.res2 = srb->res2;
> +	rec->u.uss.class = srb->class;
> +	rec->u.uss.res3 = srb->res3;
> +	rec->u.uss.s_id = ntoh24(srb->s_id);
> +	memcpy(&rec->u.uss.res4, &srb->res4, sizeof(rec->u.uss.res4));
>  
>  	/* status read buffer payload length */
>  	rec->pl_len = (!srb->length) ? 0 : srb->length -
>  			offsetof(struct fsf_status_read_buffer, payload);

[Severity: High]
This isn't a bug introduced by this patch, but is there a risk of an integer
underflow here that could cause an out-of-bounds read?

In zfcp_dbf_hba_fsf_uss(), srb->length is an unsigned 32-bit integer read
from the hardware DMA buffer without validation. If the adapter provides a
non-zero length that is smaller than the offset of the payload (which is 64
bytes), the subtraction will underflow.

Because rec->pl_len is a 16-bit integer, the massively underflowed value gets
truncated. For example, a length of 32 would yield 65504. When this length is
passed to zfcp_dbf_pl_write(), it will read that many bytes from
srb->payload.data into the trace.

Since the memory backing srb is allocated as a single 4KB page, could this
copy read tens of thousands of bytes out-of-bounds, potentially leading to a
page fault or exposing adjacent kernel memory in the debugfs traces? It seems
like a similar out-of-bounds read might also happen if srb->length is simply
larger than the 4KB allocated size.

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