* Re: [PATCH] scsi: sd: bound MODE SENSE offset in sd_read_app_tag_own()
2026-07-17 22:14 [PATCH] scsi: sd: bound MODE SENSE offset in sd_read_app_tag_own() Jay Vadayath
@ 2026-07-17 22:29 ` sashiko-bot
0 siblings, 0 replies; 2+ messages in thread
From: sashiko-bot @ 2026-07-17 22:29 UTC (permalink / raw)
To: Jay Vadayath; +Cc: linux-scsi
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
^ permalink raw reply [flat|nested] 2+ messages in thread