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

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

[Karuna Ramkumar]

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;
 		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);
-- 
2.55.0.rc0.799.gd6f94ed593-goog

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

[Damien Le Moal]

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

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

[Karuna Ramkumar]

Thanks for the feedback! I'll push a new patch failing the command
instead of the warning.

Thanks,
Karuna Ramkumar

On Mon, Jun 29, 2026 at 5:05 PM Damien Le Moal <dlemoal@kernel.org> wrote:
>
> 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

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

[Damien Le Moal]

On 6/30/26 09:19, Karuna Ramkumar wrote:
> Thanks for the feedback! I'll push a new patch failing the command
> instead of the warning.

Please do not top post.

-- 
Damien Le Moal
Western Digital Research