Re: [PATCH] ata: libata-scsi: limit simulated SCSI command copy to response length

[Damien Le Moal <dlemoal@kernel.org>]

On 6/30/26 02:50, Karuna Ramkumar wrote:
> The function ata_scsi_rbuf_fill() is used to copy the response of
> emulated SCSI commands from ata_scsi_rbuf to the SCSI command's
> scatterlist.
> 
> Currently, sg_copy_from_buffer() is called with the size argument
> set to ATA_SCSI_RBUF_SIZE (2048 bytes). Since ata_scsi_rbuf is
> zeroed out before the simulation actor is invoked, copying the
> full buffer size causes the remainder of the SCSI command's
> transfer buffer (beyond the actual response length 'len') to be
> overwritten with zeroes. This clobbers any pre-existing sentinel
> values or data in the caller's buffer tail, even though the
> correct residual count is reported via scsi_set_resid().
> 
> Fix this by passing the actual response length 'len' as the copy
> size to sg_copy_from_buffer(), ensuring that the tail of the
> caller's buffer remains untouched. Also, add a defensive
> WARN_ON() check to ensure that the actor does not return a
> length exceeding the static buffer capacity.
> 
> The fix was tested by invoking an SCSI SG_IO INQUIRY on
> an ATA disk on vanilla build, and build with the fix. Confirmed
> that the input buffer's tail end remains unmodified with the fix.
> 
> Fixes: 5251ae224d8d ("ata: libata-scsi: Return residual for emulated SCSI commands")
> Assisted-by: Antigravity:gemini-3.5-flash
> Signed-off-by: Karuna Ramkumar <rkaruna@google.com>
> ---
>  drivers/ata/libata-scsi.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> index d54ec1631e9a..0ddec111a78c 100644
> --- a/drivers/ata/libata-scsi.c
> +++ b/drivers/ata/libata-scsi.c
> @@ -1933,8 +1933,10 @@ static void ata_scsi_rbuf_fill(struct ata_device *dev, struct scsi_cmnd *cmd,
>  	memset(ata_scsi_rbuf, 0, ATA_SCSI_RBUF_SIZE);
>  	len = actor(dev, cmd, ata_scsi_rbuf);
>  	if (len) {
> +		if (WARN_ON(len > ATA_SCSI_RBUF_SIZE))
> +			len = ATA_SCSI_RBUF_SIZE;

Yes, it is good to add a warning, but if this ever happen, then it is a serious
bug in libata-scsi and we should fail the command rather than return data from
something buggy. So let's do that rather than just warn.
The other change below looks good.

>  		sg_copy_from_buffer(scsi_sglist(cmd), scsi_sg_count(cmd),
> -				    ata_scsi_rbuf, ATA_SCSI_RBUF_SIZE);
> +				    ata_scsi_rbuf, len);
>  		cmd->result = SAM_STAT_GOOD;
>  		if (scsi_bufflen(cmd) > len)
>  			scsi_set_resid(cmd, scsi_bufflen(cmd) - len);


-- 
Damien Le Moal
Western Digital Research