The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH] scsi: scsi_debug: fix REPORT ZONES alloc_len underflow OOB write
@ 2026-07-09 15:06 Ibrahim Hashimov
  2026-07-09 19:48 ` [PATCH v2] " Ibrahim Hashimov
  0 siblings, 1 reply; 6+ 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] 6+ 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 19:48 ` Ibrahim Hashimov
  2026-07-10  0:58   ` Damien Le Moal
  0 siblings, 1 reply; 6+ 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] 6+ 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; 6+ 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] 6+ 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
  2026-07-10  5:59     ` [PATCH v2] " Ibrahim Hashimov
  1 sibling, 1 reply; 6+ 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] 6+ 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; 6+ 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] 6+ 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
  0 siblings, 0 replies; 6+ 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] 6+ messages in thread

end of thread, other threads:[~2026-07-10  6:03 UTC | newest]

Thread overview: 6+ 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 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  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