[PATCH] scsi: sd: bound MODE SENSE offset in sd_read_app_tag_own()

[PATCH] scsi: sd: bound MODE SENSE offset in sd_read_app_tag_own()

[Jay Vadayath]

sd_read_app_tag_own() issues a MODE SENSE(10) against the target and
then indexes into the returned buffer at
	offset = data.header_length + data.block_descriptor_length;
buffer[offset] and buffer[offset + 4]. data.block_descriptor_length is
copied verbatim from the untrusted device response and can be as large
as 65535, so with a hostile response the offset can point well past the
36-byte SD_BUF_SIZE buffer the caller passes in, producing a
slab-out-of-bounds read.

KASAN report from an emulated USB mass storage device (raw-gadget +
dummy_hcd) returning a MODE SENSE(10) response with
block_descriptor_length = 600:

  BUG: KASAN: slab-out-of-bounds in sd_revalidate_disk+0x863f/0x8940
  Read of size 1 at addr ffff88800634de60 by task kworker/u8:0/12
  Call Trace:
   dump_stack_lvl+0x64/0x80
   print_report+0xce/0x620
   kasan_report+0xec/0x120
   sd_revalidate_disk+0x863f/0x8940
   sd_probe+0x79f/0xf20
   really_probe+0x1c8/0x950
   __driver_probe_device+0x1a5/0x430
   driver_probe_device+0x45/0xd0
   __device_attach_driver+0x156/0x230
   bus_for_each_drv+0x10f/0x190
   __device_attach+0x18e/0x3b0
   device_initial_probe+0x78/0xa0
   bus_probe_device+0x57/0x130

Reject responses whose mode page offset does not leave room for the
five bytes of the ATO mode page that this function subsequently
touches, and log the rejection.

This bug was discovered by Artiphishell's vTriage pipeline, which
generated a userspace raw-gadget reproducer that reliably triggers the
KASAN report on an unpatched kernel. The fix below was drafted with the
Claude coding assistant; a userspace reproducer is available on
request.

Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Jay Vadayath <jay@artiphishell.com>

---
 drivers/scsi/sd.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

--- a/drivers/scsi/sd.c
+++ b/drivers/scsi/sd.c
@@ -3366,6 +3366,18 @@ static void sd_read_app_tag_own(struct scsi_disk *sdkp, unsigned char *buffer)
 	}

 	offset = data.header_length + data.block_descriptor_length;
+
+	/*
+	 * block_descriptor_length is taken verbatim from the (untrusted)
+	 * device response and can be as large as 65535, so the computed
+	 * offset may point well past the SD_BUF_SIZE buffer that only holds
+	 * the 36 bytes we asked for.  Validate it before dereferencing to
+	 * avoid a slab out-of-bounds read.
+	 */
+	if (offset < 0 || offset + 5 >= 36) {
+		sd_first_printk(KERN_ERR, sdkp, "ATO mode page too long\n");
+		return;
+	}

 	if ((buffer[offset] & 0x3f) != 0x0a) {
 		sd_first_printk(KERN_ERR, sdkp, "ATO Got wrong page\n");

Re: [PATCH] scsi: sd: bound MODE SENSE offset in sd_read_app_tag_own()

[sashiko-bot]

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

Pre-existing issues:
- [Critical] In `cache_type_store()`, an unvalidated `data.block_descriptor_length` from the untrusted device causes an integer underflow when computing the copy length for a stack buffer, resulting in a stack out-of-bounds write and a kernel memory leak.
- [Critical] In `scsi_cdl_enable()`, an unvalidated `data.block_descriptor_length` from the untrusted device causes an integer underflow when computing the length for a stack buffer, resulting in a stack out-of-bounds write and a kernel memory leak.
- [High] The function `sd_read_io_hints()` calculates a loop boundary `end` using `data.length` directly from the untrusted device response. Since `data.length` is not clamped to the `SD_BUF_SIZE` (512 bytes) allocation length, `end` can point far beyond the buffer, leading to a severe slab out-of-bounds read.
--

--- Patch [1]: [PATCH] scsi: sd: bound MODE SENSE offset in sd_read_app_tag_own() ---
commit 78ac99a893d5ecde765f842f2b0590c22510f4a8
Author: Jay Vadayath <jay@artiphishell.com>

scsi: sd: bound MODE SENSE offset in sd_read_app_tag_own()

This patch adds bounds checking to sd_read_app_tag_own() to validate the
block_descriptor_length returned by a MODE SENSE(10) command, preventing
a slab out-of-bounds read.

> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index 599e75f333343..a5d0710fd82b5 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -3368,6 +3368,18 @@ static void sd_read_app_tag_own(struct scsi_disk *sdkp, unsigned char *buffer)
>  
>  	offset = data.header_length + data.block_descriptor_length;
>  
> +	/*
> +	 * block_descriptor_length is taken verbatim from the (untrusted)
> +	 * device response and can be as large as 65535, so the computed
> +	 * offset may point well past the SD_BUF_SIZE buffer that only holds
> +	 * the 36 bytes we asked for.  Validate it before dereferencing to
> +	 * avoid a slab out-of-bounds read.
> +	 */
> +	if (offset < 0 || offset + 5 >= 36) {
> +		sd_first_printk(KERN_ERR, sdkp, "ATO mode page too long\n");
> +		return;
> +	}
> +

[Severity: High]
This isn't a bug introduced by this patch, but does this same vulnerability
exist in sd_read_io_hints() in drivers/scsi/sd.c?

During device probe (sd_revalidate_disk), sd_read_io_hints() calls
scsi_mode_sense() asking for up to 512 bytes, but data.length is extracted
verbatim from the untrusted device response. Because it is not clamped to the
allocation length, can the loop boundary end exceed the buffer:

drivers/scsi/sd.c:sd_read_io_hints() {
    ...
    end = (void *)buffer + ALIGN_DOWN(data.header_length + data.length,
                                      sizeof(*end));
    ...
    for (desc = start; desc < end; desc++)
        if (!desc->st_enble || !sd_is_perm_stream(sdkp, desc - start))
            break;
    ...
}

Could this result in an out-of-bounds read when dereferencing desc->st_enble
if a malicious device returns a very large data.length?

[Severity: Critical]
This is a pre-existing issue, but does cache_type_store() in
drivers/scsi/sd.c have a similar underflow issue when using
data.block_descriptor_length?

When the cache_type sysfs attribute is written, it issues a scsi_mode_sense()
into a 64-byte stack buffer. If a device returns a large
block_descriptor_length, does the following subtraction underflow:

drivers/scsi/sd.c:cache_type_store() {
    ...
    len = min_t(size_t, sizeof(buffer), data.length - data.header_length -
          data.block_descriptor_length);
    buffer_data = buffer + data.header_length +
        data.block_descriptor_length;
    buffer_data[2] &= ~0x05;
    ...
}

Could this cause buffer_data to point outside the stack buffer, modifying
stack memory out-of-bounds, and then leak that memory to the device when
passed to scsi_mode_select()?

[Severity: Critical]
This isn't a bug introduced by this patch, but is there also a similar
pre-existing underflow in scsi_cdl_enable() in drivers/scsi/scsi.c?

During device scan, scsi_cdl_check() calls scsi_cdl_enable() which issues a
scsi_mode_sense() into a 64-byte stack buffer. If the target returns a large
block_descriptor_length, does the length calculation underflow here as well:

drivers/scsi/scsi.c:scsi_cdl_enable() {
    ...
    len = min_t(size_t, sizeof(buf),
                data.length - data.header_length -
                data.block_descriptor_length);
    buf_data = buf + data.header_length +
        data.block_descriptor_length;
    ...
    buf_data[4] &= ~0x03;
    ...
}

Since this is reachable automatically during probe with a USB storage device,
could this corrupt kernel stack memory and leak it back to the target?

>  	if ((buffer[offset] & 0x3f) != 0x0a) {
>  		sd_first_printk(KERN_ERR, sdkp, "ATO Got wrong page\n");
>  		return;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260717221442.57163-1-jay@artiphishell.com?part=1