scsi: ufs: core: Fix task management completion timeout race

Hi

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:

scsi: ufs: core: Fix task management completion timeout race
Add Bart's Reviewed-by

scsi: ufs: core: Fix another task management completion race
New patch

Adrian Hunter (2):
scsi: ufs: core: Fix task management completion timeout race
scsi: ufs: core: Fix another task management completion race

 drivers/scsi/ufs/ufshcd.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

Regards
Adrian

__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.

---
 drivers/scsi/ufs/ufshcd.c | 5 -----
 1 file changed, 5 deletions(-)

index 470affdec426..a904531ba528 100644
@@ -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

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.

---
 drivers/scsi/ufs/ufshcd.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

index a904531ba528..a1519b2b6cfe 100644
@@ -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

Adrian,

Applied to 5.16/scsi-staging, thanks!

Martin K. Petersen	Oracle Linux Engineering

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