[PATCH] ata: libata-scsi: bound the ATA passthru sense descriptor writes

[PATCH] ata: libata-scsi: bound the ATA passthru sense descriptor writes

[Matthias Goergens]

When an ATA PASS-THROUGH command to an ATAPI device fails, the sense
buffer holds the device's REQUEST SENSE reply, and
ata_scsi_set_passthru_sense_fields() trusts its additional length
byte, sb[7], when adding the ATA Status Return descriptor.  A faulty
or malicious device can use that to make the kernel read and write
past the 96-byte buffer in three ways:

- scsi_sense_desc_find() is passed sb[7] + 8 as the buffer length, so
  its clamp against sb[7] does nothing and the walk runs off the end.
- A type-9 descriptor found near the end is filled in unchecked.
- A new descriptor at sb[8 + len] needs len + 22 bytes, not len + 14,
  so len 75..82 writes up to 8 bytes past the end.

Reproduced with KASAN under qemu, with the emulated ATAPI REQUEST SENSE
reply patched:

  BUG: KASAN: slab-out-of-bounds in scsi_sense_desc_find+0x1a5/0x210
  BUG: KASAN: slab-out-of-bounds in ata_scsi_qc_complete+0x1a15/0x1a50

Both are gone with this patch, and a valid descriptor is still filled
in.

Fixes: 97981926224a ("ata: libata-scsi: Do not overwrite valid sense data when CK_COND=1")
Cc: stable@vger.kernel.org
Signed-off-by: Matthias Goergens <matthias.goergens@gmail.com>
---
 drivers/ata/libata-scsi.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 7e22bbc38238..40e56ff52f09 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -261,12 +261,18 @@ static void ata_scsi_set_passthru_sense_fields(struct ata_queued_cmd *qc)
 
 		/* descriptor format */
 		len = sb[7];
-		desc = (char *)scsi_sense_desc_find(sb, len + 8, 9);
+		desc = (char *)scsi_sense_desc_find(sb, SCSI_SENSE_BUFFERSIZE, 9);
 		if (!desc) {
-			if (SCSI_SENSE_BUFFERSIZE < len + 14)
+			/*
+			 * The descriptor is written at sb[8 + len] and is 14
+			 * bytes long, so it needs len + 22 bytes of buffer.
+			 */
+			if (len + 22 > SCSI_SENSE_BUFFERSIZE)
 				return;
 			sb[7] = len + 14;
 			desc = sb + 8 + len;
+		} else if (desc + 14 > sb + SCSI_SENSE_BUFFERSIZE) {
+			return;
 		}
 		desc[0] = 9;
 		desc[1] = 12;

base-commit: 40288c9206c17eb66a603262e06a58d300d0f279
-- 
2.55.0

Re: [PATCH] ata: libata-scsi: bound the ATA passthru sense descriptor writes

[Damien Le Moal]

On 9/23/26 03:26, Matthias Goergens wrote:
> When an ATA PASS-THROUGH command to an ATAPI device fails, the sense
> buffer holds the device's REQUEST SENSE reply, and
> ata_scsi_set_passthru_sense_fields() trusts its additional length
> byte, sb[7], when adding the ATA Status Return descriptor.  A faulty
> or malicious device can use that to make the kernel read and write
> past the 96-byte buffer in three ways:
> 
> - scsi_sense_desc_find() is passed sb[7] + 8 as the buffer length, so
>   its clamp against sb[7] does nothing and the walk runs off the end.
> - A type-9 descriptor found near the end is filled in unchecked.
> - A new descriptor at sb[8 + len] needs len + 22 bytes, not len + 14,
>   so len 75..82 writes up to 8 bytes past the end.
> 
> Reproduced with KASAN under qemu, with the emulated ATAPI REQUEST SENSE
> reply patched:
> 
>   BUG: KASAN: slab-out-of-bounds in scsi_sense_desc_find+0x1a5/0x210
>   BUG: KASAN: slab-out-of-bounds in ata_scsi_qc_complete+0x1a15/0x1a50
> 
> Both are gone with this patch, and a valid descriptor is still filled
> in.
> 
> Fixes: 97981926224a ("ata: libata-scsi: Do not overwrite valid sense data when CK_COND=1")
> Cc: stable@vger.kernel.org
> Signed-off-by: Matthias Goergens <matthias.goergens@gmail.com>

Looks good.

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>

-- 
Damien Le Moal
Western Digital Research

Re: [PATCH] ata: libata-scsi: bound the ATA passthru sense descriptor writes

[Niklas Cassel]

On Wed, Sep 23, 2026 at 02:26:55AM +0800, Matthias Goergens wrote:
> When an ATA PASS-THROUGH command to an ATAPI device fails, the sense
> buffer holds the device's REQUEST SENSE reply, and
> ata_scsi_set_passthru_sense_fields() trusts its additional length
> byte, sb[7], when adding the ATA Status Return descriptor.  A faulty
> or malicious device can use that to make the kernel read and write
> past the 96-byte buffer in three ways:
> 
> - scsi_sense_desc_find() is passed sb[7] + 8 as the buffer length, so
>   its clamp against sb[7] does nothing and the walk runs off the end.
> - A type-9 descriptor found near the end is filled in unchecked.
> - A new descriptor at sb[8 + len] needs len + 22 bytes, not len + 14,
>   so len 75..82 writes up to 8 bytes past the end.
> 
> Reproduced with KASAN under qemu, with the emulated ATAPI REQUEST SENSE
> reply patched:
> 
>   BUG: KASAN: slab-out-of-bounds in scsi_sense_desc_find+0x1a5/0x210
>   BUG: KASAN: slab-out-of-bounds in ata_scsi_qc_complete+0x1a15/0x1a50
> 
> Both are gone with this patch, and a valid descriptor is still filled
> in.
> 
> Fixes: 97981926224a ("ata: libata-scsi: Do not overwrite valid sense data when CK_COND=1")
> Cc: stable@vger.kernel.org
> Signed-off-by: Matthias Goergens <matthias.goergens@gmail.com>
> ---
>  drivers/ata/libata-scsi.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
> index 7e22bbc38238..40e56ff52f09 100644
> --- a/drivers/ata/libata-scsi.c
> +++ b/drivers/ata/libata-scsi.c
> @@ -261,12 +261,18 @@ static void ata_scsi_set_passthru_sense_fields(struct ata_queued_cmd *qc)
>  
>  		/* descriptor format */
>  		len = sb[7];
> -		desc = (char *)scsi_sense_desc_find(sb, len + 8, 9);
> +		desc = (char *)scsi_sense_desc_find(sb, SCSI_SENSE_BUFFERSIZE, 9);
>  		if (!desc) {
> -			if (SCSI_SENSE_BUFFERSIZE < len + 14)
> +			/*
> +			 * The descriptor is written at sb[8 + len] and is 14
> +			 * bytes long, so it needs len + 22 bytes of buffer.
> +			 */
> +			if (len + 22 > SCSI_SENSE_BUFFERSIZE)
>  				return;
>  			sb[7] = len + 14;
>  			desc = sb + 8 + len;
> +		} else if (desc + 14 > sb + SCSI_SENSE_BUFFERSIZE) {

desc + 14 can point past one-past-the-end before the comparison, which
strict C doesn't allow. The kernel builds with -fno-strict-overflow,
so in practice it's fine, but:

+               } else if (desc - sb > SCSI_SENSE_BUFFERSIZE - 14) {

should be equivalent and safer.


Kind regards,
Niklas