[PATCH] scsi: scsi_debug: reject too-small REPORT ZONES buffers

[PATCH] scsi: scsi_debug: reject too-small REPORT ZONES buffers

[Samuel Moelius]

REPORT ZONES subtracts the response header size from the allocation
length before ensuring that the allocation is large enough.  A short
allocation can underflow and make the remaining length look huge.

The handler can then write zone descriptors past the caller-provided
response buffer.

Validate the allocation length before subtracting the header size.

Assisted-by: Codex:gpt-5.5-cyber-preview
Signed-off-by: Samuel Moelius <sam.moelius@trailofbits.com>
---
 drivers/scsi/scsi_debug.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 1515495fd9ea..f17e59482cfc 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -5911,6 +5911,10 @@ static int resp_report_zones(struct scsi_cmnd *scp,
 	alloc_len = get_unaligned_be32(cmd + 10);
 	if (alloc_len == 0)
 		return 0;	/* not an error */
+	if (alloc_len < RZONES_DESC_HD) {
+		mk_sense_buffer(scp, ILLEGAL_REQUEST, INVALID_FIELD_IN_CDB, 0);
+		return check_condition_result;
+	}
 	rep_opts = cmd[14] & 0x3f;
 	partial = cmd[14] & 0x80;
 
-- 
2.43.0

Re: [PATCH] scsi: scsi_debug: reject too-small REPORT ZONES buffers

[James Bottomley]

On Wed, 2026-06-03 at 17:11 +0000, Samuel Moelius wrote:
> REPORT ZONES subtracts the response header size from the allocation
> length before ensuring that the allocation is large enough.  A short
> allocation can underflow and make the remaining length look huge.
> 
> The handler can then write zone descriptors past the caller-provided
> response buffer.
> 
> Validate the allocation length before subtracting the header size.
> 
> Assisted-by: Codex:gpt-5.5-cyber-preview
> Signed-off-by: Samuel Moelius <sam.moelius@trailofbits.com>
> ---
>  drivers/scsi/scsi_debug.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
> index 1515495fd9ea..f17e59482cfc 100644
> --- a/drivers/scsi/scsi_debug.c
> +++ b/drivers/scsi/scsi_debug.c
> @@ -5911,6 +5911,10 @@ static int resp_report_zones(struct scsi_cmnd
> *scp,
>  	alloc_len = get_unaligned_be32(cmd + 10);
>  	if (alloc_len == 0)
>  		return 0;	/* not an error */
> +	if (alloc_len < RZONES_DESC_HD) {
> +		mk_sense_buffer(scp, ILLEGAL_REQUEST,
> INVALID_FIELD_IN_CDB, 0);
> +		return check_condition_result;

That doesn't look right.  The returned length is almost always the
first parameter of a SCSI command (it is in this case) and a lot of
users will send a buffer just big enough for the length (4 bytes in
this case) to get the actual length before they ask for the whole
thing.  If you require 64 bytes, we're going to reject perfectly legal
requests.

Regards,

James

Re: [PATCH] scsi: scsi_debug: reject too-small REPORT ZONES buffers

[Bart Van Assche]

On 6/3/26 12:11 PM, James Bottomley wrote:
> On Wed, 2026-06-03 at 17:11 +0000, Samuel Moelius wrote:
>> REPORT ZONES subtracts the response header size from the allocation
>> length before ensuring that the allocation is large enough.  A short
>> allocation can underflow and make the remaining length look huge.
>>
>> The handler can then write zone descriptors past the caller-provided
>> response buffer.
>>
>> Validate the allocation length before subtracting the header size.
>>
>> Assisted-by: Codex:gpt-5.5-cyber-preview
>> Signed-off-by: Samuel Moelius <sam.moelius@trailofbits.com>
>> ---
>>   drivers/scsi/scsi_debug.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
>> index 1515495fd9ea..f17e59482cfc 100644
>> --- a/drivers/scsi/scsi_debug.c
>> +++ b/drivers/scsi/scsi_debug.c
>> @@ -5911,6 +5911,10 @@ static int resp_report_zones(struct scsi_cmnd
>> *scp,
>>   	alloc_len = get_unaligned_be32(cmd + 10);
>>   	if (alloc_len == 0)
>>   		return 0;	/* not an error */
>> +	if (alloc_len < RZONES_DESC_HD) {
>> +		mk_sense_buffer(scp, ILLEGAL_REQUEST,
>> INVALID_FIELD_IN_CDB, 0);
>> +		return check_condition_result;
> 
> That doesn't look right.  The returned length is almost always the
> first parameter of a SCSI command (it is in this case) and a lot of
> users will send a buffer just big enough for the length (4 bytes in
> this case) to get the actual length before they ask for the whole
> thing.  If you require 64 bytes, we're going to reject perfectly legal
> requests.

Hi James,

Is my understanding of ZBC-1, ZBC-2 and ZBC-3 correct that the REPORT
ZONES parameter data should be at least 64 bytes long? The zone
descriptors list starts at offset 64. Bytes 0..63 form a header that
precedes the zone descriptors.

It seems to me that the resp_report_zones() implementation is already
based on the assumption that alloc_len >= 64 because of the following
statement:

	rep_max_zones = (alloc_len - 64) >> ilog2(RZONES_DESC_HD);

Thanks,

Bart.

Re: [PATCH] scsi: scsi_debug: reject too-small REPORT ZONES buffers

[Samuel Moelius]

On Wed, Jun 3, 2026 at 3:11 PM James Bottomley
<James.Bottomley@hansenpartnership.com> wrote:
>
> On Wed, 2026-06-03 at 17:11 +0000, Samuel Moelius wrote:
> > REPORT ZONES subtracts the response header size from the allocation
> > length before ensuring that the allocation is large enough.  A short
> > allocation can underflow and make the remaining length look huge.
> >
> > The handler can then write zone descriptors past the caller-provided
> > response buffer.
> >
> > Validate the allocation length before subtracting the header size.
> >
> > Assisted-by: Codex:gpt-5.5-cyber-preview
> > Signed-off-by: Samuel Moelius <sam.moelius@trailofbits.com>
> > ---
> >  drivers/scsi/scsi_debug.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
> > index 1515495fd9ea..f17e59482cfc 100644
> > --- a/drivers/scsi/scsi_debug.c
> > +++ b/drivers/scsi/scsi_debug.c
> > @@ -5911,6 +5911,10 @@ static int resp_report_zones(struct scsi_cmnd
> > *scp,
> >       alloc_len = get_unaligned_be32(cmd + 10);
> >       if (alloc_len == 0)
> >               return 0;       /* not an error */
> > +     if (alloc_len < RZONES_DESC_HD) {
> > +             mk_sense_buffer(scp, ILLEGAL_REQUEST,
> > INVALID_FIELD_IN_CDB, 0);
> > +             return check_condition_result;
>
> That doesn't look right.  The returned length is almost always the
> first parameter of a SCSI command (it is in this case) and a lot of
> users will send a buffer just big enough for the length (4 bytes in
> this case) to get the actual length before they ask for the whole
> thing.  If you require 64 bytes, we're going to reject perfectly legal
> requests.

This patch is meant to be superseded by:
https://lore.kernel.org/linux-scsi/20260603225239.102803-1-sam.moelius@trailofbits.com/T/#u

Apologies for not having said that explicitly.