Re: [PATCH v2 1/4] scsi: scsi_debug: Do not advance the write pointer of a failed write

[Niklas Cassel <cassel@kernel.org>]

On Thu, Sep 17, 2026 at 11:17:38AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] scsi: scsi_debug: ZBC specification violation by returning GOOD status without advancing WP
> - [Medium] scsi: scsi_debug: integer overflow in short write check allows incorrect WP advancement
> --
> 
> commit 2b6fea07514157218eac4ea45ff1ba17e05b5125
> Author: Niklas Cassel <cassel@kernel.org>
> 
> scsi: scsi_debug: Do not advance the write pointer of a failed write
> 
> This commit modifies the write pointer advancement logic for ZBC sequential
> write required zones. It ensures the write pointer is only incremented when
> all requested data is successfully written.
> 
> > diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
> > index 6941809dfdb75..3ce2dbdff177c 100644
> > --- a/drivers/scsi/scsi_debug.c
> > +++ b/drivers/scsi/scsi_debug.c
> > @@ -5150,8 +5150,8 @@ static int resp_write_dt0(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
> >  	if (unlikely(scsi_debug_lbp()))
> >  		map_region(sip, lba, num);
> >  
> > -	/* If ZBC zone then bump its write pointer */
> > -	if (sdebug_dev_is_zoned(devip))
> > +	/* If ZBC zone then bump its write pointer, if all data was written */
> > +	if (sdebug_dev_is_zoned(devip) && ret == num * sdebug_sector_size)
> 
> [Severity: Medium]
> Could the multiplication num * sdebug_sector_size overflow 32-bit arithmetic
> for large num? If the module is loaded with a capacity >= 4GB, an extremely
> large num and an empty data-out buffer might cause do_device_access() to
> safely return 0. If the multiplication overflows and evaluates to 0, this
> check might erroneously pass and incorrectly advance the write pointer.
> 
> [Severity: High]
> If do_device_access() returns a short count due to a small data-out buffer,
> this check prevents advancing the write pointer. However, the function then
> proceeds to return 0 (GOOD status) without calling scsi_set_resid(). Does
> this violate the ZBC specification, which requires that if a write terminates
> with GOOD status, the write pointer must be advanced by the transfer length?
> 
> Could this lead to silent data corruption or UNALIGNED WRITE COMMAND errors
> on subsequent sequential writes?

So it turns out that this driver is missing calls to:
scsi_set_resid() in:

resp_write_dt0(), resp_write_scat(), resp_atomic_write().

Interestingly, the driver does correctly call scsi_set_resid()
for read commands.

Will fix in v3, and will improve the WP advancement code to only
advance the blocks that was actually written (rather than leaving
the WP alone when not all blocks were written).


Kind regards,
Niklas