scsi: scsi_debug: fix REPORT ZONES alloc_len underflow OOB write

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.

---
 drivers/scsi/scsi_debug.c | 5 +++++
 1 file changed, 5 insertions(+)

index 9d1c9c41d0f9..2fa887c65b61 100644
@@ -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)

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.

---
v2: address sashiko-bot review of v1
(https://lore.kernel.org/linux-scsi/[email protected]/):
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(-)

index 9d1c9c41d0f9..a21d76fe35f6 100644
@@ -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)

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.

---
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/[email protected]/

 drivers/scsi/scsi_debug.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

index 9d1c9c41d0f9..12e5a8624511 100644
@@ -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)

Looks OK to me.

Damien Le Moal
Western Digital Research

Ibrahim, do you plan to address this finding?

Thanks,

Bart.

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

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.

---
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/[email protected]/

 drivers/scsi/scsi_debug.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

index 9d1c9c41d0f9..643051332132 100644
@@ -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)

Looks good.

Damien Le Moal
Western Digital Research

Applied to 7.2/scsi-fixes, thanks!

[1/1] scsi: scsi_debug: fix REPORT ZONES alloc_len underflow OOB write

      https://git.kernel.org/mkp/scsi/c/93dde0bf2f39

Martin K. Petersen

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