From 368fb8a50c9a26a5632f502310066bb74daa0005 Mon Sep 17 00:00:00 2001 From: "tanghuan@vivo.com" Date: Tue, 9 May 2023 20:55:30 +0800 Subject: [PATCH] BACKPORT: scsi: ufs: fix a race condition related to device management If a device management command completion happens after wait_for_completion_timeout() times out and before ufshcd_clear_cmds() is called, then the completion code may crash on the complete() call in __ufshcd_transfer_req_compl(). Fix the following crash: Unable to handle kernel NULL pointer dereference at virtual address 0000000000000008 Call trace: complete+0x64/0x178 __ufshcd_transfer_req_compl+0x30c/0x9c0 ufshcd_poll+0xf0/0x208 ufshcd_sl_intr+0xb8/0xf0 ufshcd_intr+0x168/0x2f4 __handle_irq_event_percpu+0xa0/0x30c handle_irq_event+0x84/0x178 handle_fasteoi_irq+0x150/0x2e8 __handle_domain_irq+0x114/0x1e4 gic_handle_irq.31846+0x58/0x300 el1_irq+0xe4/0x1c0 efi_header_end+0x110/0x680 __irq_exit_rcu+0x108/0x124 __handle_domain_irq+0x118/0x1e4 gic_handle_irq.31846+0x58/0x300 el1_irq+0xe4/0x1c0 cpuidle_enter_state+0x3ac/0x8c4 do_idle+0x2fc/0x55c cpu_startup_entry+0x84/0x90 kernel_init+0x0/0x310 start_kernel+0x0/0x608 start_kernel+0x4ec/0x608 Bug:280957963 Change-Id: I317f9a92ddbd38a952328ce132a60228f41651be (cherry picked from commit f5c2976e0cb0f6236013bfb479868531b04f61d4) Link: https://lore.kernel.org/r/20220720170228.1598842-1-bvanassche@acm.org Fixes: 5a0b0cb9bee7 ("[SCSI] ufs: Add support for sending NOP OUT UPIU") Cc: Adrian Hunter Cc: Avri Altman Cc: Bean Huo Cc: Stanley Chu Signed-off-by: Bart Van Assche Signed-off-by: Martin K. Petersen Signed-off-by: tanghuan@vivo.com --- drivers/scsi/ufs/ufshcd.c | 66 ++++++++++++++++++++++++--------------- 1 file changed, 40 insertions(+), 26 deletions(-) diff --git a/drivers/scsi/ufs/ufshcd.c b/drivers/scsi/ufs/ufshcd.c index 1812edbb7d72..625ea25580da 100644 --- a/drivers/scsi/ufs/ufshcd.c +++ b/drivers/scsi/ufs/ufshcd.c @@ -730,16 +730,6 @@ static inline void ufshcd_utmrl_clear(struct ufs_hba *hba, u32 pos) ufshcd_writel(hba, ~(1 << pos), REG_UTP_TASK_REQ_LIST_CLEAR); } -/** - * ufshcd_outstanding_req_clear - Clear a bit in outstanding request field - * @hba: per adapter instance - * @tag: position of the bit to be cleared - */ -static inline void ufshcd_outstanding_req_clear(struct ufs_hba *hba, int tag) -{ - clear_bit(tag, &hba->outstanding_reqs); -} - /** * ufshcd_get_lists_status - Check UCRDY, UTRLRDY and UTMRLRDY * @reg: Register value of host controller status @@ -2882,37 +2872,61 @@ ufshcd_dev_cmd_completion(struct ufs_hba *hba, struct ufshcd_lrb *lrbp) static int ufshcd_wait_for_dev_cmd(struct ufs_hba *hba, struct ufshcd_lrb *lrbp, int max_timeout) { - int err = 0; - unsigned long time_left; + unsigned long time_left = msecs_to_jiffies(max_timeout); unsigned long flags; + bool pending; + int err; +retry: time_left = wait_for_completion_timeout(hba->dev_cmd.complete, - msecs_to_jiffies(max_timeout)); + time_left); /* Make sure descriptors are ready before ringing the doorbell */ wmb(); - spin_lock_irqsave(hba->host->host_lock, flags); - hba->dev_cmd.complete = NULL; if (likely(time_left)) { + /* + * The completion handler called complete() and the caller of + * this function still owns the @lrbp tag so the code below does + * not trigger any race conditions. + */ + hba->dev_cmd.complete = NULL; err = ufshcd_get_tr_ocs(lrbp); if (!err) err = ufshcd_dev_cmd_completion(hba, lrbp); - } - spin_unlock_irqrestore(hba->host->host_lock, flags); - - if (!time_left) { + } else { err = -ETIMEDOUT; dev_dbg(hba->dev, "%s: dev_cmd request timedout, tag %d\n", __func__, lrbp->task_tag); - if (!ufshcd_clear_cmd(hba, lrbp->task_tag)) + if (ufshcd_clear_cmd(hba, lrbp->task_tag) == 0) { /* successfully cleared the command, retry if needed */ err = -EAGAIN; - /* - * in case of an error, after clearing the doorbell, - * we also need to clear the outstanding_request - * field in hba - */ - ufshcd_outstanding_req_clear(hba, lrbp->task_tag); + /* + * Since clearing the command succeeded we also need to + * clear the task tag bit from the outstanding_reqs + * variable. + */ + spin_lock_irqsave(hba->host->host_lock, flags); + pending = test_bit(lrbp->task_tag, + &hba->outstanding_reqs); + if (pending) { + hba->dev_cmd.complete = NULL; + __clear_bit(lrbp->task_tag, + &hba->outstanding_reqs); + } + spin_unlock_irqrestore(hba->host->host_lock, flags); + + if (!pending) { + /* + * The completion handler ran while we tried to + * clear the command. + */ + time_left = 1; + goto retry; + } + } else { + dev_err(hba->dev, "%s: failed to clear tag %d\n", + __func__, lrbp->task_tag); + } } return err;