* [PATCH v2 1/4] scsi: ufs: core: Add a helper for clearing hba->outstanding_reqs
2026-10-17 8:43 [PATCH v2 0/4] scsi: ufs: core: Factor hba->outstanding_reqs helper and fix double completion Stanley Jhu
@ 2026-10-17 8:44 ` Stanley Jhu
2026-10-19 20:50 ` Bart Van Assche
2026-10-17 8:44 ` [PATCH v2 2/4] scsi: ufs: core: Fix double completion in ufshcd_clear_lu_cmds() Stanley Jhu
` (4 subsequent siblings)
5 siblings, 1 reply; 14+ messages in thread
From: Stanley Jhu @ 2026-10-17 8:44 UTC (permalink / raw)
To: Martin K . Petersen, linux-scsi
Cc: James E . J . Bottomley, Bart Van Assche, Alim Akhtar, Bean Huo,
Peter Wang, Can Guo, linux-kernel, stable
ufshcd_poll() and ufshcd_abort() each have their own copy of the code
that clears bits in hba->outstanding_reqs under hba->outstanding_lock.
Move that code into ufshcd_clear_outstanding_reqs() and its locked
variant __ufshcd_clear_outstanding_reqs(), and use them in both
functions. No functional change.
Cc: stable@vger.kernel.org
Suggested-by: Bart Van Assche <bvanassche@acm.org>
Link: https://lore.kernel.org/r/3779e125-9acc-42c8-9489-51b9e3c6716d@acm.org
Signed-off-by: Stanley Jhu <stanleyjhu@google.com>
---
drivers/ufs/core/ufshcd.c | 50 +++++++++++++++++++++++++++------------
1 file changed, 35 insertions(+), 15 deletions(-)
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index bcc2512bf34b..71f4b9d0a850 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -6052,6 +6052,37 @@ static void ufshcd_clear_polled(struct ufs_hba *hba,
}
}
+static unsigned long __ufshcd_clear_outstanding_reqs(struct ufs_hba *hba,
+ unsigned long mask,
+ bool from_irq)
+{
+ unsigned long completed_reqs = mask & hba->outstanding_reqs;
+
+ lockdep_assert_held(&hba->outstanding_lock);
+ WARN_ONCE(completed_reqs & ~hba->outstanding_reqs,
+ "completed: %#lx; outstanding: %#lx\n", completed_reqs,
+ hba->outstanding_reqs);
+ if (from_irq) {
+ /* Do not complete polled requests from interrupt context. */
+ ufshcd_clear_polled(hba, &completed_reqs);
+ }
+ hba->outstanding_reqs &= ~completed_reqs;
+
+ return completed_reqs;
+}
+
+static unsigned long ufshcd_clear_outstanding_reqs(struct ufs_hba *hba,
+ unsigned long mask)
+{
+ unsigned long completed_reqs, flags;
+
+ spin_lock_irqsave(&hba->outstanding_lock, flags);
+ completed_reqs = __ufshcd_clear_outstanding_reqs(hba, mask, false);
+ spin_unlock_irqrestore(&hba->outstanding_lock, flags);
+
+ return completed_reqs;
+}
+
/*
* Return: > 0 if one or more commands have been completed or 0 if no
* requests have been completed.
@@ -6071,15 +6102,9 @@ static int ufshcd_poll(struct Scsi_Host *shost, unsigned int queue_num)
spin_lock_irqsave(&hba->outstanding_lock, flags);
tr_doorbell = ufshcd_readl(hba, REG_UTP_TRANSFER_REQ_DOOR_BELL);
- completed_reqs = ~tr_doorbell & hba->outstanding_reqs;
- WARN_ONCE(completed_reqs & ~hba->outstanding_reqs,
- "completed: %#lx; outstanding: %#lx\n", completed_reqs,
- hba->outstanding_reqs);
- if (queue_num == UFSHCD_POLL_FROM_INTERRUPT_CONTEXT) {
- /* Do not complete polled requests from interrupt context. */
- ufshcd_clear_polled(hba, &completed_reqs);
- }
- hba->outstanding_reqs &= ~completed_reqs;
+ completed_reqs =
+ __ufshcd_clear_outstanding_reqs(hba, ~tr_doorbell,
+ queue_num == UFSHCD_POLL_FROM_INTERRUPT_CONTEXT);
spin_unlock_irqrestore(&hba->outstanding_lock, flags);
if (completed_reqs)
@@ -8114,7 +8139,6 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
struct ufshcd_lrb *lrbp = scsi_cmd_priv(cmd);
unsigned long flags;
int err = FAILED;
- bool outstanding;
u32 reg;
ufshcd_hold(hba);
@@ -8209,11 +8233,7 @@ static int ufshcd_abort(struct scsi_cmnd *cmd)
* Clear the corresponding bit from outstanding_reqs since the command
* has been aborted successfully.
*/
- spin_lock_irqsave(&hba->outstanding_lock, flags);
- outstanding = __test_and_clear_bit(tag, &hba->outstanding_reqs);
- spin_unlock_irqrestore(&hba->outstanding_lock, flags);
-
- if (outstanding)
+ if (ufshcd_clear_outstanding_reqs(hba, BIT(tag)))
ufshcd_release_scsi_cmd(hba, cmd);
err = SUCCESS;
--
2.56.0.176.g3575e65682-goog
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v2 1/4] scsi: ufs: core: Add a helper for clearing hba->outstanding_reqs
2026-10-17 8:44 ` [PATCH v2 1/4] scsi: ufs: core: Add a helper for clearing hba->outstanding_reqs Stanley Jhu
@ 2026-10-19 20:50 ` Bart Van Assche
0 siblings, 0 replies; 14+ messages in thread
From: Bart Van Assche @ 2026-10-19 20:50 UTC (permalink / raw)
To: Stanley Jhu, Martin K . Petersen, linux-scsi
Cc: James E . J . Bottomley, Alim Akhtar, Bean Huo, Peter Wang,
Can Guo, linux-kernel, stable
On 10/17/26 1:44 AM, Stanley Jhu wrote:
> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index bcc2512bf34b..71f4b9d0a850 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -6052,6 +6052,37 @@ static void ufshcd_clear_polled(struct ufs_hba *hba,
> }
> }
>
> +static unsigned long __ufshcd_clear_outstanding_reqs(struct ufs_hba *hba,
> + unsigned long mask,
> + bool from_irq)
> +{
> + unsigned long completed_reqs = mask & hba->outstanding_reqs;
> +
> + lockdep_assert_held(&hba->outstanding_lock);
> + WARN_ONCE(completed_reqs & ~hba->outstanding_reqs,
> + "completed: %#lx; outstanding: %#lx\n", completed_reqs,
> + hba->outstanding_reqs);
> + if (from_irq) {
> + /* Do not complete polled requests from interrupt context. */
> + ufshcd_clear_polled(hba, &completed_reqs);
> + }
> + hba->outstanding_reqs &= ~completed_reqs;
> +
> + return completed_reqs;
> +}
Please use __must_hold() instead of lockdep_assert_held(). __must_hold()
is evaluated at compile time (by Clang) while lockdep_assert_held() is
evaluated at run-time. If you don't want to depend on Clang, feel free
to use both.
> +static unsigned long ufshcd_clear_outstanding_reqs(struct ufs_hba *hba,
> + unsigned long mask)
> +{
> + unsigned long completed_reqs, flags;
> +
> + spin_lock_irqsave(&hba->outstanding_lock, flags);
> + completed_reqs = __ufshcd_clear_outstanding_reqs(hba, mask, false);
> + spin_unlock_irqrestore(&hba->outstanding_lock, flags);
> +
> + return completed_reqs;
> +}
The seven-line body of the above function can be reduced to two lines by
using guard():
static unsigned long ufshcd_clear_outstanding_reqs(struct ufs_hba *hba,
unsigned long mask)
{
guard(spinlock_irqsave)(&hba->outstanding_lock);
return __ufshcd_clear_outstanding_reqs(hba, mask, false);
}
Otherwise this patch looks good to me.
Thanks,
Bart.
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v2 2/4] scsi: ufs: core: Fix double completion in ufshcd_clear_lu_cmds()
2026-10-17 8:43 [PATCH v2 0/4] scsi: ufs: core: Factor hba->outstanding_reqs helper and fix double completion Stanley Jhu
2026-10-17 8:44 ` [PATCH v2 1/4] scsi: ufs: core: Add a helper for clearing hba->outstanding_reqs Stanley Jhu
@ 2026-10-17 8:44 ` Stanley Jhu
2026-10-17 8:44 ` [PATCH v2 3/4] scsi: ufs: core: Use ufshcd_clear_outstanding_reqs() in ufshcd_eh_timed_out() Stanley Jhu
` (3 subsequent siblings)
5 siblings, 0 replies; 14+ messages in thread
From: Stanley Jhu @ 2026-10-17 8:44 UTC (permalink / raw)
To: Martin K . Petersen, linux-scsi
Cc: James E . J . Bottomley, Bart Van Assche, Alim Akhtar, Bean Huo,
Peter Wang, Can Guo, linux-kernel, stable
In SDB mode, ufshcd_clear_lu_cmds() clears the request slot in the
controller and completes the request, but leaves its bit set in
hba->outstanding_reqs. ufshcd_poll() treats a request whose bit is set
in hba->outstanding_reqs and clear in the doorbell register as finished,
so the next ufshcd_poll() completes the same request again and unmaps
its DMA buffers twice.
Clear the bit with ufshcd_clear_outstanding_reqs() before completing the
request, and complete it only if the bit was still set. If ufshcd_poll()
has already completed the request, the bit is already clear and
ufshcd_clear_lu_cmds() skips it.
Fixes: f18fac1e2b72 ("scsi: ufs: core: Rework ufshcd_eh_device_reset_handler()")
Cc: stable@vger.kernel.org
Signed-off-by: Stanley Jhu <stanleyjhu@google.com>
---
drivers/ufs/core/ufshcd.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 71f4b9d0a850..531309c7fb79 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -7986,7 +7986,8 @@ static bool ufshcd_clear_lu_cmds(struct request *req, void *priv)
return true;
}
- ufshcd_compl_one_cqe(hba, tag, NULL);
+ if (ufshcd_clear_outstanding_reqs(hba, BIT(tag)))
+ ufshcd_compl_one_cqe(hba, tag, NULL);
return true;
}
--
2.56.0.176.g3575e65682-goog
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH v2 3/4] scsi: ufs: core: Use ufshcd_clear_outstanding_reqs() in ufshcd_eh_timed_out()
2026-10-17 8:43 [PATCH v2 0/4] scsi: ufs: core: Factor hba->outstanding_reqs helper and fix double completion Stanley Jhu
2026-10-17 8:44 ` [PATCH v2 1/4] scsi: ufs: core: Add a helper for clearing hba->outstanding_reqs Stanley Jhu
2026-10-17 8:44 ` [PATCH v2 2/4] scsi: ufs: core: Fix double completion in ufshcd_clear_lu_cmds() Stanley Jhu
@ 2026-10-17 8:44 ` Stanley Jhu
2026-10-17 8:44 ` [PATCH v2 4/4] scsi: ufs: core: Remove an always-false WARN_ONCE() Stanley Jhu
` (2 subsequent siblings)
5 siblings, 0 replies; 14+ messages in thread
From: Stanley Jhu @ 2026-10-17 8:44 UTC (permalink / raw)
To: Martin K . Petersen, linux-scsi
Cc: James E . J . Bottomley, Bart Van Assche, Alim Akhtar, Bean Huo,
Peter Wang, Can Guo, linux-kernel, stable
ufshcd_eh_timed_out() has its own copy of the code that clears the
timed-out tag in hba->outstanding_reqs. Use
ufshcd_clear_outstanding_reqs() instead. No functional change.
Signed-off-by: Stanley Jhu <stanleyjhu@google.com>
---
drivers/ufs/core/ufshcd.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 531309c7fb79..44e571757535 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -9659,12 +9659,9 @@ static enum scsi_timeout_action ufshcd_eh_timed_out(struct scsi_cmnd *scmd)
*/
if (!test_bit(SCMD_STATE_COMPLETE, &scmd->state)) {
if (!hba->mcq_enabled) {
- unsigned long flags;
struct request *rq = scsi_cmd_to_rq(scmd);
- spin_lock_irqsave(&hba->outstanding_lock, flags);
- __clear_bit(rq->tag, &hba->outstanding_reqs);
- spin_unlock_irqrestore(&hba->outstanding_lock, flags);
+ ufshcd_clear_outstanding_reqs(hba, BIT(rq->tag));
}
if (ufshcd_is_scsi_cmd(scmd)) {
--
2.56.0.176.g3575e65682-goog
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH v2 4/4] scsi: ufs: core: Remove an always-false WARN_ONCE()
2026-10-17 8:43 [PATCH v2 0/4] scsi: ufs: core: Factor hba->outstanding_reqs helper and fix double completion Stanley Jhu
` (2 preceding siblings ...)
2026-10-17 8:44 ` [PATCH v2 3/4] scsi: ufs: core: Use ufshcd_clear_outstanding_reqs() in ufshcd_eh_timed_out() Stanley Jhu
@ 2026-10-17 8:44 ` Stanley Jhu
2026-10-19 20:45 ` Bart Van Assche
2026-10-18 19:52 ` [PATCH v2 0/4] scsi: ufs: core: Factor hba->outstanding_reqs helper and fix double completion Bean Huo
2026-10-19 10:47 ` Peter Wang
5 siblings, 1 reply; 14+ messages in thread
From: Stanley Jhu @ 2026-10-17 8:44 UTC (permalink / raw)
To: Martin K . Petersen, linux-scsi
Cc: James E . J . Bottomley, Bart Van Assche, Alim Akhtar, Bean Huo,
Peter Wang, Can Guo, linux-kernel, stable
The WARN_ONCE() in __ufshcd_clear_outstanding_reqs() can never fire.
completed_reqs is set to mask & hba->outstanding_reqs on the line above,
and hba->outstanding_reqs cannot change in between because the caller
holds hba->outstanding_lock. So completed_reqs & ~hba->outstanding_reqs
is always zero. Remove it. No functional change.
Signed-off-by: Stanley Jhu <stanleyjhu@google.com>
---
drivers/ufs/core/ufshcd.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index 44e571757535..716dab2352e9 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -6059,9 +6059,6 @@ static unsigned long __ufshcd_clear_outstanding_reqs(struct ufs_hba *hba,
unsigned long completed_reqs = mask & hba->outstanding_reqs;
lockdep_assert_held(&hba->outstanding_lock);
- WARN_ONCE(completed_reqs & ~hba->outstanding_reqs,
- "completed: %#lx; outstanding: %#lx\n", completed_reqs,
- hba->outstanding_reqs);
if (from_irq) {
/* Do not complete polled requests from interrupt context. */
ufshcd_clear_polled(hba, &completed_reqs);
--
2.56.0.176.g3575e65682-goog
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v2 4/4] scsi: ufs: core: Remove an always-false WARN_ONCE()
2026-10-17 8:44 ` [PATCH v2 4/4] scsi: ufs: core: Remove an always-false WARN_ONCE() Stanley Jhu
@ 2026-10-19 20:45 ` Bart Van Assche
0 siblings, 0 replies; 14+ messages in thread
From: Bart Van Assche @ 2026-10-19 20:45 UTC (permalink / raw)
To: Stanley Jhu, Martin K . Petersen, linux-scsi
Cc: James E . J . Bottomley, Alim Akhtar, Bean Huo, Peter Wang,
Can Guo, linux-kernel, stable
On 10/17/26 1:44 AM, Stanley Jhu wrote:
> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index 44e571757535..716dab2352e9 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -6059,9 +6059,6 @@ static unsigned long __ufshcd_clear_outstanding_reqs(struct ufs_hba *hba,
> unsigned long completed_reqs = mask & hba->outstanding_reqs;
>
> lockdep_assert_held(&hba->outstanding_lock);
> - WARN_ONCE(completed_reqs & ~hba->outstanding_reqs,
> - "completed: %#lx; outstanding: %#lx\n", completed_reqs,
> - hba->outstanding_reqs);
> if (from_irq) {
> /* Do not complete polled requests from interrupt context. */
> ufshcd_clear_polled(hba, &completed_reqs);
This patch should either be the first patch in this series or should be
squashed into patch 1/4 from this series.
Thanks,
Bart.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 0/4] scsi: ufs: core: Factor hba->outstanding_reqs helper and fix double completion
2026-10-17 8:43 [PATCH v2 0/4] scsi: ufs: core: Factor hba->outstanding_reqs helper and fix double completion Stanley Jhu
` (3 preceding siblings ...)
2026-10-17 8:44 ` [PATCH v2 4/4] scsi: ufs: core: Remove an always-false WARN_ONCE() Stanley Jhu
@ 2026-10-18 19:52 ` Bean Huo
2026-10-19 4:29 ` Stanley Jhu
2026-10-19 20:43 ` Bart Van Assche
2026-10-19 10:47 ` Peter Wang
5 siblings, 2 replies; 14+ messages in thread
From: Bean Huo @ 2026-10-18 19:52 UTC (permalink / raw)
To: Stanley Jhu, Martin K . Petersen, linux-scsi
Cc: James E . J . Bottomley, Bart Van Assche, Alim Akhtar, Bean Huo,
Peter Wang, Can Guo, linux-kernel, stable
On Sat, 2026-10-17 at 16:43 +0800, Stanley Jhu wrote:
> In Single Doorbell (SDB) mode, ufshcd_clear_lu_cmds() clears a transfer
> request slot in the controller and completes the request without
> claiming its bit in hba->outstanding_reqs. A subsequent ufshcd_poll()
> call sees the stale bit with its doorbell bit cleared and completes the
> request a second time, causing a double DMA unmap.
>
> Patch 1 adds __ufshcd_clear_outstanding_reqs() and
> ufshcd_clear_outstanding_reqs() to share the bit-claim logic across
> ufshcd_poll() and ufshcd_abort() with no functional change. Patch 2 uses
> the new helper in ufshcd_clear_lu_cmds() to fix the SDB double
> completion. Patch 3 uses the new helper in ufshcd_eh_timed_out(). It is
> a follow-on cleanup and is not needed for the fix. Patch 4 removes a
> WARN_ONCE() whose condition is always false.
>
> This series is based on mkp/for-next (db8b4d2bf2d3).
>
> Tested on QEMU ARM64 UFS in SDB and MCQ modes across LU reset, polled
> and interrupt I/O, and task abort without double completion or DMA
> unmap warnings.
>
> Changes since v1:
> - Add a helper for clearing hba->outstanding_reqs and use it in
> ufshcd_poll(), ufshcd_abort(), and ufshcd_clear_lu_cmds()
> (Bart Van Assche).
> - Convert ufshcd_eh_timed_out() to use the new helper in patch 3.
> - Remove an always-false WARN_ONCE() in patch 4.
> - Link to v1:
> https://lore.kernel.org/r/20261016085403.1711740-1-stanleyjhu@google.com
>
> Stanley Jhu (4):
> scsi: ufs: core: Add a helper for clearing hba->outstanding_reqs
> scsi: ufs: core: Fix double completion in ufshcd_clear_lu_cmds()
> scsi: ufs: core: Use ufshcd_clear_outstanding_reqs() in
> ufshcd_eh_timed_out()
> scsi: ufs: core: Remove an always-false WARN_ONCE()
>
Hi Stanley,
For the whole series:
Reviewed-by: Bean Huo <beanhuo@micron.com>
One more place has the same problem. In ufshcd_abort(), the "cmd was completed,
but without a notifying intr" path completes the request but leaves its bit set
in hba->outstanding_reqs:
if (!hba->mcq_enabled && !(reg & (1 << tag))) {
...
__ufshcd_transfer_req_compl(hba, 1UL << tag);
...
}
could use the new helper in the same way as patch 2?
if (ufshcd_clear_outstanding_reqs(hba, BIT(tag)))
ufshcd_compl_one_cqe(hba, tag, NULL);
Kind regards,
Bean
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v2 0/4] scsi: ufs: core: Factor hba->outstanding_reqs helper and fix double completion
2026-10-18 19:52 ` [PATCH v2 0/4] scsi: ufs: core: Factor hba->outstanding_reqs helper and fix double completion Bean Huo
@ 2026-10-19 4:29 ` Stanley Jhu
2026-10-19 20:43 ` Bart Van Assche
1 sibling, 0 replies; 14+ messages in thread
From: Stanley Jhu @ 2026-10-19 4:29 UTC (permalink / raw)
To: Bean Huo, Martin K . Petersen, linux-scsi
Cc: James E . J . Bottomley, Bart Van Assche, Alim Akhtar, Bean Huo,
Peter Wang, Can Guo, linux-kernel, stable
On Sun, 2026-10-18 at 21:52 +0200, Bean Huo wrote:
> One more place has the same problem. In ufshcd_abort(), the "cmd was completed,
> but without a notifying intr" path completes the request but leaves its bit set
> in hba->outstanding_reqs:
Good catch, thanks. I can confirm it. I will add a separate patch to fix
it in v3.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 0/4] scsi: ufs: core: Factor hba->outstanding_reqs helper and fix double completion
2026-10-18 19:52 ` [PATCH v2 0/4] scsi: ufs: core: Factor hba->outstanding_reqs helper and fix double completion Bean Huo
2026-10-19 4:29 ` Stanley Jhu
@ 2026-10-19 20:43 ` Bart Van Assche
2026-10-20 3:16 ` Peter Wang
1 sibling, 1 reply; 14+ messages in thread
From: Bart Van Assche @ 2026-10-19 20:43 UTC (permalink / raw)
To: Bean Huo, Stanley Jhu, Martin K . Petersen, linux-scsi
Cc: James E . J . Bottomley, Alim Akhtar, Bean Huo, Peter Wang,
Can Guo, linux-kernel, stable
On 10/18/26 12:52 PM, Bean Huo wrote:
> One more place has the same problem. In ufshcd_abort(), the "cmd was completed,
> but without a notifying intr" path completes the request but leaves its bit set
> in hba->outstanding_reqs:
>
> if (!hba->mcq_enabled && !(reg & (1 << tag))) {
> ...
> __ufshcd_transfer_req_compl(hba, 1UL << tag);
> ...
> }
>
>
> could use the new helper in the same way as patch 2?
>
>
> if (ufshcd_clear_outstanding_reqs(hba, BIT(tag)))
> ufshcd_compl_one_cqe(hba, tag, NULL);
Hi Bean,
There are two problems with calling ufshcd_compl_one_cqe() from
ufshcd_abort():
* ufshcd_compl_one_cqe() must only be called for SCSI commands while
'tag' may refer to a device management command.
* SCSI commands must not be freed by a SCSI abort handler. Otherwise
a use-after-free may be triggered in scmd_eh_abort_handler() when it
tries to call scsi_queue_insert() or scsi_finish_command().
Bart.
^ permalink raw reply [flat|nested] 14+ messages in thread* Re: [PATCH v2 0/4] scsi: ufs: core: Factor hba->outstanding_reqs helper and fix double completion
2026-10-19 20:43 ` Bart Van Assche
@ 2026-10-20 3:16 ` Peter Wang
2026-10-20 8:27 ` Stanley Jhu
0 siblings, 1 reply; 14+ messages in thread
From: Peter Wang @ 2026-10-20 3:16 UTC (permalink / raw)
To: Bart Van Assche, Bean Huo, Stanley Jhu, Martin K . Petersen, linux-scsi
Cc: James E . J . Bottomley, Alim Akhtar, Bean Huo, Can Guo,
linux-kernel, stable
On Mon, 2026-10-19 at 13:43 -0700, Bart Van Assche wrote:
> Hi Bean,
>
> There are two problems with calling ufshcd_compl_one_cqe() from
> ufshcd_abort():
> * ufshcd_compl_one_cqe() must only be called for SCSI commands while
> 'tag' may refer to a device management command.
> * SCSI commands must not be freed by a SCSI abort handler. Otherwise
> a use-after-free may be triggered in scmd_eh_abort_handler() when
> it
> tries to call scsi_queue_insert() or scsi_finish_command().
>
> Bart.
Hi Bart,
So, should we remove the call to __ufshcd_transfer_req_compl from
ufshcd_abort, which was added by commit
64180742605f (“scsi: ufs: Fix the SCSI abort handler”)?
Thanks.
Peter
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 0/4] scsi: ufs: core: Factor hba->outstanding_reqs helper and fix double completion
2026-10-20 3:16 ` Peter Wang
@ 2026-10-20 8:27 ` Stanley Jhu
0 siblings, 0 replies; 14+ messages in thread
From: Stanley Jhu @ 2026-10-20 8:27 UTC (permalink / raw)
To: Peter Wang, Bart Van Assche, Bean Huo, Martin K . Petersen, linux-scsi
Cc: James E . J . Bottomley, Alim Akhtar, Bean Huo, Can Guo,
linux-kernel, stable
On Tue, 2026-10-20 at 11:16 +0800, Peter Wang wrote:
> So, should we remove the call to __ufshcd_transfer_req_compl from
> ufshcd_abort, which was added by commit
> 64180742605f ("scsi: ufs: Fix the SCSI abort handler")?
__ufshcd_transfer_req_compl() is a wrapper around
ufshcd_compl_one_cqe(), so calling it from ufshcd_abort() has the two
issues Bart noted. In addition, ufshcd_abort() called it without
clearing BIT(tag) in hba->outstanding_reqs, which left the command to be
released a second time by a later ufshcd_poll() or LU reset.
In v3 6/7 [1], we replace that __ufshcd_transfer_req_compl() call with
claiming BIT(tag) via ufshcd_clear_outstanding_reqs() and calling
ufshcd_release_scsi_cmd() if it is a SCSI command, matching the end of
ufshcd_abort().
[1] https://lore.kernel.org/r/20261020082110.3511797-7-stanleyjhu@google.com
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v2 0/4] scsi: ufs: core: Factor hba->outstanding_reqs helper and fix double completion
2026-10-17 8:43 [PATCH v2 0/4] scsi: ufs: core: Factor hba->outstanding_reqs helper and fix double completion Stanley Jhu
` (4 preceding siblings ...)
2026-10-18 19:52 ` [PATCH v2 0/4] scsi: ufs: core: Factor hba->outstanding_reqs helper and fix double completion Bean Huo
@ 2026-10-19 10:47 ` Peter Wang
5 siblings, 0 replies; 14+ messages in thread
From: Peter Wang @ 2026-10-19 10:47 UTC (permalink / raw)
To: Stanley Jhu, Martin K . Petersen, linux-scsi
Cc: James E . J . Bottomley, Bart Van Assche, Alim Akhtar, Bean Huo,
Can Guo, linux-kernel, stable
On Sat, 2026-10-17 at 16:43 +0800, Stanley Jhu wrote:
> In Single Doorbell (SDB) mode, ufshcd_clear_lu_cmds() clears a
> transfer
> request slot in the controller and completes the request without
> claiming its bit in hba->outstanding_reqs. A subsequent ufshcd_poll()
> call sees the stale bit with its doorbell bit cleared and completes
> the
> request a second time, causing a double DMA unmap.
>
> Patch 1 adds __ufshcd_clear_outstanding_reqs() and
> ufshcd_clear_outstanding_reqs() to share the bit-claim logic across
> ufshcd_poll() and ufshcd_abort() with no functional change. Patch 2
> uses
> the new helper in ufshcd_clear_lu_cmds() to fix the SDB double
> completion. Patch 3 uses the new helper in ufshcd_eh_timed_out(). It
> is
> a follow-on cleanup and is not needed for the fix. Patch 4 removes a
> WARN_ONCE() whose condition is always false.
>
> This series is based on mkp/for-next (db8b4d2bf2d3).
>
> Tested on QEMU ARM64 UFS in SDB and MCQ modes across LU reset, polled
> and interrupt I/O, and task abort without double completion or DMA
> unmap warnings.
>
> Changes since v1:
> - Add a helper for clearing hba->outstanding_reqs and use it in
> ufshcd_poll(), ufshcd_abort(), and ufshcd_clear_lu_cmds()
> (Bart Van Assche).
> - Convert ufshcd_eh_timed_out() to use the new helper in patch 3.
> - Remove an always-false WARN_ONCE() in patch 4.
> - Link to v1:
> https://lore.kernel.org/r/20261016085403.1711740-1-stanleyjhu@google.com
>
>
> Stanley Jhu (4):
> scsi: ufs: core: Add a helper for clearing hba->outstanding_reqs
> scsi: ufs: core: Fix double completion in ufshcd_clear_lu_cmds()
> scsi: ufs: core: Use ufshcd_clear_outstanding_reqs() in
> ufshcd_eh_timed_out()
> scsi: ufs: core: Remove an always-false WARN_ONCE()
>
> drivers/ufs/core/ufshcd.c | 55 +++++++++++++++++++++++++------------
> --
> 1 file changed, 35 insertions(+), 20 deletions(-)
>
>
> base-commit: db8b4d2bf2d3e02b4ff78bbf69d00e11795840de
For the series:
Reviewed-by: Peter Wang <peter.wang@mediatek.com>
^ permalink raw reply [flat|nested] 14+ messages in thread