All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "James Bottomley" <James.Bottomley@HansenPartnership.com>
Cc: linux-scsi@vger.kernel.org
Subject: Re: [GIT PULL] SCSI fixes for 7.3-rc4+
Date: Fri, 25 Sep 2026 21:46:13 +0000	[thread overview]
Message-ID: <20260925214614.0CD7E1F000FF@smtp.kernel.org> (raw)
In-Reply-To: <ea8f769b49f7d1185f588cf5b9202f300c280af7.camel@HansenPartnership.com>

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] scsi: ufs: Use-After-Free on pseudo_sdev in ufshcd_err_handling_prepare()
- [Low] scsi: megaraid: mixing goto-based error handling and scoped_guard in megasas_resume
--

commit 779f0b1927d5f926d874e86dcf935da1578ca2f4
Author: James Bottomley <James.Bottomley@HansenPartnership.com>
Date:   Fri Sep 25 21:22:51 2026 +0000

SCSI fixes for 7.3-rc4+

Mostly small driver fixes. The biggest fix is the one to the block zone
handling which might trip for real or virtual hardware if the number of
zones is > 2^32.

> diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c b/drivers/scsi/megaraid/megaraid_sas_base.c
> index b95f187297aed..4f9a53769966f 100644
> --- a/drivers/scsi/megaraid/megaraid_sas_base.c
> +++ b/drivers/scsi/megaraid/megaraid_sas_base.c
[ ... ]
> @@ -7886,7 +7886,9 @@ megasas_resume(struct device *dev)
>  			goto fail_init_mfi;
>  	}
>  
> -	if (megasas_get_ctrl_info(instance) != DCMD_SUCCESS)
> +	scoped_guard(mutex, &instance->reset_mutex)
> +		rval = megasas_get_ctrl_info(instance);
> +	if (rval != DCMD_SUCCESS)

[Severity: Low]
This isn't a bug, but this function relies heavily on goto-based error
handling.

Does the introduction of a scope-based cleanup helper (scoped_guard) here
violate the cleanup subsystem guidelines?

The kernel cleanup guidelines state that scope-based cleanup helpers and
goto should not be mixed in the same function, as it can create confusing
ownership semantics.

>  		goto fail_init_mfi;
>  
>  	tasklet_init(&instance->isr_tasklet, instance->instancet->tasklet,
[ ... ]
> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index 2ba244cf40ac7..54f4e7d7de02e 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -6817,11 +6817,17 @@ static void ufshcd_err_handling_prepare(struct ufs_hba *hba)
>  	}
>  	/* Wait for ongoing ufshcd_queuecommand() calls to finish. */
>  	blk_mq_quiesce_tagset(&hba->host->tag_set);
> +	/*
> +	 * Internal commands are submitted on the pseudo SCSI device. Let them
> +	 * through so that the error handler can recover the link.
> +	 */
> +	blk_mq_unquiesce_queue(hba->host->pseudo_sdev->request_queue);

[Severity: High]
Can this sequence lead to a use-after-free on hba->host->pseudo_sdev?

If a device error triggers ufshcd_intr(), it schedules hba->eh_work.
Concurrently, the driver could be unbound, invoking ufshcd_remove().

ufshcd_remove() calls scsi_remove_host(), which destroys and frees the SCSI
devices, including hba->host->pseudo_sdev.

Because ufshcd_remove() does not cancel hba->eh_work until later in
ufshcd_hba_exit(), the error handler might execute
ufshcd_err_handling_prepare() while teardown is in progress.

Is it possible for the code to dereference hba->host->pseudo_sdev here
after it has already been freed?

>  	cancel_work_sync(&hba->eeh_work);
>  }
>  
>  static void ufshcd_err_handling_unprepare(struct ufs_hba *hba)
>  {
> +	blk_mq_quiesce_queue_nowait(hba->host->pseudo_sdev->request_queue);
>  	blk_mq_unquiesce_tagset(&hba->host->tag_set);
>  	ufshcd_release(hba);
>  	if (ufshcd_is_clkscaling_supported(hba))

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/ea8f769b49f7d1185f588cf5b9202f300c280af7.camel@HansenPartnership.com?part=1

  reply	other threads:[~2026-09-25 21:46 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-25 21:22 [GIT PULL] SCSI fixes for 7.3-rc4+ James Bottomley
2026-09-25 21:46 ` sashiko-bot [this message]
2026-09-25 21:55   ` Bart Van Assche
2026-09-25 23:05 ` pr-tracker-bot

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=20260925214614.0CD7E1F000FF@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=James.Bottomley@HansenPartnership.com \
    --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.