From: Wang Chao wangchao342@hisilicon.com
driver inclusion category: bugfix bugzilla: NA CVE: NA
When device gone, it will check whether it is during reset, if not, it will send internal task abort. Before internal task abort returned, reset begins, and it will check whether SAS_PHY_UNUSED is set, if not, it will call hisi_sas_init_device(), but at that time domain_device may already be freed or part of it is freed, so it may referenece null pointer in hisi_sas_init_device(). It may occur as follows:
thread0 thread1 hisi_sas_dev_gone() check whether in RESET(no) internal task abort reset prep soft_reset ... (part of reset_done) internal task abort failed release resource anyway clear_itct device->lldd_dev=NULL hisi_sas_reset_init_all_device check sas_dev->dev_type is SAS_PHY_UNUSED and !device set dev_type SAS_PHY_UNUSED sas_free_device hisi_sas_init_device ...
Semaphore hisi_hba.sema is used to sync the processes of device gone and host reset.
To solve the issue, expand the scope that semaphore protects and let them never occur together.
And also some places will check whether domain_device is NULL to judge whether the device is gone. So when device gone, need to clear sas_dev->sas_device.
Link: https://lore.kernel.org/r/1567774537-20003-14-git-send-email-john.garry@huaw... Signed-off-by: Wang Chao wangchao342@hisilicon.com Reviewed-by: Zhu Xiongxiong zhuxiongxiong@huawei.com Signed-off-by: Yang Yingliang yangyingliang@huawei.com --- drivers/scsi/hisi_sas/hisi_sas_main.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c b/drivers/scsi/hisi_sas/hisi_sas_main.c index 396eb230ffc1..154df004b8a0 100644 --- a/drivers/scsi/hisi_sas/hisi_sas_main.c +++ b/drivers/scsi/hisi_sas/hisi_sas_main.c @@ -1069,6 +1069,7 @@ static void hisi_sas_dev_gone(struct domain_device *device) dev_info(dev, "dev[%d:%x] is gone\n", sas_dev->device_id, sas_dev->dev_type);
+ down(&hisi_hba->sem); if (!test_bit(HISI_SAS_RESET_BIT, &hisi_hba->flags)) { rc = hisi_sas_internal_task_abort(hisi_hba, device, HISI_SAS_INT_ABT_DEV, 0); @@ -1079,15 +1080,15 @@ static void hisi_sas_dev_gone(struct domain_device *device) dev_info(dev, "dev gone: release remain resources anyway.\n"); }
- down(&hisi_hba->sem); hisi_hba->hw->clear_itct(hisi_hba, sas_dev); - up(&hisi_hba->sem); device->lldd_dev = NULL; }
if (hisi_hba->hw->free_device) hisi_hba->hw->free_device(sas_dev); sas_dev->dev_type = SAS_PHY_UNUSED; + sas_dev->sas_device = NULL; + up(&hisi_hba->sem); if (rc == -EIO) { dev_err(dev, "internal abort timeout for dev gone.\n"); queue_work(hisi_hba->wq, &hisi_hba->rst_work); @@ -1586,11 +1587,11 @@ void hisi_sas_controller_reset_done(struct hisi_hba *hisi_hba) msleep(1000); hisi_sas_refresh_port_id(hisi_hba); clear_bit(HISI_SAS_REJECT_CMD_BIT, &hisi_hba->flags); - up(&hisi_hba->sem);
if (hisi_hba->reject_stp_links_msk) hisi_sas_terminate_stp_reject(hisi_hba); hisi_sas_reset_init_all_devices(hisi_hba); + up(&hisi_hba->sem); scsi_unblock_requests(shost); clear_bit(HISI_SAS_RESET_BIT, &hisi_hba->flags);