scsi: hisi_sas: Replace del_timer() calls with del_timer_sync()

Some usage of del_timer() in the driver is potentially unsafe.

When running the sas_task->slow_task timer in
hisi_sas_exec_internal_tmf_task(), execution may be blocked in function
hisi_sas_task_exec(); so it is possible that the timer is running when the
callback to disable the timer is running. This could be dangerous, as we
immediately release resources which the timer callback uses after disabling
the timer. The same situation may be found at other sites, such as
_hisi_sas_internal_task_abort().

Change calls to del_timer() to del_timer_sync() as necessary, to ensure any
timer has finished when disabling.

Also remove calls to timer_pending() prior to del_timer() as it is not
necessary.

Link: https://lore.kernel.org/r/1629799260-120116-5-git-send-email-john.garry@huawei.com
Signed-off-by: Xiang Chen <chenxiang66@hisilicon.com>
Signed-off-by: John Garry <john.garry@huawei.com>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>

diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c b/drivers/scsi/hisi_sas/hisi_sas_main.c
index fec4db46c76ca0010983a086c26d6ed4f192e071..bb1c8ef3a76fe9929dfbd8c115bd08840f790e05 100644 (file)
--- a/drivers/scsi/hisi_sas/hisi_sas_main.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
@@ -1171,7 +1171,7 @@ static int hisi_sas_control_phy(struct asd_sas_phy *sas_phy, enum phy_func func,
 
 static void hisi_sas_task_done(struct sas_task *task)
 {
-       del_timer(&task->slow_task->timer);
+       del_timer_sync(&task->slow_task->timer);
        complete(&task->slow_task->completion);
 }
 
@@ -1229,7 +1229,7 @@ static int hisi_sas_exec_internal_tmf_task(struct domain_device *device,
                res = hisi_sas_task_exec(task, GFP_KERNEL, 1, tmf);
 
                if (res) {
-                       del_timer(&task->slow_task->timer);
+                       del_timer_sync(&task->slow_task->timer);
                        dev_err(dev, "abort tmf: executing internal task failed: %d\n",
                                res);
                        goto ex_err;
@@ -1554,8 +1554,7 @@ void hisi_sas_controller_reset_prepare(struct hisi_hba *hisi_hba)
        scsi_block_requests(shost);
        hisi_hba->hw->wait_cmds_complete_timeout(hisi_hba, 100, 5000);
 
-       if (timer_pending(&hisi_hba->timer))
-               del_timer_sync(&hisi_hba->timer);
+       del_timer_sync(&hisi_hba->timer);
 
        set_bit(HISI_SAS_REJECT_CMD_BIT, &hisi_hba->flags);
 }
@@ -2097,7 +2096,7 @@ _hisi_sas_internal_task_abort(struct hisi_hba *hisi_hba,
        res = hisi_sas_internal_abort_task_exec(hisi_hba, sas_dev->device_id,
                                                task, abort_flag, tag, dq);
        if (res) {
-               del_timer(&task->slow_task->timer);
+               del_timer_sync(&task->slow_task->timer);
                dev_err(dev, "internal task abort: executing internal task failed: %d\n",
                        res);
                goto exit;
@@ -2769,8 +2768,7 @@ int hisi_sas_remove(struct platform_device *pdev)
        struct hisi_hba *hisi_hba = sha->lldd_ha;
        struct Scsi_Host *shost = sha->core.shost;
 
-       if (timer_pending(&hisi_hba->timer))
-               del_timer(&hisi_hba->timer);
+       del_timer_sync(&hisi_hba->timer);
 
        sas_unregister_ha(sha);
        sas_remove_host(sha->core.shost);

diff --git a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
index a5fc79b3f7320b43503f2745238a3a85f45000ba..236cf65c2f97da51cf5538ced54dbf6fee36d5d9 100644 (file)
--- a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
@@ -2368,18 +2368,18 @@ static void slot_complete_v2_hw(struct hisi_hba *hisi_hba,
        case STAT_IO_COMPLETE:
                /* internal abort command complete */
                ts->stat = TMF_RESP_FUNC_SUCC;
-               del_timer(&slot->internal_abort_timer);
+               del_timer_sync(&slot->internal_abort_timer);
                goto out;
        case STAT_IO_NO_DEVICE:
                ts->stat = TMF_RESP_FUNC_COMPLETE;
-               del_timer(&slot->internal_abort_timer);
+               del_timer_sync(&slot->internal_abort_timer);
                goto out;
        case STAT_IO_NOT_VALID:
                /* abort single io, controller don't find
                 * the io need to abort
                 */
                ts->stat = TMF_RESP_FUNC_FAILED;
-               del_timer(&slot->internal_abort_timer);
+               del_timer_sync(&slot->internal_abort_timer);
                goto out;
        default:
                break;

diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
index b137e5619da166b9934fd003cf4152377aede802..cbc6c2d867458f34acc09334b7e53b1b5fe883d6 100644 (file)
--- a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
@@ -4830,8 +4830,7 @@ static void hisi_sas_v3_remove(struct pci_dev *pdev)
        struct Scsi_Host *shost = sha->core.shost;
 
        pm_runtime_get_noresume(dev);
-       if (timer_pending(&hisi_hba->timer))
-               del_timer(&hisi_hba->timer);
+       del_timer_sync(&hisi_hba->timer);
 
        sas_unregister_ha(sha);
        flush_workqueue(hisi_hba->wq);