Re: [PATCH v2 0/4] scsi: ufs: core: Factor hba->outstanding_reqs helper and fix double completion

Re: [PATCH v2 0/4] scsi: ufs: core: Factor hba->outstanding_reqs helper and fix double completion

[Peter Wang (王信友)]

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

Re: [PATCH v2 0/4] scsi: ufs: core: Factor hba->outstanding_reqs helper and fix double completion

[Peter Wang]

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

Re: [PATCH v2 0/4] scsi: ufs: core: Factor hba->outstanding_reqs helper and fix double completion

[Bart Van Assche]

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.

Re: [PATCH v2 0/4] scsi: ufs: core: Factor hba->outstanding_reqs helper and fix double completion

[Peter Wang]

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>

Re: [PATCH v2 0/4] scsi: ufs: core: Factor hba->outstanding_reqs helper and fix double completion

[Stanley Jhu]

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.

Re: [PATCH v2 0/4] scsi: ufs: core: Factor hba->outstanding_reqs helper and fix double completion

[Bean Huo]

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

[PATCH v2 0/4] scsi: ufs: core: Factor hba->outstanding_reqs helper and fix double completion

[Stanley Jhu]

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
-- 
2.56.0.176.g3575e65682-goog