[PATCH v3 0/6] scsi: scsi_debug: fix zoned write validation

[PATCH v3 0/6] scsi: scsi_debug: fix zoned write validation

[Niklas Cassel]

This series fixes scsi_debug with regards to writes to a ZBC drive.

The series is based on 7.4/scsi-staging rather than 7.3/scsi-fixes (as
Damien suggested on v1) to avoid a build warning that would have happened
if this series was simply merged with linux-next (7.4/scsi-staging):

  error: too many arguments to function 'mk_sense_buffer'

Tested on a zoned scsi_debug device with 512 byte logical blocks and a
4096 byte physical block.

Changes since v2:
- Added patch 1, to that write paths correctly report the residual.
- Added patch 2, so that do_device_access() will always write a
  multiple of physical block size. (Reads are unchanged.)
- Patch 3 changed to advance the WP by how much that was actually written
  (rather than leaving the WP alone when not all blocks were written).
- Patch 5 changed to take the meta write lock if scsi_debug_lbp() is set.

Niklas Cassel (6):
  scsi: scsi_debug: Report the residual of a write
  scsi: scsi_debug: Do not write a partial physical block
  scsi: scsi_debug: Advance the write pointer over the data written
  scsi: scsi_debug: Enforce physical block alignment of zoned writes
  scsi: scsi_debug: Map the region written by WRITE ATOMIC (16)
  scsi: scsi_debug: Validate zone access for WRITE ATOMIC (16)

 drivers/scsi/scsi_debug.c | 66 +++++++++++++++++++++++++++++++++++----
 1 file changed, 60 insertions(+), 6 deletions(-)


base-commit: c3cff7fac01638ab58e85fe7df41a04fa25c5bae
-- 
2.55.0

[PATCH v3 1/6] scsi: scsi_debug: Report the residual of a write

[Niklas Cassel]

A command transfers the number of logical blocks that it asks for,
bounded by the data buffer that the initiator provided. When the buffer
is larger than that, the bytes beyond are not transferred, and the
difference is a residual that the initiator is entitled to be told
about.

resp_read_dt0() reports it, as does resp_write_tape(), but the write
paths for a disk do not: scsi_get_resid() keeps the zero that
scsi_debug_queuecommand() initialised it with, so a write with an
oversized buffer completes with GOOD status and a residual of zero, as
though the whole buffer had been consumed. A READ of the same length
into the same buffer reports the residual correctly, so the two
directions disagree.

Report it in resp_write_dt0(), resp_write_scat() and
resp_atomic_write().

resp_write_scat() needs a different expression from the other two. Its
data-out buffer holds the parameter list header and the LBA range
descriptors as well as the data, and sg_off walks over all of it, from
the offset at which the data begins to the end of the last range that
was written, so what the command consumed is sg_off and not the number
of bytes that the last range transferred.

Assisted-by: LLM
Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
 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 6941809dfdb7..641dd6f93791 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -5164,6 +5164,8 @@ static int resp_write_dt0(struct scsi_cmnd *scp, struct sdebug_dev_info *devip)
 			    "%s: write: cdb indicated=%u, IO sent=%d bytes\n",
 			    my_name, num * sdebug_sector_size, ret);
 
+	scsi_set_resid(scp, scsi_bufflen(scp) - ret);
+
 	if (unlikely((sdebug_opts & SDEBUG_OPT_RECOV_DIF_DIX) &&
 		     atomic_read(&sdeb_inject_pending))) {
 		if (sdebug_opts & SDEBUG_OPT_RECOVERED_ERR) {
@@ -5352,6 +5354,7 @@ static int resp_write_scat(struct scsi_cmnd *scp,
 		sg_off += num_by;
 		cum_lb += num;
 	}
+	scsi_set_resid(scp, scsi_bufflen(scp) - sg_off);
 	ret = 0;
 err_out_unlock:
 	sdeb_meta_write_unlock(sip);
@@ -6207,6 +6210,8 @@ static int resp_atomic_write(struct scsi_cmnd *scp,
 		return DID_ERROR << 16;
 	if (unlikely(ret != len * sdebug_sector_size))
 		return DID_ERROR << 16;
+
+	scsi_set_resid(scp, scsi_bufflen(scp) - ret);
 	return 0;
 }
 
-- 
2.55.0

Re: [PATCH v3 1/6] scsi: scsi_debug: Report the residual of a write

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] scsi_debug: unsigned underflow in resp_write_scat() residual calculation
--

commit a487bfccdf1b47222ab79a838751982d68882fc7
Author: Niklas Cassel <cassel@kernel.org>
Subject: scsi: scsi_debug: Report the residual of a write

This patch updates the write paths in scsi_debug to report residual
bytes when a buffer provided by the initiator is larger than the logical
blocks consumed. The residual is now reported in resp_write_dt0(),
resp_write_scat(), and resp_atomic_write().

> diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
> index 6941809dfdb75..641dd6f937912 100644
> --- a/drivers/scsi/scsi_debug.c
> +++ b/drivers/scsi/scsi_debug.c
[ ... ]
> @@ -5352,6 +5354,7 @@ static int resp_write_scat(struct scsi_cmnd *scp,
>  		sg_off += num_by;
>  		cum_lb += num;
>  	}
> +	scsi_set_resid(scp, scsi_bufflen(scp) - sg_off);

[Severity: High]
Can this calculation result in an unsigned underflow if the initiator
provides a buffer shorter than what the CDB descriptors request?

In resp_write_scat(), sg_off tracks the data offset. For each LBA range
descriptor, sg_off is unconditionally incremented by num_by, which is the
size requested by the descriptor. This happens even if do_device_access()
copied less data due to hitting the end of the data-out buffer.

If the initiator provides a shorter buffer (scsi_bufflen) than what the
CDB descriptors request, sg_off will eventually exceed the buffer length.
The subtraction here would then underflow to a massive positive number.

The SCSI midlayer uses this residual to compute good_bytes in
scsi_io_completion(). Subtracting a massive residual from the expected
transfer length could cause good_bytes to wrap and become negative, which
might severely confuse the block layer I/O completion logic.

>  	ret = 0;
>  err_out_unlock:
>  	sdeb_meta_write_unlock(sip);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260917125445.1376493-8-cassel@kernel.org?part=1

Re: [PATCH v3 1/6] scsi: scsi_debug: Report the residual of a write

[Damien Le Moal]

On 2026/09/17 19:54, Niklas Cassel wrote:
> A command transfers the number of logical blocks that it asks for,
> bounded by the data buffer that the initiator provided. When the buffer
> is larger than that, the bytes beyond are not transferred, and the
> difference is a residual that the initiator is entitled to be told
> about.
> 
> resp_read_dt0() reports it, as does resp_write_tape(), but the write
> paths for a disk do not: scsi_get_resid() keeps the zero that
> scsi_debug_queuecommand() initialised it with, so a write with an
> oversized buffer completes with GOOD status and a residual of zero, as
> though the whole buffer had been consumed. A READ of the same length
> into the same buffer reports the residual correctly, so the two
> directions disagree.
> 
> Report it in resp_write_dt0(), resp_write_scat() and
> resp_atomic_write().
> 
> resp_write_scat() needs a different expression from the other two. Its
> data-out buffer holds the parameter list header and the LBA range
> descriptors as well as the data, and sg_off walks over all of it, from
> the offset at which the data begins to the end of the last range that
> was written, so what the command consumed is sg_off and not the number
> of bytes that the last range transferred.
> 
> Assisted-by: LLM
> Signed-off-by: Niklas Cassel <cassel@kernel.org>

Yes. That goes with my comment about partial writes on v2.

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>

-- 
Damien Le Moal
Western Digital Research

[PATCH v3 2/6] scsi: scsi_debug: Do not write a partial physical block

[Niklas Cassel]

do_device_access() copies one logical block at a time and stops at the
first short copy, so when the data-out buffer is smaller than the
transfer length of the command it can write part of a physical block.
An initiator can arrange that with SG_IO.

A device writes whole physical blocks, and ZBC-3 r06 (T10/BSR INCITS
579), 4.5.3.3.2, requires a write to a sequential write required zone to
end on a physical block boundary, so a partly written physical block is
not a state that a device can be left in.

Stop at the last whole physical block that the buffer holds. The bytes
that are left over are not written, and are reported to the initiator as
part of the residual.

With the default physblk_exp=0 the physical block size equals the
logical block size and this changes nothing. Nothing changes either when
the buffer holds all of the data that the command asks for, so a command
that transfers fewer logical blocks than a physical block, which is
legal outside a sequential write required zone, is unaffected.

Assisted-by: LLM
Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
 drivers/scsi/scsi_debug.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 641dd6f93791..8ed7d5cd0ae0 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -4290,6 +4290,19 @@ static int do_device_access(struct sdeb_store_info *sip, struct scsi_cmnd *scp,
 
 	fsp = sip->storep;
 
+	/*
+	 * A data-out buffer that does not hold all of the data that the
+	 * command asks for is written up to the last whole physical block
+	 * that it does hold, so that a partial physical block is never
+	 * written. The bytes that are left over are reported as a residual.
+	 */
+	if (do_write) {
+		u32 avail = (sdb->length - sg_skip) / sdebug_sector_size;
+
+		if (avail < num)
+			num = round_down(avail, 1U << sdebug_physblk_exp);
+	}
+
 	block = do_div(lba, sdebug_store_sectors);
 
 	/* Only allow 1x atomic write or multiple non-atomic writes at any given time */
-- 
2.55.0

Re: [PATCH v3 2/6] scsi: scsi_debug: Do not write a partial physical block

[Damien Le Moal]

On 2026/09/17 19:54, Niklas Cassel wrote:
> do_device_access() copies one logical block at a time and stops at the
> first short copy, so when the data-out buffer is smaller than the
> transfer length of the command it can write part of a physical block.
> An initiator can arrange that with SG_IO.
> 
> A device writes whole physical blocks, and ZBC-3 r06 (T10/BSR INCITS
> 579), 4.5.3.3.2, requires a write to a sequential write required zone to
> end on a physical block boundary, so a partly written physical block is
> not a state that a device can be left in.

Physical block aligned writes are mandated only with ZBC for wries to sequential
zones. Unaligned writes to conventional zones are accepted, like they are with
regular drives (though not recommended due to potential performance issues with
read-modify-write cycles).

> 
> Stop at the last whole physical block that the buffer holds. The bytes
> that are left over are not written, and are reported to the initiator as
> part of the residual.
> 
> With the default physblk_exp=0 the physical block size equals the
> logical block size and this changes nothing. Nothing changes either when
> the buffer holds all of the data that the command asks for, so a command
> that transfers fewer logical blocks than a physical block, which is
> legal outside a sequential write required zone, is unaffected.
> 
> Assisted-by: LLM
> Signed-off-by: Niklas Cassel <cassel@kernel.org>
> ---
>  drivers/scsi/scsi_debug.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
> index 641dd6f93791..8ed7d5cd0ae0 100644
> --- a/drivers/scsi/scsi_debug.c
> +++ b/drivers/scsi/scsi_debug.c
> @@ -4290,6 +4290,19 @@ static int do_device_access(struct sdeb_store_info *sip, struct scsi_cmnd *scp,
>  
>  	fsp = sip->storep;
>  
> +	/*
> +	 * A data-out buffer that does not hold all of the data that the
> +	 * command asks for is written up to the last whole physical block
> +	 * that it does hold, so that a partial physical block is never
> +	 * written. The bytes that are left over are reported as a residual.
> +	 */
> +	if (do_write) {
> +		u32 avail = (sdb->length - sg_skip) / sdebug_sector_size;
> +
> +		if (avail < num)
> +			num = round_down(avail, 1U << sdebug_physblk_exp);

Nope, that is not correct. physical sector unaligned writes are OK with regular
disks. They are not for ZBC, but we should check that in
check_zbc_access_params() I think.

> +	}
> +
>  	block = do_div(lba, sdebug_store_sectors);
>  
>  	/* Only allow 1x atomic write or multiple non-atomic writes at any given time */


-- 
Damien Le Moal
Western Digital Research

Re: [PATCH v3 2/6] scsi: scsi_debug: Do not write a partial physical block

[Niklas Cassel]

On Thu, Sep 17, 2026 at 08:11:39PM +0700, Damien Le Moal wrote:
> > @@ -4290,6 +4290,19 @@ static int do_device_access(struct sdeb_store_info *sip, struct scsi_cmnd *scp,
> >  
> >  	fsp = sip->storep;
> >  
> > +	/*
> > +	 * A data-out buffer that does not hold all of the data that the
> > +	 * command asks for is written up to the last whole physical block
> > +	 * that it does hold, so that a partial physical block is never
> > +	 * written. The bytes that are left over are reported as a residual.
> > +	 */
> > +	if (do_write) {
> > +		u32 avail = (sdb->length - sg_skip) / sdebug_sector_size;
> > +
> > +		if (avail < num)
> > +			num = round_down(avail, 1U << sdebug_physblk_exp);
> 
> Nope, that is not correct. physical sector unaligned writes are OK with regular
> disks. They are not for ZBC, but we should check that in
> check_zbc_access_params() I think.

Patch [4/6] scsi: scsi_debug: Enforce physical block alignment of zonedwrites

does add a check for SWR zones, and for SWR zone only, which errors out if
the write is not aligned to the physical block size.

However, in the case of a short data-out buffer, the request is valid, so I
don't think that check_zbc_access_params() is the right place for the above
check.

But you are right that the check in do_device_access() should be gated on
SWR zones as well...


Something like this:

@@ -4263,6 +4273,8 @@ static int do_device_access(struct sdeb_store_info *sip, struct scsi_cmnd *scp,
        u64 block;
        enum dma_data_direction dir;
        struct scsi_data_buffer *sdb = &scp->sdb;
+       struct scsi_device *sdp = scp->device;
+       struct sdebug_dev_info *devip = (struct sdebug_dev_info *)sdp->hostdata;
        u8 *fsp;
        int i, total = 0;
 
@@ -4290,6 +4302,23 @@ static int do_device_access(struct sdeb_store_info *sip, struct scsi_cmnd *scp,
 
        fsp = sip->storep;
 
+       /*
+        * For SWR zones, a data-out buffer that does not hold all of the data
+        * that the command asks for is written up to the last whole physical
+        * block that it does hold, so that a partial physical block is never
+        * written. The bytes that are left over are reported as a residual.
+        */
+       if (do_write && sdebug_dev_is_zoned(devip)) {
+               struct sdeb_zone_state *zsp = zbc_zone(devip, lba);
+
+               if (zsp->z_type == ZBC_ZTYPE_SWR) {
+                       u32 avail = (sdb->length - sg_skip) / sdebug_sector_size;
+
+                       if (avail < num)
+                               num = round_down(avail, 1U << sdebug_physblk_exp);
+               }
+       }
+
        block = do_div(lba, sdebug_store_sectors);
 
        /* Only allow 1x atomic write or multiple non-atomic writes at any given time */



Kind regards,
Niklas

[PATCH v3 3/6] scsi: scsi_debug: Advance the write pointer over the data written

[Niklas Cassel]

resp_write_dt0() and resp_write_scat() advance the write pointer of a
sequential write required zone by the transfer length of the command
before looking at what do_device_access() returned. It does not always
move that much data: it returns -1 when the data direction of the
command does not match the operation, and a short byte count when the
data-out buffer is smaller than the transfer length, both of which an
initiator can produce with SG_IO. The first terminates the command, the
second completes it with GOOD status and a residual.

The zone state then describes more data than is on the medium, and a
write at the position where the data really ends is terminated with
UNALIGNED WRITE COMMAND, because the write pointer has moved beyond it.
The zone has to be reset before it can be written to again.

Advance the write pointer over the data that was written instead. As
do_device_access() does not write a partial physical block, what it
reports is a whole number of physical blocks, so the write pointer is
left on a physical block boundary, which is where a write can end.

resp_write_same() does not need the same treatment, as it writes with
memmove() and cannot fail part way through.

Assisted-by: LLM
Fixes: f0d1cf9378bd ("scsi: scsi_debug: Add ZBC zone commands")
Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
 drivers/scsi/scsi_debug.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 8ed7d5cd0ae0..41c8d958f445 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -5163,9 +5163,9 @@ 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))
-		zbc_inc_wp(devip, lba, num);
+	/* If ZBC zone then bump its write pointer over the data written */
+	if (sdebug_dev_is_zoned(devip) && ret > 0)
+		zbc_inc_wp(devip, lba, ret / sdebug_sector_size);
 	if (meta_data_locked)
 		sdeb_meta_write_unlock(sip);
 
@@ -5329,9 +5329,9 @@ static int resp_write_scat(struct scsi_cmnd *scp,
 		 * writes behaviour as possible.
 		 */
 		ret = do_device_access(sip, scp, sg_off, lba, num, group, true, true);
-		/* If ZBC zone then bump its write pointer */
-		if (sdebug_dev_is_zoned(devip))
-			zbc_inc_wp(devip, lba, num);
+		/* If ZBC zone then bump its write pointer over the data written */
+		if (sdebug_dev_is_zoned(devip) && ret > 0)
+			zbc_inc_wp(devip, lba, ret / sdebug_sector_size);
 		if (unlikely(scsi_debug_lbp()))
 			map_region(sip, lba, num);
 		if (unlikely(-1 == ret)) {
-- 
2.55.0

Re: [PATCH v3 3/6] scsi: scsi_debug: Advance the write pointer over the data written

[Damien Le Moal]

On 2026/09/17 19:54, Niklas Cassel wrote:
> resp_write_dt0() and resp_write_scat() advance the write pointer of a
> sequential write required zone by the transfer length of the command
> before looking at what do_device_access() returned. It does not always
> move that much data: it returns -1 when the data direction of the
> command does not match the operation, and a short byte count when the
> data-out buffer is smaller than the transfer length, both of which an
> initiator can produce with SG_IO. The first terminates the command, the
> second completes it with GOOD status and a residual.
> 
> The zone state then describes more data than is on the medium, and a
> write at the position where the data really ends is terminated with
> UNALIGNED WRITE COMMAND, because the write pointer has moved beyond it.
> The zone has to be reset before it can be written to again.
> 
> Advance the write pointer over the data that was written instead. As
> do_device_access() does not write a partial physical block, what it
> reports is a whole number of physical blocks, so the write pointer is
> left on a physical block boundary, which is where a write can end.
> 
> resp_write_same() does not need the same treatment, as it writes with
> memmove() and cannot fail part way through.
> 
> Assisted-by: LLM
> Fixes: f0d1cf9378bd ("scsi: scsi_debug: Add ZBC zone commands")
> Signed-off-by: Niklas Cassel <cassel@kernel.org>

OK. So you read my mind :)

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>

-- 
Damien Le Moal
Western Digital Research

[PATCH v3 4/6] scsi: scsi_debug: Enforce physical block alignment of zoned writes

[Niklas Cassel]

ZBC-3 r06 (T10/BSR INCITS 579), 4.5.3.3.2 Write access pattern
requirements for sequential write required zones, states:

  The device server terminates with CHECK CONDITION status, with the
  sense key set to ILLEGAL REQUEST, and the additional sense code set
  to UNALIGNED WRITE COMMAND a write command, other than an entire
  medium write same command, that specifies:
    a) the starting LBA in a sequential write required zone set to a
       value that is not equal to the write pointer for that sequential
       write required zone; or
    b) an ending LBA that is not equal to the last logical block within
       a physical block (see SBC-5).

That is why sd_zbc_read_zones() sets the zone_write_granularity queue
limit to the physical block size of a host-managed device, exposing the
constraint to user space.

check_zbc_access_params() implements condition a) but not condition b):
it verifies that a write to a sequential write required zone starts at
the write pointer of the zone, but never validates the ending LBA. As a
consequence, when scsi_debug emulates a host-managed device whose
physical block size is larger than its logical block size, for instance
with zbc=managed sector_size=512 physblk_exp=3, a write of a single
logical block at the write pointer of a sequential zone is accepted and
advances the write pointer by one logical block. The write pointer is
then no longer a multiple of the zone_write_granularity reported for the
device, so nothing can write at it at the granularity that was
advertised, and the zone can only be used again after being reset.

Implement condition b) as well, with the same sense data as the write
pointer check, as the standard gives both conditions the same sense key
and additional sense code. The exclusion of an entire medium write same
command needs no special case: such a command spans the whole medium, so
it is already terminated with WRITE BOUNDARY VIOLATION by the preceding
check.

The check is placed in check_zbc_access_params(), which every command
that advances a zone write pointer reaches first: WRITE, WRITE SCATTERED
and WRITE SAME. Reads return earlier in the function and are unaffected.

Sequential write preferred zones, which are emulated for host-aware
devices with zbc=aware, are left alone: writes to them are not required
to be sequential, and Linux does not restrict the write granularity of
host-aware devices.

With the default physblk_exp=0, the physical block size equals the
logical block size and the new check is a no-op.

Assisted-by: LLM
Fixes: f0d1cf9378bd ("scsi: scsi_debug: Add ZBC zone commands")
Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
 drivers/scsi/scsi_debug.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 41c8d958f445..64f32d97455d 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -3980,6 +3980,16 @@ static int check_zbc_access_params(struct scsi_cmnd *scp,
 					UNALIGNED_WRITE_COMMAND);
 			return check_condition_result;
 		}
+		/*
+		 * Writes must end on a physical block boundary, that is, the
+		 * transfer length must be a multiple of the physical block
+		 * size.
+		 */
+		if (!IS_ALIGNED(lba + num, 1U << sdebug_physblk_exp)) {
+			mk_sense_buffer(scp, ILLEGAL_REQUEST,
+					UNALIGNED_WRITE_COMMAND);
+			return check_condition_result;
+		}
 	}
 
 	/* Handle implicit open of closed and empty zones */
-- 
2.55.0

Re: [PATCH v3 4/6] scsi: scsi_debug: Enforce physical block alignment of zoned writes

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] scsi: scsi_debug: ZBC write physical block boundary check ignores lowest_aligned
--

commit 6fe3c0eff911c1b4e6a4a33707f2eb37306c8d19
Author: Niklas Cassel <cassel@kernel.org>

scsi: scsi_debug: Enforce physical block alignment of zoned writes

This commit enforces the requirement from ZBC-3 that write commands to
sequential write required zones must end on a physical block boundary.
It does this by adding a check in check_zbc_access_params() to ensure
the transfer length aligns with the physical block size.

> diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
> index 41c8d958f4458..64f32d97455d6 100644
> --- a/drivers/scsi/scsi_debug.c
> +++ b/drivers/scsi/scsi_debug.c
> @@ -3980,6 +3980,16 @@ static int check_zbc_access_params(struct scsi_cmnd *scp,
[ ... ]
> +		/*
> +		 * Writes must end on a physical block boundary, that is, the
> +		 * transfer length must be a multiple of the physical block
> +		 * size.
> +		 */
> +		if (!IS_ALIGNED(lba + num, 1U << sdebug_physblk_exp)) {

[Severity: Medium]
Does this alignment check ignore the physical block offset?

If a ZBC device is configured with lowest_aligned > 0 (for example
zbc=managed, physblk_exp > 0, lowest_aligned > 0), valid write commands
to sequential required zones where the ending LBA perfectly aligns with
the advertised physical block boundary (meaning lba + num - lowest_aligned
is a multiple of the physical block size) will be rejected with an ILLEGAL
REQUEST (UNALIGNED WRITE COMMAND) sense key. This breaks device emulation
and fails valid I/O. Invalid writes might also be accepted, violating the
ZBC-3 standard.

Should this check subtract sdebug_lowest_aligned to compute the correct
physical block boundary as required by SBC-5?

> +			mk_sense_buffer(scp, ILLEGAL_REQUEST,
> +					UNALIGNED_WRITE_COMMAND);
> +			return check_condition_result;
> +		}
>  	}
>  
>  	/* Handle implicit open of closed and empty zones */

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260917125445.1376493-8-cassel@kernel.org?part=4

[PATCH v3 5/6] scsi: scsi_debug: Map the region written by WRITE ATOMIC (16)

[Niklas Cassel]

When logical block provisioning is enabled, a command that writes user
data marks the region that it wrote in the provisioning map, so that
GET LBA STATUS reports the region as mapped. resp_write_dt0(),
resp_write_scat() and resp_write_same() all call map_region() for that.

resp_atomic_write() does not, so a WRITE ATOMIC (16) leaves the
provisioning map untouched, and GET LBA STATUS keeps reporting the
region as deallocated after it has been written. map_state(), which
GET LBA STATUS uses, is the only reader of the map, so that is the whole
of the effect.

Call map_region() the way resp_write_dt0() does, and take the zone
metadata write lock across the access as it does when logical block
provisioning is enabled. That lock is what serialises the provisioning
map against resp_unmap(), which holds it while unmap_region() clears map
bits and zeroes the data that they cover. resp_unmap() does nothing
unless logical block provisioning is enabled, so the lock is only needed
in that case.

Assisted-by: LLM
Fixes: 84f3a3c01d70 ("scsi: scsi_debug: Atomic write support")
Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
 drivers/scsi/scsi_debug.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 64f32d97455d..33f2df26e5af 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -6184,6 +6184,7 @@ static int resp_atomic_write(struct scsi_cmnd *scp,
 	u8 *cmd = scp->cmnd;
 	u16 boundary, len;
 	u64 lba, lba_tmp;
+	bool meta_data_locked = false;
 	int ret;
 
 	if (!scsi_debug_atomic_write()) {
@@ -6228,7 +6229,18 @@ static int resp_atomic_write(struct scsi_cmnd *scp,
 		}
 	}
 
+	if (scsi_debug_lbp()) {
+		sdeb_meta_write_lock(sip);
+		meta_data_locked = true;
+	}
+
 	ret = do_device_access(sip, scp, 0, lba, len, 0, true, true);
+	if (unlikely(scsi_debug_lbp()))
+		map_region(sip, lba, len);
+
+	if (meta_data_locked)
+		sdeb_meta_write_unlock(sip);
+
 	if (unlikely(ret == -1))
 		return DID_ERROR << 16;
 	if (unlikely(ret != len * sdebug_sector_size))
-- 
2.55.0

Re: [PATCH v3 5/6] scsi: scsi_debug: Map the region written by WRITE ATOMIC (16)

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] scsi_debug: ABBA deadlock between resp_atomic_write() and resp_comp_write()
- [Medium] scsi_debug: TOCTOU race in resp_atomic_write() via double evaluation
--

commit d00845f77d0b11824b6bd723b3a99d9e2026b838
Author: Niklas Cassel <cassel@kernel.org>

scsi: scsi_debug: Map the region written by WRITE ATOMIC (16)

When logical block provisioning is enabled, commands that write user data mark
the region in the provisioning map. WRITE ATOMIC (16) leaves the map untouched,
so this patch calls map_region() and takes the metadata write lock across the
access.

> diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
> index 64f32d97455d6..33f2df26e5af7 100644
> --- a/drivers/scsi/scsi_debug.c
> +++ b/drivers/scsi/scsi_debug.c
[ ... ]
> @@ -6228,7 +6229,18 @@ static int resp_atomic_write(struct scsi_cmnd *scp,
>  		}
>  	}
>  
> +	if (scsi_debug_lbp()) {
> +		sdeb_meta_write_lock(sip);
> +		meta_data_locked = true;
> +	}
> +
>  	ret = do_device_access(sip, scp, 0, lba, len, 0, true, true);

[Severity: High]
Does this code introduce a lock inversion deadlock with resp_comp_write()?

Here in resp_atomic_write(), the metadata lock is acquired before calling
do_device_access(), which acquires the data lock:

resp_atomic_write()
  sdeb_meta_write_lock(sip);
  do_device_access()
    sdeb_data_lock(sip, true)
      sdeb_data_write_lock(sip);

However, in resp_comp_write(), the locks are acquired in the reverse order:

resp_comp_write()
  sdeb_data_write_lock(sip);
  sdeb_meta_write_lock(sip);

Could concurrent execution of write atomic (16) and compare and write
commands to the same target cause the scsi processing kworkers to deadlock?

> +	if (unlikely(scsi_debug_lbp()))
> +		map_region(sip, lba, len);

[Severity: Medium]
Could this result in a race condition due to evaluating scsi_debug_lbp() twice?

If an administrator modifies /sys/bus/pseudo/drivers/scsi_debug/fake_rw while
a write atomic (16) command is executing, the first evaluation could return
false, bypassing the lock acquisition. If the second evaluation then returns
true, map_region() would be executed without the metadata lock being held.

Could this be fixed by caching the result of the first evaluation,
or checking meta_data_locked instead?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260917125445.1376493-8-cassel@kernel.org?part=5

Re: [PATCH v3 5/6] scsi: scsi_debug: Map the region written by WRITE ATOMIC (16)

[Niklas Cassel]

On Thu, Sep 17, 2026 at 01:12:50PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] scsi_debug: ABBA deadlock between resp_atomic_write() and resp_comp_write()
> - [Medium] scsi_debug: TOCTOU race in resp_atomic_write() via double evaluation
> --
> 
> commit d00845f77d0b11824b6bd723b3a99d9e2026b838
> Author: Niklas Cassel <cassel@kernel.org>
> 
> scsi: scsi_debug: Map the region written by WRITE ATOMIC (16)
> 
> When logical block provisioning is enabled, commands that write user data mark
> the region in the provisioning map. WRITE ATOMIC (16) leaves the map untouched,
> so this patch calls map_region() and takes the metadata write lock across the
> access.
> 
> > diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
> > index 64f32d97455d6..33f2df26e5af7 100644
> > --- a/drivers/scsi/scsi_debug.c
> > +++ b/drivers/scsi/scsi_debug.c
> [ ... ]
> > @@ -6228,7 +6229,18 @@ static int resp_atomic_write(struct scsi_cmnd *scp,
> >  		}
> >  	}
> >  
> > +	if (scsi_debug_lbp()) {
> > +		sdeb_meta_write_lock(sip);
> > +		meta_data_locked = true;
> > +	}
> > +
> >  	ret = do_device_access(sip, scp, 0, lba, len, 0, true, true);
> 
> [Severity: High]
> Does this code introduce a lock inversion deadlock with resp_comp_write()?
> 
> Here in resp_atomic_write(), the metadata lock is acquired before calling
> do_device_access(), which acquires the data lock:
> 
> resp_atomic_write()
>   sdeb_meta_write_lock(sip);
>   do_device_access()
>     sdeb_data_lock(sip, true)
>       sdeb_data_write_lock(sip);
> 
> However, in resp_comp_write(), the locks are acquired in the reverse order:
> 
> resp_comp_write()
>   sdeb_data_write_lock(sip);
>   sdeb_meta_write_lock(sip);

Well, this seems to be a pre-existing problem, since resp_write_dt0() takes
the locks in the same order as resp_atomic_write().

It looks like resp_comp_write() is the only one who takes the locks in opposite
order, so the fix is probably to swap the order in resp_comp_write().


> 
> Could concurrent execution of write atomic (16) and compare and write
> commands to the same target cause the scsi processing kworkers to deadlock?
> 
> > +	if (unlikely(scsi_debug_lbp()))
> > +		map_region(sip, lba, len);
> 
> [Severity: Medium]
> Could this result in a race condition due to evaluating scsi_debug_lbp() twice?
> 
> If an administrator modifies /sys/bus/pseudo/drivers/scsi_debug/fake_rw while
> a write atomic (16) command is executing, the first evaluation could return
> false, bypassing the lock acquisition. If the second evaluation then returns
> true, map_region() would be executed without the metadata lock being held.
> 
> Could this be fixed by caching the result of the first evaluation,
> or checking meta_data_locked instead?

Sure, but this is also a pre-existing problem, since resp_write_dt0() looks
exactly the same.

But sure, can introduce a local variable in all functions that call
scsi_debug_lbp(), so that it is never evaluated twice.


Kind regards,
Niklas

Re: [PATCH v3 5/6] scsi: scsi_debug: Map the region written by WRITE ATOMIC (16)

[Damien Le Moal]

On 2026/09/17 19:54, Niklas Cassel wrote:
> When logical block provisioning is enabled, a command that writes user
> data marks the region that it wrote in the provisioning map, so that
> GET LBA STATUS reports the region as mapped. resp_write_dt0(),
> resp_write_scat() and resp_write_same() all call map_region() for that.
> 
> resp_atomic_write() does not, so a WRITE ATOMIC (16) leaves the
> provisioning map untouched, and GET LBA STATUS keeps reporting the
> region as deallocated after it has been written. map_state(), which
> GET LBA STATUS uses, is the only reader of the map, so that is the whole
> of the effect.
> 
> Call map_region() the way resp_write_dt0() does, and take the zone
> metadata write lock across the access as it does when logical block
> provisioning is enabled. That lock is what serialises the provisioning
> map against resp_unmap(), which holds it while unmap_region() clears map
> bits and zeroes the data that they cover. resp_unmap() does nothing
> unless logical block provisioning is enabled, so the lock is only needed
> in that case.
> 
> Assisted-by: LLM
> Fixes: 84f3a3c01d70 ("scsi: scsi_debug: Atomic write support")
> Signed-off-by: Niklas Cassel <cassel@kernel.org>

Looks OK.

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>

-- 
Damien Le Moal
Western Digital Research

[PATCH v3 6/6] scsi: scsi_debug: Validate zone access for WRITE ATOMIC (16)

[Niklas Cassel]

WRITE ATOMIC (16) is a write command, so on a zoned device it is subject
to the access requirements of the zone that it addresses, and it advances
the write pointer of a sequential write required zone. See ZBC-3 r06
(T10/BSR INCITS 579), 4.5.3.3.2 Write access pattern requirements for
sequential write required zones.

resp_atomic_write() calls do_device_access() directly, without calling
check_device_access_params() first and without advancing the write
pointer afterwards. It is the only command that writes user data which
does not; WRITE, WRITE SCATTERED and WRITE SAME all go through
check_device_access_params(), which validates zone access for a zoned
device.

As a consequence, with zbc=managed atomic_wr=1, a WRITE ATOMIC (16) can
write anywhere within a sequential write required zone regardless of its
write pointer and zone condition, into a gap zone, or across a zone
boundary, and none of it is reflected in the zone state. The write
pointer is left where it was, so a subsequent REPORT ZONES does not
describe the data on the medium, and the next write at that write
pointer overwrites data that was written without error.

Validate the access and advance the write pointer the way
resp_write_dt0() does. The zone metadata write lock is already taken
across the access for the provisioning map; take it for a zoned device
too, as the write pointer has to be read and updated atomically with
respect to other commands. Unlike an ordinary write, the write pointer
is advanced only when all of the data was written, as an atomic write
either completes or has no effect.

Assisted-by: LLM
Fixes: 84f3a3c01d70 ("scsi: scsi_debug: Atomic write support")
Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
 drivers/scsi/scsi_debug.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 33f2df26e5af..b57d25d7214c 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -6229,15 +6229,29 @@ static int resp_atomic_write(struct scsi_cmnd *scp,
 		}
 	}
 
-	if (scsi_debug_lbp()) {
+	if (sdebug_dev_is_zoned(devip) || scsi_debug_lbp()) {
 		sdeb_meta_write_lock(sip);
 		meta_data_locked = true;
 	}
 
+	ret = check_device_access_params(scp, lba, len, true);
+	if (ret) {
+		if (meta_data_locked)
+			sdeb_meta_write_unlock(sip);
+		return ret;
+	}
+
 	ret = do_device_access(sip, scp, 0, lba, len, 0, true, true);
 	if (unlikely(scsi_debug_lbp()))
 		map_region(sip, lba, len);
 
+	/*
+	 * If ZBC zone then bump its write pointer, but only if all of the data
+	 * was written: an atomic write either completes or has no effect.
+	 */
+	if (sdebug_dev_is_zoned(devip) && ret == len * sdebug_sector_size)
+		zbc_inc_wp(devip, lba, len);
+
 	if (meta_data_locked)
 		sdeb_meta_write_unlock(sip);
 
-- 
2.55.0

Re: [PATCH v3 6/6] scsi: scsi_debug: Validate zone access for WRITE ATOMIC (16)

[Damien Le Moal]

On 2026/09/17 19:54, Niklas Cassel wrote:
> WRITE ATOMIC (16) is a write command, so on a zoned device it is subject
> to the access requirements of the zone that it addresses, and it advances
> the write pointer of a sequential write required zone. See ZBC-3 r06
> (T10/BSR INCITS 579), 4.5.3.3.2 Write access pattern requirements for
> sequential write required zones.
> 
> resp_atomic_write() calls do_device_access() directly, without calling
> check_device_access_params() first and without advancing the write
> pointer afterwards. It is the only command that writes user data which
> does not; WRITE, WRITE SCATTERED and WRITE SAME all go through
> check_device_access_params(), which validates zone access for a zoned
> device.
> 
> As a consequence, with zbc=managed atomic_wr=1, a WRITE ATOMIC (16) can
> write anywhere within a sequential write required zone regardless of its
> write pointer and zone condition, into a gap zone, or across a zone
> boundary, and none of it is reflected in the zone state. The write
> pointer is left where it was, so a subsequent REPORT ZONES does not
> describe the data on the medium, and the next write at that write
> pointer overwrites data that was written without error.
> 
> Validate the access and advance the write pointer the way
> resp_write_dt0() does. The zone metadata write lock is already taken
> across the access for the provisioning map; take it for a zoned device
> too, as the write pointer has to be read and updated atomically with
> respect to other commands. Unlike an ordinary write, the write pointer
> is advanced only when all of the data was written, as an atomic write
> either completes or has no effect.
> 
> Assisted-by: LLM
> Fixes: 84f3a3c01d70 ("scsi: scsi_debug: Atomic write support")
> Signed-off-by: Niklas Cassel <cassel@kernel.org>

See my comment on v2.

> ---
>  drivers/scsi/scsi_debug.c | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
> index 33f2df26e5af..b57d25d7214c 100644
> --- a/drivers/scsi/scsi_debug.c
> +++ b/drivers/scsi/scsi_debug.c
> @@ -6229,15 +6229,29 @@ static int resp_atomic_write(struct scsi_cmnd *scp,
>  		}
>  	}
>  
> -	if (scsi_debug_lbp()) {
> +	if (sdebug_dev_is_zoned(devip) || scsi_debug_lbp()) {
>  		sdeb_meta_write_lock(sip);
>  		meta_data_locked = true;
>  	}
>  
> +	ret = check_device_access_params(scp, lba, len, true);
> +	if (ret) {
> +		if (meta_data_locked)
> +			sdeb_meta_write_unlock(sip);
> +		return ret;
> +	}
> +
>  	ret = do_device_access(sip, scp, 0, lba, len, 0, true, true);
>  	if (unlikely(scsi_debug_lbp()))
>  		map_region(sip, lba, len);
>  
> +	/*
> +	 * If ZBC zone then bump its write pointer, but only if all of the data
> +	 * was written: an atomic write either completes or has no effect.
> +	 */
> +	if (sdebug_dev_is_zoned(devip) && ret == len * sdebug_sector_size)
> +		zbc_inc_wp(devip, lba, len);
> +
>  	if (meta_data_locked)
>  		sdeb_meta_write_unlock(sip);
>  


-- 
Damien Le Moal
Western Digital Research

Re: [PATCH v3 0/6] scsi: scsi_debug: fix zoned write validation

[Damien Le Moal]

On 2026/09/17 19:54, Niklas Cassel wrote:
> This series fixes scsi_debug with regards to writes to a ZBC drive.
> 
> The series is based on 7.4/scsi-staging rather than 7.3/scsi-fixes (as
> Damien suggested on v1) to avoid a build warning that would have happened
> if this series was simply merged with linux-next (7.4/scsi-staging):
> 
>   error: too many arguments to function 'mk_sense_buffer'
> 
> Tested on a zoned scsi_debug device with 512 byte logical blocks and a
> 4096 byte physical block.
> 
> Changes since v2:

Slow down please. I was still replying to v2...

> - Added patch 1, to that write paths correctly report the residual.
> - Added patch 2, so that do_device_access() will always write a
>   multiple of physical block size. (Reads are unchanged.)
> - Patch 3 changed to advance the WP by how much that was actually written
>   (rather than leaving the WP alone when not all blocks were written).
> - Patch 5 changed to take the meta write lock if scsi_debug_lbp() is set.
> 
> Niklas Cassel (6):
>   scsi: scsi_debug: Report the residual of a write
>   scsi: scsi_debug: Do not write a partial physical block
>   scsi: scsi_debug: Advance the write pointer over the data written
>   scsi: scsi_debug: Enforce physical block alignment of zoned writes
>   scsi: scsi_debug: Map the region written by WRITE ATOMIC (16)
>   scsi: scsi_debug: Validate zone access for WRITE ATOMIC (16)
> 
>  drivers/scsi/scsi_debug.c | 66 +++++++++++++++++++++++++++++++++++----
>  1 file changed, 60 insertions(+), 6 deletions(-)
> 
> 
> base-commit: c3cff7fac01638ab58e85fe7df41a04fa25c5bae


-- 
Damien Le Moal
Western Digital Research