All of lore.kernel.org
 help / color / mirror / Atom feed
From: Niklas Cassel <cassel@kernel.org>
To: sashiko-reviews@lists.linux.dev
Cc: linux-scsi@vger.kernel.org
Subject: Re: [PATCH 2/2] scsi: scsi_debug: Validate zone access for WRITE ATOMIC (16)
Date: Thu, 17 Sep 2026 11:09:18 +0200	[thread overview]
Message-ID: <aquuPkwDEK6Ppnte@ryzen> (raw)
In-Reply-To: <20260917090021.7C6E81F000FF@smtp.kernel.org>

On Thu, Sep 17, 2026 at 09:00:21AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] scsi: scsi_debug: zone write pointer incremented on failed ATOMIC WRITE
> --
> 
> commit 9183d5d7f092b92218c3638f33edc6839086d9f3
> Author: Niklas Cassel <cassel@kernel.org>
> 
> scsi: scsi_debug: Validate zone access for WRITE ATOMIC (16)
> 
> 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. This patch validates the
> access and advances the write pointer holding the zone metadata write lock
> across both operations.
> 
> > diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
> > index 5a95b7cd7a44c..d52b1f762afb5 100644
> > --- a/drivers/scsi/scsi_debug.c
> > +++ b/drivers/scsi/scsi_debug.c
> > @@ -6209,7 +6209,24 @@ static int resp_atomic_write(struct scsi_cmnd *scp,
> [ ... ]
> >  	ret = do_device_access(sip, scp, 0, lba, len, 0, true, true);
> > +
> > +	/* If ZBC zone then bump its write pointer */
> > +	if (sdebug_dev_is_zoned(devip)) {
> > +		zbc_inc_wp(devip, lba, len);
> 
> [Severity: Medium]
> Does this code advance the write pointer even if do_device_access() fails?
> 
> If a command like WRITE ATOMIC (16) is submitted with an invalid DMA
> direction (e.g., DMA_FROM_DEVICE via SG_IO) or a truncated scatter-gather
> list, do_device_access() can return -1 or a short length.
> 
> Because the write pointer in resp_atomic_write() is incremented before
> checking the return value of do_device_access(), the mock device's internal
> zone state could become out-of-sync with the simulated physical medium.
> This could cause subsequent valid writes to be incorrectly rejected with
> UNALIGNED WRITE COMMAND.

That is how resp_write_dt0() and all other write functions is scsi_debug is
implemented.

That said, I agree that it looks suboptimal...

I can write prep-patch that cleans up all the write functions to only
advance the WP when do_device_access() succeeds.


Kind regards,
Niklas

  reply	other threads:[~2026-09-17  9:09 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-17  8:45 [PATCH 0/2] scsi: scsi_debug: fix zoned write validation Niklas Cassel
2026-09-17  8:45 ` [PATCH 1/2] scsi: scsi_debug: Enforce physical block alignment of zoned writes Niklas Cassel
2026-09-17  9:05   ` Damien Le Moal
2026-09-17 10:42     ` Niklas Cassel
2026-09-17  8:45 ` [PATCH 2/2] scsi: scsi_debug: Validate zone access for WRITE ATOMIC (16) Niklas Cassel
2026-09-17  9:00   ` sashiko-bot
2026-09-17  9:09     ` Niklas Cassel [this message]
2026-09-17  9:07   ` Damien Le Moal
2026-09-17  9:38   ` John Garry
2026-09-17  9:56     ` Niklas Cassel
2026-09-17 10:26       ` John Garry
2026-09-17 10:35         ` Niklas Cassel
2026-09-17 15:42           ` John Garry
2026-09-17 16:10             ` Niklas Cassel
2026-09-17 16:40               ` John Garry
2026-09-17 17:37               ` Niklas Cassel
2026-09-18  2:27                 ` Damien Le Moal
2026-09-18  5:37                   ` Niklas Cassel
2026-09-18  6:15                     ` Damien Le Moal
2026-09-18  7:06                       ` Christoph Hellwig
2026-09-18  7:27                         ` Niklas Cassel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aquuPkwDEK6Ppnte@ryzen \
    --to=cassel@kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.