All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "John Garry" <john.garry@linux.dev>
Cc: linux-scsi@vger.kernel.org
Subject: Re: [PATCH] scsi: scsi_debug: default to a higher throughput config
Date: Wed, 02 Sep 2026 09:57:58 +0000	[thread overview]
Message-ID: <20260902095759.253D21F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260902092844.1741533-1-john.garry@linux.dev>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Stale defer_t in sdebug_scsi_cmd causes premature double scsi_done() leading to UAF.
--

commit 92c33bac6eb173f6156f5a3b2daf5604d8994b5b
Author: John Garry <john.garry@linux.dev>

scsi: scsi_debug: default to a higher throughput config

Currently the default config goes not give as high a throughput as some
would like. Give a higher default throughput by modifying the delay,
can_queue, and clustering defaults.

> diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
> index 3346562275f80..f2afe757c4380 100644
> --- a/drivers/scsi/scsi_debug.c
> +++ b/drivers/scsi/scsi_debug.c
> @@ -79,7 +79,7 @@ static const char *sdebug_version_date = "20210520";
>   */
>  #define DEF_ATO 1
>  #define DEF_CDB_LEN 10
> -#define DEF_JDELAY   1		/* if > 0 unit is a jiffy */
> +#define DEF_JDELAY   0		/* if > 0 unit is a jiffy */

[Severity: Critical]
This is a pre-existing issue, but changing the default delay to 0 exposes it
out-of-the-box to all users.

When delay=0, schedule_resp() takes the early completion path without updating
sd_dp->defer_t or sd_dp->cmpl_ts:

drivers/scsi/scsi_debug.c:schedule_resp() {
    ...
respond_in_thread:      /* call back to mid-layer using invocation thread */
        cmnd->result = pfp != NULL ? pfp(cmnd, devip) : 0;
        cmnd->result &= ~SDEG_RES_IMMED_MASK;
        if (cmnd->result == 0 && scsi_result != 0)
                cmnd->result = scsi_result;
        scsi_done(cmnd);
        return 0;
}

Because scsi_cmd_priv(cmd) is not zeroed per-request by the SCSI midlayer,
a reused tag retains its previous defer_t value.

If the previous command on this tag was polled, defer_t is left as
SDEB_DEFER_POLL. Concurrently, if a userspace thread invokes
sdebug_blk_mq_poll_iter(), it iterates over busy tags, sees the stale
SDEB_DEFER_POLL and expired cmpl_ts, and spuriously calls scsi_done(cmd) a
second time:

drivers/scsi/scsi_debug.c:sdebug_blk_mq_poll_iter() {
    ...
        spin_lock_irqsave(&sdsc->lock, flags);
        sd_dp = &sdsc->sd_dp;
        if (sd_dp->defer_t != SDEB_DEFER_POLL) {
                spin_unlock_irqrestore(&sdsc->lock, flags);
                return true;
        }

        if (time < sd_dp->cmpl_ts) {
                spin_unlock_irqrestore(&sdsc->lock, flags);
                return true;
        }
        spin_unlock_irqrestore(&sdsc->lock, flags);
    ...
        scsi_done(cmd); /* callback to mid level */
}

Could this lead to a double scsi_done() and a Use-After-Free in the block
layer when racing with the early completion path?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260902092844.1741533-1-john.garry@linux.dev?part=1

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

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-02  9:28 [PATCH] scsi: scsi_debug: default to a higher throughput config John Garry
2026-09-02  9:57 ` sashiko-bot [this message]
2026-09-02 11:28 ` Christoph Hellwig
2026-09-10  1:59 ` Martin K. Petersen (Oracle)

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=20260902095759.253D21F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=john.garry@linux.dev \
    --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.