All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Niklas Cassel" <cassel@kernel.org>
Cc: linux-scsi@vger.kernel.org
Subject: Re: [PATCH v8 05/11] scsi: scsi_debug: Report the residual of a write
Date: Sat, 26 Sep 2026 18:29:19 +0000	[thread overview]
Message-ID: <20260926182920.46FDF1F000FF@smtp.kernel.org> (raw)
In-Reply-To: <20260926181702.508975-18-cassel@kernel.org>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] scsi: scsi_debug: residual reporting bypassed on error paths in resp_write_scat
--

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

scsi: scsi_debug: Report the residual of a write

This commit adds residual reporting for writes in the scsi_debug driver. It
ensures that commands whose data buffer is larger than their transfer length
report the unconsumed portion as a residual, making writes behave consistently
with reads.

> diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
> index b64ae3ad300d8..9220758bb8017 100644
> --- a/drivers/scsi/scsi_debug.c
> +++ b/drivers/scsi/scsi_debug.c

[ ... ]

> @@ -5350,6 +5357,13 @@ static int resp_write_scat(struct scsi_cmnd *scp,
>  		sg_off += num_by;
>  		cum_lb += num;
>  	}
> +	/*
> +	 * sg_off counts what the command asked for, which can exceed the
> +	 * buffer: lbdof is not validated against it, and a range is counted
> +	 * in full even if do_device_access() copied less.
> +	 */
> +	if (scsi_bufflen(scp) > sg_off)
> +		scsi_set_resid(scp, scsi_bufflen(scp) - sg_off);
>  	ret = 0;
>  err_out_unlock:
>  	sdeb_meta_write_unlock(sip);

[Severity: Medium]
Does placing the err_out_unlock label after the residual reporting logic
cause error exits to bypass it?

If a write scattered command processes the first LBA range successfully but
fails on a subsequent range (for example, if the second range exceeds
device capacity and check_device_access_params() fails), the code in
resp_write_scat() jumps directly to err_out_unlock:

		ret = check_device_access_params(scp, lba, num, true);
		if (ret)
			goto err_out_unlock;

This bypasses the new residual reporting logic at the end of the function,
leaving the residual incorrectly at 0 instead of reflecting the unwritten
data. Should the residual also be updated before returning from these error
paths?

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

  reply	other threads:[~2026-09-26 18:29 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-26 18:17 [PATCH v8 00/11] scsi: scsi_debug: fix zoned write validation Niklas Cassel
2026-09-26 18:17 ` [PATCH v8 01/11] scsi: scsi_debug: Refuse a zoned device with a non-zero lowest aligned LBA Niklas Cassel
2026-09-26 18:17 ` [PATCH v8 02/11] scsi: scsi_debug: Make atomic writes and ZBC emulation mutually exclusive Niklas Cassel
2026-09-26 18:17 ` [PATCH v8 03/11] scsi: scsi_debug: Take the zone metadata lock before the data lock Niklas Cassel
2026-09-26 18:17 ` [PATCH v8 04/11] scsi: scsi_debug: Evaluate scsi_debug_lbp() only once Niklas Cassel
2026-09-26 18:17 ` [PATCH v8 05/11] scsi: scsi_debug: Report the residual of a write Niklas Cassel
2026-09-26 18:29   ` sashiko-bot [this message]
2026-09-26 18:17 ` [PATCH v8 06/11] scsi: scsi_debug: Enforce physical block alignment of zoned writes Niklas Cassel
2026-09-26 18:17 ` [PATCH v8 07/11] scsi: scsi_debug: Do not write a partial physical block to a zoned device Niklas Cassel
2026-09-26 18:17 ` [PATCH v8 08/11] scsi: scsi_debug: Advance the write pointer over the data written Niklas Cassel
2026-09-26 18:17 ` [PATCH v8 09/11] scsi: scsi_debug: Refuse a short WRITE ATOMIC (16) before writing it Niklas Cassel
2026-09-26 18:17 ` [PATCH v8 10/11] scsi: scsi_debug: Map the region written by WRITE ATOMIC (16) Niklas Cassel
2026-09-26 18:17 ` [PATCH v8 11/11] scsi: scsi_debug: Validate the access parameters of " 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=20260926182920.46FDF1F000FF@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=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.