[PATCH 0/2] scsi: scsi_debug: fix zone reset with virtual_gb and DIX

[PATCH 0/2] scsi: scsi_debug: fix zone reset with virtual_gb and DIX

[Tal Zussman]

Resetting a written zone on scsi_debug zeroes the zone's blocks in the
store, but does so with a raw offset that doesn't wrap around the store
like writes do, and without resetting the blocks' protection
information. With virtual_gb set, resetting a zone that starts past the
end of the store oopses. With DIX enabled, a reset zone can't be read
back. The first patch makes the reset wrap around the end of the store
like writes do. The second resets the protection information of the
zeroed blocks.

The fixes were proposed by an LLM.

---
Tal Zussman (2):
      scsi: scsi_debug: wrap around the store when zeroing a reset zone
      scsi: scsi_debug: reset protection information on zone reset

 drivers/scsi/scsi_debug.c | 30 ++++++++++++++++++++++++++++--
 1 file changed, 28 insertions(+), 2 deletions(-)
---
base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
change-id: 20261109-scsi-debug-zone-reset-14fc587c9d45

Best regards,
--  
Tal Zussman <tz2294@columbia.edu>

[PATCH 1/2] scsi: scsi_debug: wrap around the store when zeroing a reset zone

[Tal Zussman]

zbc_rwp_zone() zeroes the blocks of a zone that is reset, using the
start of the zone as an offset into the store. With virtual_gb set, the
device and its zones can extend past the end of the store, which is
only dev_size_mb in size. Writes wrap around in do_device_access(), but
the memset() doesn't, so resetting a zone that has been written to can
write past the end of the store.

On a device whose fifth zone starts exactly at the end of the store,
write to that zone and reset it:

  # modprobe scsi_debug zbc=managed sector_size=4096 dev_size_mb=16 \
        virtual_gb=1 zone_size_mb=4
  # dd if=/dev/urandom of=/dev/sda bs=1M count=1 seek=16 oflag=direct
  # blkzone reset -o 32768 -c 1 /dev/sda

The memset() starts at the first byte past the store, in the guard page
after the vmalloc'ed area, for the 1 MiB that was written:

BUG: unable to handle page fault for address: ffffc90002c00000
Oops: Oops: 0002 [#1] SMP KASAN NOPTI
RIP: 0010:memset_orig+0x33/0xb0
RDX: 0000000000100000 RSI: 0000000000000000 RDI: ffffc90002c00000
Call Trace:
 zbc_rwp_zone+0x1a7/0x2a0 [scsi_debug]
 resp_rwp_zone+0x19c/0x450 [scsi_debug]
 schedule_resp+0x2dd/0xbc0 [scsi_debug]
 scsi_debug_queuecommand+0x1286/0x2950 [scsi_debug]
 scsi_queue_rq+0x1d74/0x3200
 blk_mq_dispatch_rq_list+0x3ad/0x23c0

The command never completes, so the reset returns EIO to userspace and
the device is unusable afterwards.

Zero the blocks in a helper that wraps around at the end of the store,
and that stops after one pass over it for a zone that is larger than
the store.

Fixes: f0d1cf9378bd ("scsi: scsi_debug: Add ZBC zone commands")
Signed-off-by: Tal Zussman <tz2294@columbia.edu>
---
 drivers/scsi/scsi_debug.c | 26 ++++++++++++++++++++++++--
 1 file changed, 24 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index a922c8318a2f..1c27ea0f9b69 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -6612,6 +6612,29 @@ static int resp_finish_zone(struct scsi_cmnd *scp,
 	return res;
 }
 
+/*
+ * Zero the blocks of a zone that is being reset. The store can be smaller
+ * than the device, so wrap around at its end like do_device_access() does.
+ */
+static void zbc_zero_blocks(struct sdeb_store_info *sip, sector_t lba,
+			    sector_t num)
+{
+	/* One pass over the store zeroes all of it */
+	num = min_t(sector_t, num, sdebug_store_sectors);
+
+	while (num) {
+		sector_t block = lba;
+		u32 n;
+
+		block = sector_div(block, sdebug_store_sectors);
+		n = min_t(sector_t, num, sdebug_store_sectors - block);
+		memset(sip->storep + block * sdebug_sector_size, 0,
+		       (size_t)n * sdebug_sector_size);
+		lba += n;
+		num -= n;
+	}
+}
+
 static void zbc_rwp_zone(struct sdebug_dev_info *devip,
 			 struct sdeb_zone_state *zsp)
 {
@@ -6629,8 +6652,7 @@ static void zbc_rwp_zone(struct sdebug_dev_info *devip,
 		devip->nr_closed--;
 
 	if (zsp->z_wp > zsp->z_start)
-		memset(sip->storep + zsp->z_start * sdebug_sector_size, 0,
-		       (zsp->z_wp - zsp->z_start) * sdebug_sector_size);
+		zbc_zero_blocks(sip, zsp->z_start, zsp->z_wp - zsp->z_start);
 
 	zsp->z_non_seq_resource = false;
 	zsp->z_wp = zsp->z_start;

-- 
2.39.5

[PATCH 2/2] scsi: scsi_debug: reset protection information on zone reset

[Tal Zussman]

zbc_rwp_zone() zeroes the data of a zone that is reset, but leaves the
protection information of its previous contents in place. Reads of a
zoned device are not limited to the blocks below the write pointer, so
with DIX enabled reading those blocks back fails the guard check.

Write to a zone, reset it and read it back:

  # modprobe scsi_debug zbc=managed sector_size=4096 dev_size_mb=64 \
        zone_size_mb=4 dif=1 dix=1
  # dd if=/dev/urandom of=/dev/sda bs=1M count=1 seek=8 oflag=direct
  # blkzone reset -o 16384 -c 1 /dev/sda
  # dd if=/dev/sda of=/dev/null bs=1M count=1 skip=8 iflag=direct
  dd: error reading '/dev/sda': Invalid or incomplete multibyte or wide character

The read fails with EILSEQ, and the guard tag that the device reports
is still that of the data that was written, while the block now holds
zeroes:

scsi_debug:dif_verify: GUARD check failed on sector 2048 rcvd 0x288f, data 0x0000
protection error, dev sda, sector 16384 op 0x0:(READ) flags 0x10800 phys_seg 256 prio class 2

Reset the protection information of the zeroed blocks to 0xff, as
unmap_region() does, so that they are not checked until they are
written again.

Fixes: f0d1cf9378bd ("scsi: scsi_debug: Add ZBC zone commands")
Signed-off-by: Tal Zussman <tz2294@columbia.edu>
---
 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 1c27ea0f9b69..9eb64caebdf9 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -6613,8 +6613,9 @@ static int resp_finish_zone(struct scsi_cmnd *scp,
 }
 
 /*
- * Zero the blocks of a zone that is being reset. The store can be smaller
- * than the device, so wrap around at its end like do_device_access() does.
+ * Zero the blocks of a zone that is being reset, and reset their protection
+ * information. The store can be smaller than the device, so wrap around at
+ * its end like do_device_access() does.
  */
 static void zbc_zero_blocks(struct sdeb_store_info *sip, sector_t lba,
 			    sector_t num)
@@ -6630,6 +6631,9 @@ static void zbc_zero_blocks(struct sdeb_store_info *sip, sector_t lba,
 		n = min_t(sector_t, num, sdebug_store_sectors - block);
 		memset(sip->storep + block * sdebug_sector_size, 0,
 		       (size_t)n * sdebug_sector_size);
+		if (sip->dif_storep)
+			memset(sip->dif_storep + block, 0xff,
+			       n * sizeof(*sip->dif_storep));
 		lba += n;
 		num -= n;
 	}

-- 
2.39.5

Re: [PATCH 1/2] scsi: scsi_debug: wrap around the store when zeroing a reset zone

[Damien Le Moal]

On 2026/11/11 6:23, Tal Zussman wrote:
> zbc_rwp_zone() zeroes the blocks of a zone that is reset, using the
> start of the zone as an offset into the store. With virtual_gb set, the
> device and its zones can extend past the end of the store, which is
> only dev_size_mb in size. Writes wrap around in do_device_access(), but
> the memset() doesn't, so resetting a zone that has been written to can
> write past the end of the store.
> 
> On a device whose fifth zone starts exactly at the end of the store,
> write to that zone and reset it:
> 
>   # modprobe scsi_debug zbc=managed sector_size=4096 dev_size_mb=16 \
>         virtual_gb=1 zone_size_mb=4
>   # dd if=/dev/urandom of=/dev/sda bs=1M count=1 seek=16 oflag=direct
>   # blkzone reset -o 32768 -c 1 /dev/sda
> 
> The memset() starts at the first byte past the store, in the guard page
> after the vmalloc'ed area, for the 1 MiB that was written:
> 
> BUG: unable to handle page fault for address: ffffc90002c00000
> Oops: Oops: 0002 [#1] SMP KASAN NOPTI
> RIP: 0010:memset_orig+0x33/0xb0
> RDX: 0000000000100000 RSI: 0000000000000000 RDI: ffffc90002c00000
> Call Trace:
>  zbc_rwp_zone+0x1a7/0x2a0 [scsi_debug]
>  resp_rwp_zone+0x19c/0x450 [scsi_debug]
>  schedule_resp+0x2dd/0xbc0 [scsi_debug]
>  scsi_debug_queuecommand+0x1286/0x2950 [scsi_debug]
>  scsi_queue_rq+0x1d74/0x3200
>  blk_mq_dispatch_rq_list+0x3ad/0x23c0
> 
> The command never completes, so the reset returns EIO to userspace and
> the device is unusable afterwards.
> 
> Zero the blocks in a helper that wraps around at the end of the store,
> and that stops after one pass over it for a zone that is larger than
> the store.
> 
> Fixes: f0d1cf9378bd ("scsi: scsi_debug: Add ZBC zone commands")
> Signed-off-by: Tal Zussman <tz2294@columbia.edu>
> ---
>  drivers/scsi/scsi_debug.c | 26 ++++++++++++++++++++++++--
>  1 file changed, 24 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
> index a922c8318a2f..1c27ea0f9b69 100644
> --- a/drivers/scsi/scsi_debug.c
> +++ b/drivers/scsi/scsi_debug.c
> @@ -6612,6 +6612,29 @@ static int resp_finish_zone(struct scsi_cmnd *scp,
>  	return res;
>  }
>  
> +/*
> + * Zero the blocks of a zone that is being reset. The store can be smaller
> + * than the device, so wrap around at its end like do_device_access() does.
> + */
> +static void zbc_zero_blocks(struct sdeb_store_info *sip, sector_t lba,
> +			    sector_t num)
> +{
> +	/* One pass over the store zeroes all of it */
> +	num = min_t(sector_t, num, sdebug_store_sectors);

Why do you need to do this in a loop?
You can certainly calculate the total number of blocks that need zeroing and do
a single memset, no? What am I missing here?
> +
> +	while (num) {
> +		sector_t block = lba;
> +		u32 n;
> +
> +		block = sector_div(block, sdebug_store_sectors);
> +		n = min_t(sector_t, num, sdebug_store_sectors - block);
> +		memset(sip->storep + block * sdebug_sector_size, 0,
> +		       (size_t)n * sdebug_sector_size);
> +		lba += n;
> +		num -= n;
> +	}
> +}
> +
>  static void zbc_rwp_zone(struct sdebug_dev_info *devip,
>  			 struct sdeb_zone_state *zsp)
>  {
> @@ -6629,8 +6652,7 @@ static void zbc_rwp_zone(struct sdebug_dev_info *devip,
>  		devip->nr_closed--;
>  
>  	if (zsp->z_wp > zsp->z_start)
> -		memset(sip->storep + zsp->z_start * sdebug_sector_size, 0,
> -		       (zsp->z_wp - zsp->z_start) * sdebug_sector_size);
> +		zbc_zero_blocks(sip, zsp->z_start, zsp->z_wp - zsp->z_start);
>  
>  	zsp->z_non_seq_resource = false;
>  	zsp->z_wp = zsp->z_start;
> 


-- 
Damien Le Moal
Western Digital Research