[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

[sashiko-bot]

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

Pre-existing issues:
- [Critical] A malicious or misconfigured ATA device can trigger an out-of-bounds heap read in ata_dev_config_cpr() and a massive global buffer overflow in ata_scsiop_inq_b9() by reporting a large nr_cpr value. The newly added defensive check in ata_scsi_rbuf_fill() executes after the overflow has already occurred.
- [Critical] ata_format_dsm_trim_descr() overflows the static ata_scsi_rbuf for devices with large sector sizes.
--

commit 8b4a8e5bfe8806f87d80466e94aa7a4684517178
Author: Karuna Ramkumar <rkaruna@google.com>

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

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.

[ ... ]

> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> index d54ec1631e9a6..0ddec111a78c4 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;

[Severity: Critical]
This is a pre-existing issue, but does the overflow already occur before this
check is reached?

The actor function writes directly into the static ata_scsi_rbuf array.
For example, in ata_scsiop_inq_b9():

    for (i = 0; i < cpr_log->nr_cpr; i++, desc += 32) {
        ...
        put_unaligned_be64(cpr_log->cpr[i].num_lbas, &desc[16]);
    }

If a device reports a large nr_cpr value (up to 255), this loop could write
up to 8224 bytes into the 2048-byte ata_scsi_rbuf array during the actor()
call, before the defensive WARN_ON() check has a chance to execute.

Additionally, could ata_dev_config_cpr() perform an out-of-bounds heap read
when reading this large nr_cpr value?

    cpr_log->nr_cpr = nr_cpr;
    desc = &buf[64];
    for (i = 0; i < nr_cpr; i++, desc += 32) {
        cpr_log->cpr[i].num = desc[0];
        ...

If nr_cpr exceeds the allocated buffer space based on buf_len, this loop
appears to read past the end of the allocation.


[Severity: Critical]
This is also a pre-existing issue, but does ata_format_dsm_trim_descr()
overflow this same static buffer for devices with large sector sizes?

When a filesystem discard is issued to a device with a 4K logical sector size,
ata_scsi_write_same_xlat() calculates trmax as 512:

    size_t len = sdp->sector_size;
    ...
    const u32 trmax = len >> 3;

In ata_format_dsm_trim_descr():

    if (len > ATA_SCSI_RBUF_SIZE)
        len = ATA_SCSI_RBUF_SIZE;
    ...
    memset(buf, 0, len);
    while (i < trmax) {
        ...
        buf[i++] = __cpu_to_le64(entry);

While the memset() length is properly capped to 2048 bytes, the while loop
continues up to trmax. For a trmax of 512, this writes 512 8-byte entries
(4096 bytes) into the 2048-byte ata_scsi_rbuf array.

>  		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);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260629175024.2187500-1-rkaruna@google.com?part=1

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