* [PATCH V2 1/2] scsi: ufs: core: Fix task management completion timeout race
2021-11-08 6:48 [PATCH V2 0/2] scsi: ufs: core: Fix task management completion timeout race Adrian Hunter
@ 2021-11-08 6:48 ` Adrian Hunter
2021-11-08 6:48 ` [PATCH V2 2/2] scsi: ufs: core: Fix another task management completion race Adrian Hunter
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Adrian Hunter @ 2021-11-08 6:48 UTC (permalink / raw)
To: Martin K . Petersen
Cc: James E . J . Bottomley, Bean Huo, Avri Altman, Alim Akhtar,
Can Guo, Asutosh Das, Bart Van Assche, linux-scsi
__ufshcd_issue_tm_cmd() clears req->end_io_data after timing out,
which races with the completion function ufshcd_tmc_handler() which
expects req->end_io_data to have a value.
Note __ufshcd_issue_tm_cmd() and ufshcd_tmc_handler() are already
synchronized using hba->tmf_rqs and hba->outstanding_tasks under the
host_lock spinlock.
It is also not necessary (nor typical) to clear req->end_io_data because
the block layer does it before allocating out requests e.g. via
blk_get_request().
So fix by not clearing it.
Fixes: f5ef336fd2e4c3 ("scsi: ufs: core: Fix task management completion")
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
---
drivers/scsi/ufs/ufshcd.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index 470affdec426..a904531ba528 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -6616,11 +6616,6 @@ static int __ufshcd_issue_tm_cmd(struct ufs_hba *hba,
err = wait_for_completion_io_timeout(&wait,
msecs_to_jiffies(TM_CMD_TIMEOUT));
if (!err) {
- /*
- * Make sure that ufshcd_compl_tm() does not trigger a
- * use-after-free.
- */
- req->end_io_data = NULL;
ufshcd_add_tm_upiu_trace(hba, task_tag, UFS_TM_ERR);
dev_err(hba->dev, "%s: task management cmd 0x%.2x timed-out\n",
__func__, tm_function);
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH V2 2/2] scsi: ufs: core: Fix another task management completion race
2021-11-08 6:48 [PATCH V2 0/2] scsi: ufs: core: Fix task management completion timeout race Adrian Hunter
2021-11-08 6:48 ` [PATCH V2 1/2] " Adrian Hunter
@ 2021-11-08 6:48 ` Adrian Hunter
2021-11-08 17:33 ` Bart Van Assche
2021-11-09 4:13 ` [PATCH V2 0/2] scsi: ufs: core: Fix task management completion timeout race Martin K. Petersen
2021-11-19 4:16 ` Martin K. Petersen
3 siblings, 1 reply; 6+ messages in thread
From: Adrian Hunter @ 2021-11-08 6:48 UTC (permalink / raw)
To: Martin K . Petersen
Cc: James E . J . Bottomley, Bean Huo, Avri Altman, Alim Akhtar,
Can Guo, Asutosh Das, Bart Van Assche, linux-scsi
hba->outstanding_tasks, which is read under host_lock spinlock, tells
the interrupt handler what task management tags are in use by the driver.
The doorbell register bits indicate which tags are in use by the hardware.
A doorbell bit that is 0 is because the bit has yet to be set by the
driver, or because the task is complete. It is only possible to
disambiguate the 2 cases, if reading/writing the doorbell register is
synchronized with reading/writing hba->outstanding_tasks.
For that reason, reading REG_UTP_TASK_REQ_DOOR_BELL must be done under
spinlock.
Fixes: f5ef336fd2e4c3 ("scsi: ufs: core: Fix task management completion")
Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
drivers/scsi/ufs/ufshcd.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c
index a904531ba528..a1519b2b6cfe 100644
--- a/drivers/scsi/ufs/ufshcd.c
+++ b/drivers/scsi/ufs/ufshcd.c
@@ -6453,9 +6453,8 @@ static irqreturn_t ufshcd_tmc_handler(struct ufs_hba *hba)
irqreturn_t ret = IRQ_NONE;
int tag;
- pending = ufshcd_readl(hba, REG_UTP_TASK_REQ_DOOR_BELL);
-
spin_lock_irqsave(hba->host->host_lock, flags);
+ pending = ufshcd_readl(hba, REG_UTP_TASK_REQ_DOOR_BELL);
issued = hba->outstanding_tasks & ~pending;
for_each_set_bit(tag, &issued, hba->nutmrs) {
struct request *req = hba->tmf_rqs[tag];
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH V2 2/2] scsi: ufs: core: Fix another task management completion race
2021-11-08 6:48 ` [PATCH V2 2/2] scsi: ufs: core: Fix another task management completion race Adrian Hunter
@ 2021-11-08 17:33 ` Bart Van Assche
0 siblings, 0 replies; 6+ messages in thread
From: Bart Van Assche @ 2021-11-08 17:33 UTC (permalink / raw)
To: Adrian Hunter, Martin K . Petersen
Cc: James E . J . Bottomley, Bean Huo, Avri Altman, Alim Akhtar,
Can Guo, Asutosh Das, linux-scsi
On 11/7/21 10:48 PM, Adrian Hunter wrote:
> hba->outstanding_tasks, which is read under host_lock spinlock, tells
> the interrupt handler what task management tags are in use by the driver.
> The doorbell register bits indicate which tags are in use by the hardware.
> A doorbell bit that is 0 is because the bit has yet to be set by the
> driver, or because the task is complete. It is only possible to
> disambiguate the 2 cases, if reading/writing the doorbell register is
> synchronized with reading/writing hba->outstanding_tasks.
>
> For that reason, reading REG_UTP_TASK_REQ_DOOR_BELL must be done under
> spinlock.
Reviewed-by: Bart Van Assche <bvanassche@acm.org>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH V2 0/2] scsi: ufs: core: Fix task management completion timeout race
2021-11-08 6:48 [PATCH V2 0/2] scsi: ufs: core: Fix task management completion timeout race Adrian Hunter
2021-11-08 6:48 ` [PATCH V2 1/2] " Adrian Hunter
2021-11-08 6:48 ` [PATCH V2 2/2] scsi: ufs: core: Fix another task management completion race Adrian Hunter
@ 2021-11-09 4:13 ` Martin K. Petersen
2021-11-19 4:16 ` Martin K. Petersen
3 siblings, 0 replies; 6+ messages in thread
From: Martin K. Petersen @ 2021-11-09 4:13 UTC (permalink / raw)
To: Adrian Hunter
Cc: Martin K . Petersen, James E . J . Bottomley, Bean Huo,
Avri Altman, Alim Akhtar, Can Guo, Asutosh Das, Bart Van Assche,
linux-scsi
Adrian,
> I though I sent this back on 15 October, but apparently I didn't, so
> here it is now.
>
> Please consider this for fixes.
Applied to 5.16/scsi-staging, thanks!
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH V2 0/2] scsi: ufs: core: Fix task management completion timeout race
2021-11-08 6:48 [PATCH V2 0/2] scsi: ufs: core: Fix task management completion timeout race Adrian Hunter
` (2 preceding siblings ...)
2021-11-09 4:13 ` [PATCH V2 0/2] scsi: ufs: core: Fix task management completion timeout race Martin K. Petersen
@ 2021-11-19 4:16 ` Martin K. Petersen
3 siblings, 0 replies; 6+ messages in thread
From: Martin K. Petersen @ 2021-11-19 4:16 UTC (permalink / raw)
To: Adrian Hunter
Cc: Martin K . Petersen, Avri Altman, Can Guo, Bean Huo, Alim Akhtar,
linux-scsi, Bart Van Assche, Asutosh Das,
James E . J . Bottomley
On Mon, 8 Nov 2021 08:48:13 +0200, Adrian Hunter wrote:
> I though I sent this back on 15 October, but apparently I didn't, so
> here it is now.
>
> Please consider this for fixes.
>
>
> Changes in V2:
>
> [...]
Applied to 5.16/scsi-fixes, thanks!
[1/2] scsi: ufs: core: Fix task management completion timeout race
https://git.kernel.org/mkp/scsi/c/886fe2915cce
[2/2] scsi: ufs: core: Fix another task management completion race
https://git.kernel.org/mkp/scsi/c/5cb37a26355d
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 6+ messages in thread