scsi: sd: bound MODE SENSE offset in sd_read_app_tag_own()
PATCH 1/1 scsi: sd: bound MODE SENSE offset in sd_read_app_tag_own()
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.
--- drivers/scsi/sd.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
drivers/scsi/sd.c +12 -0
@@ -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" );
[email protected]
AI reviewThank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
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 <[email protected]>
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.
Quoted 10 lines
drivers/scsi/sd.c +2
-0@@ -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)
drivers/scsi/sd.c static void sd_read_app_tag_own(struct scsi_disk *sdkp, unsigned char *buffer) · line 3382
Quoted 10 lines
drivers/scsi/sd.c +10
-0+* 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?
Quoted 3 lines
drivers/scsi/sd.c +0
-0if ((buffer[offset] &0x3f ) !=0x0a ) {sd_first_printk (KERN_ERR , sdkp,"ATO Got wrong page\n" );return ;
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1