All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Yang Xiuwei" <yangxiuwei@kylinos.cn>
Cc: linux-scsi@vger.kernel.org
Subject: Re: [PATCH v2 1/3] scsi: sd: fix error handling in sd_probe() after large pool creation failure
Date: Tue, 07 Jul 2026 03:30:58 +0000	[thread overview]
Message-ID: <20260707033058.A8D2D1F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260707030333.22245-2-yangxiuwei@kylinos.cn>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] Calling device_unregister() before put_disk() in the error path creates a use-after-free vulnerability during SCSI Error Handling.
--

--- Patch [1]: [PATCH v2 1/3] scsi: sd: fix error handling in sd_probe() after large pool creation failure ---
commit f2e6ec332a4785be7451dad3afaf04d9cb43c90c
Author: Yang Xiuwei <yangxiuwei@kylinos.cn>
scsi: sd: fix error handling in sd_probe() after large pool creation failure

This commit modifies the error handling path in sd_probe() when
sd_large_pool_create() fails. It replaces a goto with direct calls to
device_unregister() and put_disk() to prevent leaking sysfs entries and
freeing an already registered device.

> diff --git a/drivers/scsi/sd.c b/drivers/scsi/sd.c
> index 599e75f333343..d18693d390b2f 100644
> --- a/drivers/scsi/sd.c
> +++ b/drivers/scsi/sd.c
> @@ -4089,7 +4089,9 @@ static int sd_probe(struct scsi_device *sdp)
>  	if (sdp->sector_size > PAGE_SIZE) {
>  		if (sd_large_pool_create()) {
>  			error = -ENOMEM;
> -			goto out_free_index;
> +			device_unregister(&sdkp->disk_dev);
> +			put_disk(gd);

[Severity: Critical]
Does this ordering create a use-after-free window?

By calling device_unregister() before put_disk(), the sdkp structure is
synchronously freed, but the request queue disk pointer remains set until
put_disk() runs.

If an asynchronous SCSI command times out precisely in this window, could
the SCSI error handler dereference the freed pointer?

drivers/scsi/sd.c:sd_eh_reset() {
        struct scsi_disk *sdkp = scsi_disk(scsi_cmd_to_rq(scmd)->q->disk);

        /* New SCSI EH run, reset gate variable */
        sdkp->ignore_medium_access_errors = false;
        ...
}

Would it be safer to call put_disk() before device_unregister() to clear
the queue disk pointer and prevent this concurrent access?

> +			goto out;
>  		}
>  	}
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707030333.22245-1-yangxiuwei@kylinos.cn?part=1

  reply	other threads:[~2026-07-07  3:30 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-07  3:03 [PATCH v2 0/3] scsi: sd: fix probe error cleanup, special_vec leak and sd_done() sense gate Yang Xiuwei
2026-07-07  3:03 ` [PATCH v2 1/3] scsi: sd: fix error handling in sd_probe() after large pool creation failure Yang Xiuwei
2026-07-07  3:30   ` sashiko-bot [this message]
2026-07-08  6:29   ` John Garry
2026-07-08  7:22     ` Yang Xiuwei
2026-07-08  7:57       ` John Garry
2026-07-07  3:03 ` [PATCH v2 2/3] scsi: sd: fix special_vec mempool leak when scsi_alloc_sgtables() fails Yang Xiuwei
2026-07-07  3:26   ` sashiko-bot
2026-07-08  6:32   ` John Garry
2026-07-07  3:03 ` [PATCH v2 3/3] scsi: sd: fix sd_done() sense handling condition Yang Xiuwei
2026-07-08 16:07   ` Bart Van Assche

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=20260707033058.A8D2D1F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    --cc=yangxiuwei@kylinos.cn \
    /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.