* [PATCH] scsi: scsi_debug: fix REPORT ZONES alloc_len underflow OOB write
@ 2026-07-09 15:06 Ibrahim Hashimov
2026-07-09 15:18 ` sashiko-bot
2026-07-09 19:48 ` [PATCH v2] " Ibrahim Hashimov
0 siblings, 2 replies; 13+ messages in thread
From: Ibrahim Hashimov @ 2026-07-09 15:06 UTC (permalink / raw)
To: James E . J . Bottomley, Martin K . Petersen
Cc: linux-scsi, linux-kernel, stable
resp_report_zones() only rejects a REPORT ZONES(16) CDB when the
requested allocation length (cmd[10..13], SBC/ZBC "ALLOCATION LENGTH")
is exactly zero:
alloc_len = get_unaligned_be32(cmd + 10);
if (alloc_len == 0)
return 0; /* not an error */
...
rep_max_zones = (alloc_len - 64) >> ilog2(RZONES_DESC_HD);
arr = kzalloc(alloc_len, GFP_ATOMIC | __GFP_NOWARN);
...
desc = arr + 64;
For any nonzero alloc_len in the range 1..63, `alloc_len - 64`
underflows (alloc_len and rep_max_zones are unsigned), turning
rep_max_zones into a huge value (~2^25 for typical small alloc_len).
Meanwhile arr is allocated with the raw, unvalidated alloc_len, so it
can be smaller than the 64-byte report header. desc is then set to
arr + 64, which already points past the end of the (too small)
allocation, and the per-zone descriptor loop:
if (nrz < rep_max_zones) {
desc[0] = zsp->z_type;
...
put_unaligned_be64((u64)zsp->z_wp, desc + 24);
desc += 64;
}
keeps writing 64-byte zone descriptors starting at that out-of-bounds
pointer and marching forward, because the inflated rep_max_zones no
longer bounds anything. A local CAP_SYS_RAWIO attacker who loads
scsi_debug in zoned mode (zbc=host-managed) and sends a REPORT ZONES
CDB with e.g. alloc_len=32 via SG_IO triggers a heap
slab-out-of-bounds write, confirmed under KASAN
("slab-out-of-bounds in resp_report_zones", writes landing at
arr+64, arr+128, ... i.e. exactly the desc[0]/desc[1]/
put_unaligned_be64(desc+8/16/24) sequence, stepping by 64 bytes per
iteration).
Every other REPORT-style/allocation-length-consuming handler in this
file floors alloc_len against its own header/descriptor size before
using it, e.g.:
resp_report_tgtpgs(): if (alloc_len < 4 || alloc_len > 0xffff) ...
resp_readcap16(): if (alloc_len < 24) return 0;
resp_get_stream_status(): if (alloc_len < 8) { ... }
resp_report_luns(): if (alloc_len < 4) { ... }
resp_report_zones() is missing the equivalent floor. Since the report
header itself is RZONES_DESC_HD (64) bytes, alloc_len must be at
least that before rep_max_zones is computed and before desc is walked
past the header. Add that check, rejecting an under-sized allocation
length the same way resp_get_stream_status() rejects an under-sized
one for the identical cmd[10..13] field (SDEB_IN_CDB, field offset
10) via mk_sense_invalid_fld() + check_condition_result. alloc_len==0
keeps its existing "not an error, nothing to report" fast path.
This bounds both the underflowing subtraction and the kzalloc() size
against the 64-byte header the function unconditionally writes,
without touching the per-zone descriptor loop itself.
Verified on a v6.19 KASAN build: issuing REPORT ZONES(16) via SG_IO
with alloc_len=32 against a scsi_debug zbc=host-managed device hits
the slab-out-of-bounds write in resp_report_zones() before this
patch, and the same command no longer triggers a KASAN report once
the floor check above is applied.
Fixes: 7db0e0c8190a ("scsi: scsi_debug: Fix buffer size of REPORT ZONES command")
Cc: stable@vger.kernel.org
Signed-off-by: Ibrahim Hashimov <security@auditcode.ai>
Assisted-by: AuditCode-AI:2026.07
---
drivers/scsi/scsi_debug.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 9d1c9c41d0f9..2fa887c65b61 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -5911,6 +5911,11 @@ static int resp_report_zones(struct scsi_cmnd *scp,
return check_condition_result;
}
+ if (alloc_len < RZONES_DESC_HD) {
+ mk_sense_invalid_fld(scp, SDEB_IN_CDB, 10, -1);
+ return check_condition_result;
+ }
+
rep_max_zones = (alloc_len - 64) >> ilog2(RZONES_DESC_HD);
arr = kzalloc(alloc_len, GFP_ATOMIC | __GFP_NOWARN);
--
2.50.1 (Apple Git-155)
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH] scsi: scsi_debug: fix REPORT ZONES alloc_len underflow OOB write 2026-07-09 15:06 [PATCH] scsi: scsi_debug: fix REPORT ZONES alloc_len underflow OOB write Ibrahim Hashimov @ 2026-07-09 15:18 ` sashiko-bot 2026-07-09 19:48 ` [PATCH v2] " Ibrahim Hashimov 1 sibling, 0 replies; 13+ messages in thread From: sashiko-bot @ 2026-07-09 15:18 UTC (permalink / raw) To: Ibrahim Hashimov; +Cc: linux-scsi Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] Rejecting a REPORT ZONES command with an allocation length smaller than the header size (64 bytes) violates SCSI specifications and breaks legitimate host queries. -- --- Patch [1]: [PATCH] scsi: scsi_debug: fix REPORT ZONES alloc_len underflow OOB write --- commit e364d5684dfdc126ea92154dab5b52f73d6c269d Author: Ibrahim Hashimov <security@auditcode.ai> scsi: scsi_debug: fix REPORT ZONES alloc_len underflow OOB write This commit adds a check in resp_report_zones() to reject a report zones command if the allocation length is smaller than the 64-byte header size. This is intended to fix an underflow and subsequent out-of-bounds write when calculating rep_max_zones. > diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c > index 9d1c9c41d0f99..2fa887c65b61b 100644 > --- a/drivers/scsi/scsi_debug.c > +++ b/drivers/scsi/scsi_debug.c [ ... ] > @@ -5911,6 +5911,11 @@ static int resp_report_zones(struct scsi_cmnd *scp, > return check_condition_result; > } > > + if (alloc_len < RZONES_DESC_HD) { > + mk_sense_invalid_fld(scp, SDEB_IN_CDB, 10, -1); > + return check_condition_result; > + } > + [Severity: Medium] Does rejecting the command here when alloc_len < RZONES_DESC_HD violate the SCSI specifications? The SPC-5 and ZBC standards mandate that allocation length truncation is not an error. Host applications may probe the zone list length by sending a report zones command with a small allocation length (between 1 and 63 bytes) before allocating a full buffer. Will returning a check condition (invalid field in CDB) break these legitimate host queries? > rep_max_zones = (alloc_len - 64) >> ilog2(RZONES_DESC_HD); > > arr = kzalloc(alloc_len, GFP_ATOMIC | __GFP_NOWARN); -- Sashiko AI review · https://sashiko.dev/#/patchset/20260709150631.45018-1-security@auditcode.ai?part=1 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2] scsi: scsi_debug: fix REPORT ZONES alloc_len underflow OOB write 2026-07-09 15:06 [PATCH] scsi: scsi_debug: fix REPORT ZONES alloc_len underflow OOB write Ibrahim Hashimov 2026-07-09 15:18 ` sashiko-bot @ 2026-07-09 19:48 ` Ibrahim Hashimov 2026-07-10 0:58 ` Damien Le Moal 1 sibling, 1 reply; 13+ messages in thread From: Ibrahim Hashimov @ 2026-07-09 19:48 UTC (permalink / raw) To: martin.petersen, James.Bottomley Cc: shinichiro.kawasaki, damien.lemoal, linux-scsi, linux-kernel, stable resp_report_zones() reads the REPORT ZONES(16) ALLOCATION LENGTH field (cmd[10..13]) into the unsigned alloc_len and, apart from the alloc_len == 0 fast path, uses it without flooring it against the 64-byte report header: rep_max_zones = (alloc_len - 64) >> ilog2(RZONES_DESC_HD); arr = kzalloc(alloc_len, GFP_ATOMIC | __GFP_NOWARN); ... desc = arr + 64; For any alloc_len in the range 1..63, alloc_len - 64 wraps around (alloc_len and rep_max_zones are unsigned), so rep_max_zones becomes a huge value instead of zero. At the same time arr is allocated with the raw alloc_len, which is smaller than the 64-byte header the function always builds, and desc is set to arr + 64, already past the end of the allocation. The report header stores (put_unaligned_be32 at arr+0, put_unaligned_be64 at arr+8 and arr+16) can then run past a sub-24-byte buffer, and the per-zone descriptor loop, no longer bounded by the inflated rep_max_zones, writes 64-byte descriptors from desc onward, producing a slab out-of-bounds write. Fix it the way ZBC and SPC require: allocation length truncation is not an error, and a small alloc_len is a legitimate probe a host uses to read the zone list length before allocating a full buffer. Clamp rep_max_zones to zero when alloc_len is below the header size so no descriptor is emitted, and size the allocation to at least the header so the unconditional 64-byte header build cannot overflow. The existing copy-out already truncates the result with fill_from_dev_buffer(scp, arr, min_t(u32, alloc_len, rep_len)), so the host still receives exactly the alloc_len bytes it asked for. There is no functional change for alloc_len >= 64. This supersedes the previous approach of rejecting a sub-header allocation length with a check condition, which would have broken those legitimate small-alloc_len probes. Verified on a v6.19 KASAN build: with scsi_debug loaded as zbc=host-managed, issuing REPORT ZONES(16) via SG_IO with alloc_len=32 triggers a KASAN slab-out-of-bounds write in resp_report_zones() before this change, and the same command produces no report once the clamp and allocation floor are applied. Reproduction requires CAP_SYS_RAWIO to submit the raw CDB. Fixes: 7db0e0c8190a ("scsi: scsi_debug: Fix buffer size of REPORT ZONES command") Cc: stable@vger.kernel.org Signed-off-by: Ibrahim Hashimov <security@auditcode.ai> Assisted-by: AuditCode-AI:2026.07 --- v2: address sashiko-bot review of v1 (https://lore.kernel.org/linux-scsi/20260709150631.45018-1-security@auditcode.ai/): rejecting a sub-header allocation length with a check condition violates the ZBC/SPC rule that allocation-length truncation is not an error and breaks legitimate small-alloc_len zone-list-length probes. Instead clamp rep_max_zones to zero and floor the allocation at the 64-byte header, letting the existing min(alloc_len, rep_len) copy-out return the truncated header. No functional change for alloc_len >= 64. drivers/scsi/scsi_debug.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c index 9d1c9c41d0f9..a21d76fe35f6 100644 --- a/drivers/scsi/scsi_debug.c +++ b/drivers/scsi/scsi_debug.c @@ -5911,9 +5911,11 @@ static int resp_report_zones(struct scsi_cmnd *scp, return check_condition_result; } - rep_max_zones = (alloc_len - 64) >> ilog2(RZONES_DESC_HD); + rep_max_zones = (alloc_len < RZONES_DESC_HD) ? 0 : + (alloc_len - RZONES_DESC_HD) >> ilog2(RZONES_DESC_HD); - arr = kzalloc(alloc_len, GFP_ATOMIC | __GFP_NOWARN); + arr = kzalloc(max_t(u32, alloc_len, RZONES_DESC_HD), + GFP_ATOMIC | __GFP_NOWARN); if (!arr) { mk_sense_buffer(scp, ILLEGAL_REQUEST, INSUFF_RES_ASC, INSUFF_RES_ASCQ); -- 2.50.1 (Apple Git-155) ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v2] scsi: scsi_debug: fix REPORT ZONES alloc_len underflow OOB write 2026-07-09 19:48 ` [PATCH v2] " Ibrahim Hashimov @ 2026-07-10 0:58 ` Damien Le Moal 2026-07-10 5:57 ` [PATCH v3] " Ibrahim Hashimov 2026-07-10 5:59 ` [PATCH v2] " Ibrahim Hashimov 0 siblings, 2 replies; 13+ messages in thread From: Damien Le Moal @ 2026-07-10 0:58 UTC (permalink / raw) To: Ibrahim Hashimov, martin.petersen, James.Bottomley Cc: shinichiro.kawasaki, damien.lemoal, linux-scsi, linux-kernel, stable On 2026/07/10 4:48, Ibrahim Hashimov wrote: > resp_report_zones() reads the REPORT ZONES(16) ALLOCATION LENGTH field > (cmd[10..13]) into the unsigned alloc_len and, apart from the > alloc_len == 0 fast path, uses it without flooring it against the > 64-byte report header: > > rep_max_zones = (alloc_len - 64) >> ilog2(RZONES_DESC_HD); > arr = kzalloc(alloc_len, GFP_ATOMIC | __GFP_NOWARN); > ... > desc = arr + 64; > > For any alloc_len in the range 1..63, alloc_len - 64 wraps around > (alloc_len and rep_max_zones are unsigned), so rep_max_zones becomes a > huge value instead of zero. At the same time arr is allocated with the > raw alloc_len, which is smaller than the 64-byte header the function > always builds, and desc is set to arr + 64, already past the end of the > allocation. The report header stores (put_unaligned_be32 at arr+0, > put_unaligned_be64 at arr+8 and arr+16) can then run past a sub-24-byte > buffer, and the per-zone descriptor loop, no longer bounded by the > inflated rep_max_zones, writes 64-byte descriptors from desc onward, > producing a slab out-of-bounds write. > > Fix it the way ZBC and SPC require: allocation length truncation is not > an error, and a small alloc_len is a legitimate probe a host uses to > read the zone list length before allocating a full buffer. Clamp > rep_max_zones to zero when alloc_len is below the header size so no > descriptor is emitted, and size the allocation to at least the header > so the unconditional 64-byte header build cannot overflow. The existing > copy-out already truncates the result with > fill_from_dev_buffer(scp, arr, min_t(u32, alloc_len, rep_len)), so the > host still receives exactly the alloc_len bytes it asked for. There is > no functional change for alloc_len >= 64. > > This supersedes the previous approach of rejecting a sub-header > allocation length with a check condition, which would have broken those > legitimate small-alloc_len probes. > > Verified on a v6.19 KASAN build: with scsi_debug loaded as > zbc=host-managed, issuing REPORT ZONES(16) via SG_IO with alloc_len=32 > triggers a KASAN slab-out-of-bounds write in resp_report_zones() > before this change, and the same command produces no report once the > clamp and allocation floor are applied. Reproduction requires > CAP_SYS_RAWIO to submit the raw CDB. > > Fixes: 7db0e0c8190a ("scsi: scsi_debug: Fix buffer size of REPORT ZONES command") > Cc: stable@vger.kernel.org > Signed-off-by: Ibrahim Hashimov <security@auditcode.ai> > Assisted-by: AuditCode-AI:2026.07 > --- > v2: address sashiko-bot review of v1 > (https://lore.kernel.org/linux-scsi/20260709150631.45018-1-security@auditcode.ai/): > rejecting a sub-header allocation length with a check condition violates the > ZBC/SPC rule that allocation-length truncation is not an error and breaks > legitimate small-alloc_len zone-list-length probes. Instead clamp rep_max_zones > to zero and floor the allocation at the 64-byte header, letting the existing > min(alloc_len, rep_len) copy-out return the truncated header. No functional > change for alloc_len >= 64. > drivers/scsi/scsi_debug.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c > index 9d1c9c41d0f9..a21d76fe35f6 100644 > --- a/drivers/scsi/scsi_debug.c > +++ b/drivers/scsi/scsi_debug.c > @@ -5911,9 +5911,11 @@ static int resp_report_zones(struct scsi_cmnd *scp, > return check_condition_result; > } > > - rep_max_zones = (alloc_len - 64) >> ilog2(RZONES_DESC_HD); > + rep_max_zones = (alloc_len < RZONES_DESC_HD) ? 0 : > + (alloc_len - RZONES_DESC_HD) >> ilog2(RZONES_DESC_HD); Please expend this using an actual if: if (alloc_len < RZONES_DESC_HD) rep_max_zones = 0; else rep_max_zones = (alloc_len - RZONES_DESC_HD) >> ilog2(RZONES_DESC_HD); > > - arr = kzalloc(alloc_len, GFP_ATOMIC | __GFP_NOWARN); > + arr = kzalloc(max_t(u32, alloc_len, RZONES_DESC_HD), > + GFP_ATOMIC | __GFP_NOWARN); Yes, but this only partly address the issue. E.g. if the command has an allocation length of 64+32, rep_max_zones will incorrectly be 0, leaving the 32B after the header unfilled. Sure, that is a "useless" case since no one wants a partial zone descriptor. But the SCSI specs allow this, so let's do it correctly. I have a patch in my local queue that I was about to send to fix this, but you where faster :) What I did is: diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c index 4a95e6bae38b..96a5a0af4564 100644 --- a/drivers/scsi/scsi_debug.c +++ b/drivers/scsi/scsi_debug.c @@ -5842,6 +5842,7 @@ static int resp_report_zones(struct scsi_cmnd *scp, unsigned int rep_max_zones, nrz = 0; int ret = 0; u32 alloc_len, rep_opts, rep_len; + u64 arr_len; bool partial; u64 lba, zs_lba; u8 *arr = NULL, *desc; @@ -5865,9 +5866,11 @@ static int resp_report_zones(struct scsi_cmnd *scp, return check_condition_result; } - rep_max_zones = (alloc_len - 64) >> ilog2(RZONES_DESC_HD); + rep_max_zones = + (ALIGN(alloc_len, RZONES_DESC_HD) - RZONES_DESC_HD) >> + ilog2(RZONES_DESC_HD); + arr_len = RZONES_DESC_HD * (rep_max_zones + 1); - arr = kzalloc(alloc_len, GFP_ATOMIC | __GFP_NOWARN); + arr = kzalloc(arr_len, GFP_ATOMIC | __GFP_NOWARN); if (!arr) { mk_sense_buffer(scp, ILLEGAL_REQUEST, INSUFF_RES_ASC, INSUFF_RES_ASCQ); With that, the buffer arr is always big enough to generate a correct report with no overflows, but at the end, we only transfer alloc_len, which may be less than arr_len. -- Damien Le Moal Western Digital Research ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v3] scsi: scsi_debug: fix REPORT ZONES alloc_len underflow OOB write 2026-07-10 0:58 ` Damien Le Moal @ 2026-07-10 5:57 ` Ibrahim Hashimov 2026-07-10 6:03 ` Damien Le Moal ` (2 more replies) 2026-07-10 5:59 ` [PATCH v2] " Ibrahim Hashimov 1 sibling, 3 replies; 13+ messages in thread From: Ibrahim Hashimov @ 2026-07-10 5:57 UTC (permalink / raw) To: martin.petersen, James.Bottomley, dlemoal Cc: shinichiro.kawasaki, damien.lemoal, linux-scsi, linux-kernel, stable resp_report_zones() derives the number of zone descriptors that fit in the reply buffer from the command allocation length: rep_max_zones = (alloc_len - 64) >> ilog2(RZONES_DESC_HD); arr = kzalloc(alloc_len, GFP_ATOMIC | __GFP_NOWARN); alloc_len is taken directly from the CDB and is fully controlled by the initiator. When alloc_len is smaller than the 64-byte report header (RZONES_DESC_HD), the subtraction underflows and rep_max_zones becomes a huge value. The buffer is then allocated with only alloc_len bytes, which is smaller than the 64-byte header the code unconditionally writes, and the descriptor loop is bounded by the bogus rep_max_zones. Both the header store and the following zone descriptors are then written past the end of the undersized allocation, corrupting adjacent slab memory. Fix it by sizing the buffer to a whole number of 64-byte blocks that cover the requested allocation length: rep_max_zones = (ALIGN((u64)alloc_len, RZONES_DESC_HD) - RZONES_DESC_HD) >> ilog2(RZONES_DESC_HD); arr_len = (u64)RZONES_DESC_HD * (rep_max_zones + 1); arr = kzalloc(arr_len, GFP_ATOMIC | __GFP_NOWARN); RZONES_DESC_HD is a power of two, so ALIGN() rounds alloc_len up to the next multiple of 64 and rep_max_zones can no longer underflow: for any alloc_len of 1 to 64 it is 0, so only the header is built. arr_len is always RZONES_DESC_HD * (rep_max_zones + 1), which is exactly large enough for the header plus every descriptor the loop may write, so the report is always assembled within bounds, including a possibly partial trailing zone descriptor. The existing copy-out still transfers only what the host asked for: fill_from_dev_buffer(scp, arr, min_t(u32, alloc_len, rep_len)); so an allocation length that ends in the middle of a zone descriptor returns the correctly truncated partial descriptor, as permitted by the SCSI/ZBC specifications, while never reading past arr_len. The aligned length and the buffer size are computed in 64-bit (alloc_len is cast to u64 before ALIGN and the size product uses a u64 block size) so a crafted allocation length near U32_MAX cannot wrap them to a small value; such a request simply fails the large allocation and returns a check condition instead of overflowing the buffer. This was found by static analysis. A KASAN slab-out-of-bounds runtime reproduction of the original underflow is being re-run against the ALIGN based fix and will be reported separately. Fixes: 7db0e0c8190a ("scsi: scsi_debug: Fix buffer size of REPORT ZONES command") Suggested-by: Damien Le Moal <dlemoal@kernel.org> Cc: stable@vger.kernel.org Signed-off-by: Ibrahim Hashimov <security@auditcode.ai> Assisted-by: AuditCode-AI:2026.07 --- v3: adopt Damien Le Moal's ALIGN-based buffer sizing (Suggested-by) so a partial trailing zone descriptor is filled and returned per the SCSI/ZBC specs; v2 emitted only the report header for allocation lengths of 65..127 bytes. The buffer is sized to a whole number of 64-byte blocks covering alloc_len; the existing min(alloc_len, rep_len) copy-out still truncates the transfer to the requested length. Computed in 64-bit to avoid a u32 wrap of the aligned length/size for allocation lengths near U32_MAX (which would otherwise reintroduce the overflow). v2: https://lore.kernel.org/linux-scsi/20260709194824.50777-1-security@auditcode.ai/ drivers/scsi/scsi_debug.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c index 9d1c9c41d0f9..12e5a8624511 100644 --- a/drivers/scsi/scsi_debug.c +++ b/drivers/scsi/scsi_debug.c @@ -5890,6 +5890,7 @@ static int resp_report_zones(struct scsi_cmnd *scp, u32 alloc_len, rep_opts, rep_len; bool partial; u64 lba, zs_lba; + u64 arr_len; u8 *arr = NULL, *desc; u8 *cmd = scp->cmnd; struct sdeb_zone_state *zsp = NULL; @@ -5911,9 +5912,11 @@ static int resp_report_zones(struct scsi_cmnd *scp, return check_condition_result; } - rep_max_zones = (alloc_len - 64) >> ilog2(RZONES_DESC_HD); + rep_max_zones = (ALIGN((u64)alloc_len, RZONES_DESC_HD) - RZONES_DESC_HD) >> + ilog2(RZONES_DESC_HD); + arr_len = (u64)RZONES_DESC_HD * (rep_max_zones + 1); - arr = kzalloc(alloc_len, GFP_ATOMIC | __GFP_NOWARN); + arr = kzalloc(arr_len, GFP_ATOMIC | __GFP_NOWARN); if (!arr) { mk_sense_buffer(scp, ILLEGAL_REQUEST, INSUFF_RES_ASC, INSUFF_RES_ASCQ); -- 2.50.1 (Apple Git-155) ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v3] scsi: scsi_debug: fix REPORT ZONES alloc_len underflow OOB write 2026-07-10 5:57 ` [PATCH v3] " Ibrahim Hashimov @ 2026-07-10 6:03 ` Damien Le Moal 2026-07-10 6:05 ` sashiko-bot 2026-07-12 18:37 ` [PATCH v4] " Ibrahim Hashimov 2 siblings, 0 replies; 13+ messages in thread From: Damien Le Moal @ 2026-07-10 6:03 UTC (permalink / raw) To: Ibrahim Hashimov, martin.petersen, James.Bottomley Cc: shinichiro.kawasaki, damien.lemoal, linux-scsi, linux-kernel, stable On 7/10/26 14:57, Ibrahim Hashimov wrote: > resp_report_zones() derives the number of zone descriptors that fit in > the reply buffer from the command allocation length: > > rep_max_zones = (alloc_len - 64) >> ilog2(RZONES_DESC_HD); > > arr = kzalloc(alloc_len, GFP_ATOMIC | __GFP_NOWARN); > > alloc_len is taken directly from the CDB and is fully controlled by the > initiator. When alloc_len is smaller than the 64-byte report header > (RZONES_DESC_HD), the subtraction underflows and rep_max_zones becomes a > huge value. The buffer is then allocated with only alloc_len bytes, which > is smaller than the 64-byte header the code unconditionally writes, and > the descriptor loop is bounded by the bogus rep_max_zones. Both the header > store and the following zone descriptors are then written past the end of > the undersized allocation, corrupting adjacent slab memory. > > Fix it by sizing the buffer to a whole number of 64-byte blocks that > cover the requested allocation length: > > rep_max_zones = > (ALIGN((u64)alloc_len, RZONES_DESC_HD) - RZONES_DESC_HD) > >> ilog2(RZONES_DESC_HD); > arr_len = (u64)RZONES_DESC_HD * (rep_max_zones + 1); > > arr = kzalloc(arr_len, GFP_ATOMIC | __GFP_NOWARN); > > RZONES_DESC_HD is a power of two, so ALIGN() rounds alloc_len up to the > next multiple of 64 and rep_max_zones can no longer underflow: for any > alloc_len of 1 to 64 it is 0, so only the header is built. arr_len is > always RZONES_DESC_HD * (rep_max_zones + 1), which is exactly large enough > for the header plus every descriptor the loop may write, so the report is > always assembled within bounds, including a possibly partial trailing > zone descriptor. The existing copy-out still transfers only what the host > asked for: > > fill_from_dev_buffer(scp, arr, min_t(u32, alloc_len, rep_len)); > > so an allocation length that ends in the middle of a zone descriptor > returns the correctly truncated partial descriptor, as permitted by the > SCSI/ZBC specifications, while never reading past arr_len. > > The aligned length and the buffer size are computed in 64-bit (alloc_len > is cast to u64 before ALIGN and the size product uses a u64 block size) so > a crafted allocation length near U32_MAX cannot wrap them to a small value; > such a request simply fails the large allocation and returns a check > condition instead of overflowing the buffer. > > This was found by static analysis. A KASAN slab-out-of-bounds runtime > reproduction of the original underflow is being re-run against the ALIGN > based fix and will be reported separately. > > Fixes: 7db0e0c8190a ("scsi: scsi_debug: Fix buffer size of REPORT ZONES command") > Suggested-by: Damien Le Moal <dlemoal@kernel.org> > Cc: stable@vger.kernel.org > Signed-off-by: Ibrahim Hashimov <security@auditcode.ai> > Assisted-by: AuditCode-AI:2026.07 Looks OK to me. Reviewed-by: Damien Le Moal <dlemoal@kernel.org> -- Damien Le Moal Western Digital Research ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3] scsi: scsi_debug: fix REPORT ZONES alloc_len underflow OOB write 2026-07-10 5:57 ` [PATCH v3] " Ibrahim Hashimov 2026-07-10 6:03 ` Damien Le Moal @ 2026-07-10 6:05 ` sashiko-bot 2026-07-10 14:37 ` Bart Van Assche 2026-07-12 18:37 ` [PATCH v4] " Ibrahim Hashimov 2 siblings, 1 reply; 13+ messages in thread From: sashiko-bot @ 2026-07-10 6:05 UTC (permalink / raw) To: Ibrahim Hashimov; +Cc: linux-scsi Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Integer truncation on 32-bit platforms leads to ZERO_SIZE_PTR out-of-bounds write and kernel panic -- --- Patch [1]: [PATCH v3] scsi: scsi_debug: fix REPORT ZONES alloc_len underflow OOB write --- commit 1e0d50f19491d296aa99221a898bda21deff84db Author: Ibrahim Hashimov <security@auditcode.ai> scsi: scsi_debug: fix REPORT ZONES alloc_len underflow OOB write This commit fixes an out-of-bounds write in resp_report_zones() by sizing the buffer to a whole number of 64-byte blocks that cover the requested allocation length. It aligns the length and computes the buffer size in 64-bit to avoid a u32 wrap. > diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c > --- a/drivers/scsi/scsi_debug.c > +++ b/drivers/scsi/scsi_debug.c > @@ -5911,9 +5912,11 @@ static int resp_report_zones(struct scsi_cmnd *scp, > return check_condition_result; > } > > - rep_max_zones = (alloc_len - 64) >> ilog2(RZONES_DESC_HD); > + rep_max_zones = (ALIGN((u64)alloc_len, RZONES_DESC_HD) - RZONES_DESC_HD) >> > + ilog2(RZONES_DESC_HD); > + arr_len = (u64)RZONES_DESC_HD * (rep_max_zones + 1); > > - arr = kzalloc(alloc_len, GFP_ATOMIC | __GFP_NOWARN); > + arr = kzalloc(arr_len, GFP_ATOMIC | __GFP_NOWARN); [Severity: High] Does this code cause a truncation and kernel panic on 32-bit systems? If a user with raw IO access sends a REPORT ZONES SCSI command with an alloc_len between 0xFFFFFFC1 and 0xFFFFFFFF, ALIGN() rounds it up to exactly 0x100000000 (4 GB) making arr_len exactly 0x100000000. On 32-bit platforms, kzalloc() takes a 32-bit size_t, which silently truncates 0x100000000 to 0. Calling kzalloc(0) returns ZERO_SIZE_PTR, which bypasses the (!arr) NULL check. > if (!arr) { > mk_sense_buffer(scp, ILLEGAL_REQUEST, INSUFF_RES_ASC, > INSUFF_RES_ASCQ); > return check_condition_result; > } > > sdeb_meta_read_lock(sip); > > desc = arr + 64; Since arr is ZERO_SIZE_PTR (typically 0x10), desc becomes an unmapped address (0x50). [ ... ] > if (nrz < rep_max_zones) { > /* Fill zone descriptor */ > desc[0] = zsp->z_type; > desc[1] = zsp->z_cond << 4; The driver unconditionally dereferences the unmapped memory here, which will trigger a deterministic kernel panic. Could we add a bounds check for alloc_len to ensure it doesn't exceed 32-bit bounds or a reasonable maximum allocation size before passing it to kzalloc()? -- Sashiko AI review · https://sashiko.dev/#/patchset/20260710055755.53830-1-security@auditcode.ai?part=1 ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3] scsi: scsi_debug: fix REPORT ZONES alloc_len underflow OOB write 2026-07-10 6:05 ` sashiko-bot @ 2026-07-10 14:37 ` Bart Van Assche 2026-07-12 18:36 ` Ibrahim Hashimov 0 siblings, 1 reply; 13+ messages in thread From: Bart Van Assche @ 2026-07-10 14:37 UTC (permalink / raw) To: sashiko-reviews, Ibrahim Hashimov; +Cc: linux-scsi On 7/9/26 11:05 PM, sashiko-bot@kernel.org wrote: > On 32-bit platforms, kzalloc() takes a 32-bit size_t, which silently > truncates 0x100000000 to 0. Calling kzalloc(0) returns ZERO_SIZE_PTR, which > bypasses the (!arr) NULL check. Ibrahim, do you plan to address this finding? Thanks, Bart. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3] scsi: scsi_debug: fix REPORT ZONES alloc_len underflow OOB write 2026-07-10 14:37 ` Bart Van Assche @ 2026-07-12 18:36 ` Ibrahim Hashimov 0 siblings, 0 replies; 13+ messages in thread From: Ibrahim Hashimov @ 2026-07-12 18:36 UTC (permalink / raw) To: bvanassche Cc: dlemoal, martin.petersen, James.Bottomley, shinichiro.kawasaki, linux-scsi, linux-kernel On 7/10/26, Bart Van Assche wrote: > Ibrahim, do you plan to address this finding? Yes -- addressed in v4, which I will send shortly. The v3 ALIGN() can round an alloc_len near U32_MAX up to 0x100000000, and on 32-bit that 4 GB arr_len truncates to 0 in kzalloc()'s size_t, returning ZERO_SIZE_PTR and slipping past the !arr check as the bot noted. v4 clamps rep_max_zones to devip->nr_zones, so arr_len is bounded by the real zone count and can never reach 0x100000000 or truncate on 32-bit. The descriptor loop already stops at sdebug_capacity, so the clamp does not change any report -- it is purely an additive bound, and Damien's ALIGN sizing is untouched. Thanks for catching the 32-bit corner. Ibrahim ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v4] scsi: scsi_debug: fix REPORT ZONES alloc_len underflow OOB write 2026-07-10 5:57 ` [PATCH v3] " Ibrahim Hashimov 2026-07-10 6:03 ` Damien Le Moal 2026-07-10 6:05 ` sashiko-bot @ 2026-07-12 18:37 ` Ibrahim Hashimov 2026-07-13 5:43 ` Damien Le Moal 2026-07-13 17:12 ` Bart Van Assche 2 siblings, 2 replies; 13+ messages in thread From: Ibrahim Hashimov @ 2026-07-12 18:37 UTC (permalink / raw) To: martin.petersen, James.Bottomley, dlemoal Cc: bvanassche, shinichiro.kawasaki, damien.lemoal, linux-scsi, linux-kernel, stable resp_report_zones() sizes the reply buffer from the CDB allocation length. The v3 fix rounds alloc_len up with ALIGN() before deriving the descriptor count: rep_max_zones = (ALIGN((u64)alloc_len, RZONES_DESC_HD) - RZONES_DESC_HD) >> ilog2(RZONES_DESC_HD); arr_len = (u64)RZONES_DESC_HD * (rep_max_zones + 1); For alloc_len in 0xFFFFFFC1..0xFFFFFFFF, ALIGN() rounds up to 0x100000000, so arr_len is 4 GB. On 32-bit, kzalloc()'s size_t is 32-bit and truncates 0x100000000 to 0; kzalloc(0) returns ZERO_SIZE_PTR, which passes the !arr check, and desc = arr + 64 is then dereferenced in the loop -> out-of-bounds write / panic. Clamp rep_max_zones to devip->nr_zones. The loop already stops at sdebug_capacity (after nr_zones zones), so a report can never hold more than nr_zones descriptors; the clamp does not change the report, it only bounds arr_len to (nr_zones + 1) * RZONES_DESC_HD, a real device property that can never reach 0x100000000. Fixes: 7db0e0c8190a ("scsi: scsi_debug: Fix buffer size of REPORT ZONES command") Suggested-by: Damien Le Moal <dlemoal@kernel.org> Cc: stable@vger.kernel.org Signed-off-by: Ibrahim Hashimov <security@auditcode.ai> Assisted-by: AuditCode-AI:2026.07 --- v4: clamp rep_max_zones to the device zone count (devip->nr_zones) so arr_len cannot reach 0x100000000 and be truncated by kzalloc()'s 32-bit size_t on 32-bit platforms (which returns ZERO_SIZE_PTR and bypasses the NULL check), as reported by sashiko-bot / Bart Van Assche on v3. This restores a bound that pre-dated the loop refactor: the original REPORT ZONES code already capped rep_max_zones at the device zone count. The clamp is additive and does not change any report (the loop already stops at sdebug_capacity), so it only bounds the allocation. Dropped Damien's v3 Reviewed-by since v4 adds a functional line he has not reviewed; his ALIGN sizing is otherwise unchanged. v3: https://lore.kernel.org/linux-scsi/20260710055755.53830-1-security@auditcode.ai/ drivers/scsi/scsi_debug.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c index 9d1c9c41d0f9..643051332132 100644 --- a/drivers/scsi/scsi_debug.c +++ b/drivers/scsi/scsi_debug.c @@ -5890,6 +5890,7 @@ static int resp_report_zones(struct scsi_cmnd *scp, u32 alloc_len, rep_opts, rep_len; bool partial; u64 lba, zs_lba; + u64 arr_len; u8 *arr = NULL, *desc; u8 *cmd = scp->cmnd; struct sdeb_zone_state *zsp = NULL; @@ -5911,9 +5912,12 @@ static int resp_report_zones(struct scsi_cmnd *scp, return check_condition_result; } - rep_max_zones = (alloc_len - 64) >> ilog2(RZONES_DESC_HD); + rep_max_zones = (ALIGN((u64)alloc_len, RZONES_DESC_HD) - RZONES_DESC_HD) >> + ilog2(RZONES_DESC_HD); + rep_max_zones = min_t(unsigned int, rep_max_zones, devip->nr_zones); + arr_len = (u64)RZONES_DESC_HD * (rep_max_zones + 1); - arr = kzalloc(alloc_len, GFP_ATOMIC | __GFP_NOWARN); + arr = kzalloc(arr_len, GFP_ATOMIC | __GFP_NOWARN); if (!arr) { mk_sense_buffer(scp, ILLEGAL_REQUEST, INSUFF_RES_ASC, INSUFF_RES_ASCQ); -- 2.50.1 (Apple Git-155) ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v4] scsi: scsi_debug: fix REPORT ZONES alloc_len underflow OOB write 2026-07-12 18:37 ` [PATCH v4] " Ibrahim Hashimov @ 2026-07-13 5:43 ` Damien Le Moal 2026-07-13 17:12 ` Bart Van Assche 1 sibling, 0 replies; 13+ messages in thread From: Damien Le Moal @ 2026-07-13 5:43 UTC (permalink / raw) To: Ibrahim Hashimov, martin.petersen, James.Bottomley Cc: bvanassche, shinichiro.kawasaki, damien.lemoal, linux-scsi, linux-kernel, stable On 7/13/26 03:37, Ibrahim Hashimov wrote: > resp_report_zones() sizes the reply buffer from the CDB allocation > length. The v3 fix rounds alloc_len up with ALIGN() before deriving the > descriptor count: > > rep_max_zones = (ALIGN((u64)alloc_len, RZONES_DESC_HD) - > RZONES_DESC_HD) >> ilog2(RZONES_DESC_HD); > arr_len = (u64)RZONES_DESC_HD * (rep_max_zones + 1); > > For alloc_len in 0xFFFFFFC1..0xFFFFFFFF, ALIGN() rounds up to > 0x100000000, so arr_len is 4 GB. On 32-bit, kzalloc()'s size_t is > 32-bit and truncates 0x100000000 to 0; kzalloc(0) returns > ZERO_SIZE_PTR, which passes the !arr check, and desc = arr + 64 is then > dereferenced in the loop -> out-of-bounds write / panic. > > Clamp rep_max_zones to devip->nr_zones. The loop already stops at > sdebug_capacity (after nr_zones zones), so a report can never hold more > than nr_zones descriptors; the clamp does not change the report, it > only bounds arr_len to (nr_zones + 1) * RZONES_DESC_HD, a real device > property that can never reach 0x100000000. > > Fixes: 7db0e0c8190a ("scsi: scsi_debug: Fix buffer size of REPORT ZONES command") > Suggested-by: Damien Le Moal <dlemoal@kernel.org> > Cc: stable@vger.kernel.org > Signed-off-by: Ibrahim Hashimov <security@auditcode.ai> > Assisted-by: AuditCode-AI:2026.07 Looks good. Reviewed-by: Damien Le Moal <dlemoal@kernel.org> -- Damien Le Moal Western Digital Research ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v4] scsi: scsi_debug: fix REPORT ZONES alloc_len underflow OOB write 2026-07-12 18:37 ` [PATCH v4] " Ibrahim Hashimov 2026-07-13 5:43 ` Damien Le Moal @ 2026-07-13 17:12 ` Bart Van Assche 1 sibling, 0 replies; 13+ messages in thread From: Bart Van Assche @ 2026-07-13 17:12 UTC (permalink / raw) To: Ibrahim Hashimov, martin.petersen, James.Bottomley, dlemoal Cc: shinichiro.kawasaki, damien.lemoal, linux-scsi, linux-kernel, stable On 7/12/26 11:37 AM, Ibrahim Hashimov wrote: > Clamp rep_max_zones to devip->nr_zones. The loop already stops at > sdebug_capacity (after nr_zones zones), so a report can never hold more > than nr_zones descriptors; the clamp does not change the report, it > only bounds arr_len to (nr_zones + 1) * RZONES_DESC_HD, a real device > property that can never reach 0x100000000. Reviewed-by: Bart Van Assche <bvanassche@acm.org> ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2] scsi: scsi_debug: fix REPORT ZONES alloc_len underflow OOB write 2026-07-10 0:58 ` Damien Le Moal 2026-07-10 5:57 ` [PATCH v3] " Ibrahim Hashimov @ 2026-07-10 5:59 ` Ibrahim Hashimov 1 sibling, 0 replies; 13+ messages in thread From: Ibrahim Hashimov @ 2026-07-10 5:59 UTC (permalink / raw) To: dlemoal Cc: martin.petersen, James.Bottomley, shinichiro.kawasaki, damien.lemoal, linux-scsi, linux-kernel On 2026/07/10 09:58, Damien Le Moal wrote: > Yes, but this only partly address the issue. E.g. if the command has an > allocation length of 64+32, rep_max_zones will incorrectly be 0, leaving > the 32B after the header unfilled. Sure, that is a "useless" case since no > one wants a partial zone descriptor. But the SCSI specs allow this, so > let's do it correctly. > > I have a patch in my local queue that I was about to send to fix this, but > you where faster :) What I did is: > [...] Thanks Damien, that makes sense. v3 adopts your ALIGN-based sizing, so a partial trailing zone descriptor is now built and returned correctly (Suggested-by: you). While verifying it I noticed one corner in that snippet: ALIGN(alloc_len, 64) and the 64 * (rep_max_zones + 1) size product are both evaluated in 32-bit. alloc_len is the full 32-bit CDB allocation length, so a value just below U32_MAX (e.g. 0xFFFFFFF0) wraps the aligned length and the size back down to a tiny/zero value: kzalloc() then returns a ZERO_SIZE_PTR / tiny buffer while rep_max_zones is huge, which reintroduces the same out-of-bounds write. v3 does the ALIGN and the size computation in 64-bit (cast alloc_len to u64, u64 block size), so that case just fails the large allocation and returns a check condition instead. Verified under KASAN: alloc_len=96 now returns the header plus a 32-byte partial descriptor, and alloc_len=0xFFFFFFF0 returns INSUFF_RES with no KASAN report. Flagging it in case the copy of this pattern in your queued patch has the same corner. Thanks again for the review, Ibrahim ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2026-07-13 17:12 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-07-09 15:06 [PATCH] scsi: scsi_debug: fix REPORT ZONES alloc_len underflow OOB write Ibrahim Hashimov 2026-07-09 15:18 ` sashiko-bot 2026-07-09 19:48 ` [PATCH v2] " Ibrahim Hashimov 2026-07-10 0:58 ` Damien Le Moal 2026-07-10 5:57 ` [PATCH v3] " Ibrahim Hashimov 2026-07-10 6:03 ` Damien Le Moal 2026-07-10 6:05 ` sashiko-bot 2026-07-10 14:37 ` Bart Van Assche 2026-07-12 18:36 ` Ibrahim Hashimov 2026-07-12 18:37 ` [PATCH v4] " Ibrahim Hashimov 2026-07-13 5:43 ` Damien Le Moal 2026-07-13 17:12 ` Bart Van Assche 2026-07-10 5:59 ` [PATCH v2] " Ibrahim Hashimov
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox