[bug report] scsi: ufs: core: Fix runtime suspend error deadlock

[bug report] scsi: ufs: core: Fix runtime suspend error deadlock

[Dan Carpenter]

Hello Peter Wang,

Commit f966e02ae521 ("scsi: ufs: core: Fix runtime suspend error
deadlock") from Sep 26, 2025 (linux-next), leads to the following
Smatch static checker warning:

  drivers/ufs/core/ufshcd.c:6844 ufshcd_err_handler()
  warn: inconsistent returns '&hba->host_sem'.
    Locked on  : 6691
    Unlocked on: 6683,6844

drivers/ufs/core/ufshcd.c
    6658 static void ufshcd_err_handler(struct work_struct *work)
    6659 {
    6660         int retries = MAX_ERR_HANDLER_RETRIES;
    6661         struct ufs_hba *hba;
    6662         unsigned long flags;
    6663         bool needs_restore;
    6664         bool needs_reset;
    6665         int pmc_err;
    6666 
    6667         hba = container_of(work, struct ufs_hba, eh_work);
    6668 
    6669         dev_info(hba->dev,
    6670                  "%s started; HBA state %s; powered %d; shutting down %d; saved_err = 0x%x; saved_uic_err = 0x%x; force_reset = %d%s\n",
    6671                  __func__, ufshcd_state_name[hba->ufshcd_state],
    6672                  hba->is_powered, hba->shutting_down, hba->saved_err,
    6673                  hba->saved_uic_err, hba->force_reset,
    6674                  ufshcd_is_link_broken(hba) ? "; link is broken" : "");
    6675 
    6676         down(&hba->host_sem);
                 ^^^^^^^^^^^^^^^^^^^^
We're holding this semaphore.

    6677         spin_lock_irqsave(hba->host->host_lock, flags);
    6678         if (ufshcd_err_handling_should_stop(hba)) {
    6679                 if (hba->ufshcd_state != UFSHCD_STATE_ERROR)
    6680                         hba->ufshcd_state = UFSHCD_STATE_OPERATIONAL;
    6681                 spin_unlock_irqrestore(hba->host->host_lock, flags);
    6682                 up(&hba->host_sem);

Released.

    6683                 return;
    6684         }
    6685         spin_unlock_irqrestore(hba->host->host_lock, flags);
    6686 
    6687         ufshcd_rpm_get_noresume(hba);
    6688         if (hba->pm_op_in_progress) {
    6689                 ufshcd_link_recovery(hba);
    6690                 ufshcd_rpm_put(hba);
    6691                 return;

The patch introduces a new return but doesn't release the hba->host_sem.
The patch was supposed to fix a deadlock but I would have thought it
would introduce a new deadlock...

    6692         }
    6693         ufshcd_rpm_put(hba);
    6694 

regards,
dan carpenter

RE: [bug report] scsi: ufs: core: Fix runtime suspend error deadlock

[Peter Wang (王信友)]

Hi Dan,

Thanks for report.
Will fix it ASAP.

Thanks.
Peter

-----Original Message-----
From: Dan Carpenter <dan.carpenter@linaro.org> 
Sent: Friday, October 3, 2025 5:30 PM
To: Peter Wang (王信友) <peter.wang@mediatek.com>
Cc: linux-scsi@vger.kernel.org
Subject: [bug report] scsi: ufs: core: Fix runtime suspend error deadlock


External email : Please do not click links or open attachments until you have verified the sender or the content.


Hello Peter Wang,

Commit f966e02ae521 ("scsi: ufs: core: Fix runtime suspend error
deadlock") from Sep 26, 2025 (linux-next), leads to the following Smatch static checker warning:

  drivers/ufs/core/ufshcd.c:6844 ufshcd_err_handler()
  warn: inconsistent returns '&hba->host_sem'.
    Locked on  : 6691
    Unlocked on: 6683,6844

drivers/ufs/core/ufshcd.c
    6658 static void ufshcd_err_handler(struct work_struct *work)
    6659 {
    6660         int retries = MAX_ERR_HANDLER_RETRIES;
    6661         struct ufs_hba *hba;
    6662         unsigned long flags;
    6663         bool needs_restore;
    6664         bool needs_reset;
    6665         int pmc_err;
    6666
    6667         hba = container_of(work, struct ufs_hba, eh_work);
    6668
    6669         dev_info(hba->dev,
    6670                  "%s started; HBA state %s; powered %d; shutting down %d; saved_err = 0x%x; saved_uic_err = 0x%x; force_reset = %d%s\n",
    6671                  __func__, ufshcd_state_name[hba->ufshcd_state],
    6672                  hba->is_powered, hba->shutting_down, hba->saved_err,
    6673                  hba->saved_uic_err, hba->force_reset,
    6674                  ufshcd_is_link_broken(hba) ? "; link is broken" : "");
    6675
    6676         down(&hba->host_sem);
                 ^^^^^^^^^^^^^^^^^^^^
We're holding this semaphore.

    6677         spin_lock_irqsave(hba->host->host_lock, flags);
    6678         if (ufshcd_err_handling_should_stop(hba)) {
    6679                 if (hba->ufshcd_state != UFSHCD_STATE_ERROR)
    6680                         hba->ufshcd_state = UFSHCD_STATE_OPERATIONAL;
    6681                 spin_unlock_irqrestore(hba->host->host_lock, flags);
    6682                 up(&hba->host_sem);

Released.

    6683                 return;
    6684         }
    6685         spin_unlock_irqrestore(hba->host->host_lock, flags);
    6686
    6687         ufshcd_rpm_get_noresume(hba);
    6688         if (hba->pm_op_in_progress) {
    6689                 ufshcd_link_recovery(hba);
    6690                 ufshcd_rpm_put(hba);
    6691                 return;

The patch introduces a new return but doesn't release the hba->host_sem.
The patch was supposed to fix a deadlock but I would have thought it would introduce a new deadlock...

    6692         }
    6693         ufshcd_rpm_put(hba);
    6694

regards,
dan carpenter