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

[Damien Le Moal <dlemoal@kernel.org>]

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