From 1aea5d406de8806de227f79c2728899959153400 Mon Sep 17 00:00:00 2001 From: John Garry Date: Fri, 4 Feb 2022 11:12:12 +0000 Subject: [PATCH] scsi: pm8001: Set SAS_TASK_AT_INITIATOR before delivering task Both Damien and I have seen a KASAN use-after-free warn for accessing a sas_task after delivering the associated command to HW. The issue is that once the command is delivered it is completed (and freed) async, and, as such, it is not safe to touch the sas_task again in the delivery path. However the sas_task is touched again in setting the task state SAS_TASK_AT_INITIATOR flag. The semantics for that flag is not defined, specifically because it is not checked by libsas. Indeed, it is not even checked by this driver. For the sake of completeness, set that flag before delivering the command to the HW. Reported-by: Damien Le Moal Signed-off-by: John Garry diff --git a/drivers/scsi/pm8001/pm8001_hwi.c b/drivers/scsi/pm8001/pm8001_hwi.c index c814e5071712..c6f5aa27bfbf 100644 --- a/drivers/scsi/pm8001/pm8001_hwi.c +++ b/drivers/scsi/pm8001/pm8001_hwi.c @@ -1796,6 +1796,7 @@ static void pm8001_send_abort_all(struct pm8001_hba_info *pm8001_ha, task_abort.device_id = cpu_to_le32(pm8001_ha_dev->device_id); task_abort.tag = cpu_to_le32(ccb_tag); + pm8001_set_task_at_initiator(task); ret = pm8001_mpi_build_cmd(pm8001_ha, circularQ, opc, &task_abort, sizeof(task_abort), 0); if (ret) @@ -1868,6 +1869,7 @@ static void pm8001_send_read_log(struct pm8001_hba_info *pm8001_ha, sata_cmd.ncqtag_atap_dir_m |= ((0x1 << 7) | (0x5 << 9)); memcpy(&sata_cmd.sata_fis, &fis, sizeof(struct host_to_dev_fis)); + pm8001_set_task_at_initiator(task); res = pm8001_mpi_build_cmd(pm8001_ha, circularQ, opc, &sata_cmd, sizeof(sata_cmd), 0); if (res) { @@ -4198,6 +4200,8 @@ static int pm8001_chip_smp_req(struct pm8001_hba_info *pm8001_ha, smp_cmd.long_smp_req.long_resp_size = cpu_to_le32((u32)sg_dma_len(&task->smp_task.smp_resp)-4); build_smp_cmd(pm8001_dev->device_id, smp_cmd.tag, &smp_cmd); + + pm8001_set_task_at_initiator(task); rc = pm8001_mpi_build_cmd(pm8001_ha, circularQ, opc, &smp_cmd, sizeof(smp_cmd), 0); if (rc) @@ -4266,6 +4270,7 @@ static int pm8001_chip_ssp_io_req(struct pm8001_hba_info *pm8001_ha, ssp_cmd.len = cpu_to_le32(task->total_xfer_len); ssp_cmd.esgl = 0; } + pm8001_set_task_at_initiator(task); ret = pm8001_mpi_build_cmd(pm8001_ha, circularQ, opc, &ssp_cmd, sizeof(ssp_cmd), 0); return ret; @@ -4375,6 +4380,7 @@ static int pm8001_chip_sata_req(struct pm8001_hba_info *pm8001_ha, } } + pm8001_set_task_at_initiator(task); ret = pm8001_mpi_build_cmd(pm8001_ha, circularQ, opc, &sata_cmd, sizeof(sata_cmd), 0); return ret; @@ -4646,6 +4652,7 @@ int pm8001_chip_ssp_tm_req(struct pm8001_hba_info *pm8001_ha, if (pm8001_ha->chip_id != chip_8001) sspTMCmd.ds_ads_m = 0x08; circularQ = &pm8001_ha->inbnd_q_tbl[0]; + pm8001_set_task_at_initiator(task); ret = pm8001_mpi_build_cmd(pm8001_ha, circularQ, opc, &sspTMCmd, sizeof(sspTMCmd), 0); return ret; diff --git a/drivers/scsi/pm8001/pm8001_sas.c b/drivers/scsi/pm8001/pm8001_sas.c index cec1ad47bf5b..104fb4fe3526 100644 --- a/drivers/scsi/pm8001/pm8001_sas.c +++ b/drivers/scsi/pm8001/pm8001_sas.c @@ -488,9 +488,6 @@ static int pm8001_task_exec(struct sas_task *task, } mdelay(10); /* TODO: select normal or high priority */ - spin_lock(&t->task_state_lock); - t->task_state_flags |= SAS_TASK_AT_INITIATOR; - spin_unlock(&t->task_state_lock); } while (0); rc = 0; goto out_done; diff --git a/drivers/scsi/pm8001/pm8001_sas.h b/drivers/scsi/pm8001/pm8001_sas.h index a17da1cebce1..a7da0cb35a77 100644 --- a/drivers/scsi/pm8001/pm8001_sas.h +++ b/drivers/scsi/pm8001/pm8001_sas.h @@ -748,5 +748,12 @@ pm8001_ccb_task_free_done(struct pm8001_hba_info *pm8001_ha, task->task_done(task); } +static inline void pm8001_set_task_at_initiator(struct sas_task *t) +{ + spin_lock(&t->task_state_lock); + t->task_state_flags |= SAS_TASK_AT_INITIATOR; + spin_unlock(&t->task_state_lock); +} + #endif diff --git a/drivers/scsi/pm8001/pm80xx_hwi.c b/drivers/scsi/pm8001/pm80xx_hwi.c index 2530d1365556..11b911fac2c8 100644 --- a/drivers/scsi/pm8001/pm80xx_hwi.c +++ b/drivers/scsi/pm8001/pm80xx_hwi.c @@ -1809,6 +1809,7 @@ static void pm80xx_send_abort_all(struct pm8001_hba_info *pm8001_ha, task_abort.device_id = cpu_to_le32(pm8001_ha_dev->device_id); task_abort.tag = cpu_to_le32(ccb_tag); + pm8001_set_task_at_initiator(task); ret = pm8001_mpi_build_cmd(pm8001_ha, circularQ, opc, &task_abort, sizeof(task_abort), 0); pm8001_dbg(pm8001_ha, FAIL, "Executing abort task end\n"); @@ -1885,6 +1886,7 @@ static void pm80xx_send_read_log(struct pm8001_hba_info *pm8001_ha, sata_cmd.ncqtag_atap_dir_m_dad |= ((0x1 << 7) | (0x5 << 9)); memcpy(&sata_cmd.sata_fis, &fis, sizeof(struct host_to_dev_fis)); + pm8001_set_task_at_initiator(task); res = pm8001_mpi_build_cmd(pm8001_ha, circularQ, opc, &sata_cmd, sizeof(sata_cmd), 0); pm8001_dbg(pm8001_ha, FAIL, "Executing read log end\n"); @@ -4342,6 +4344,7 @@ static int pm80xx_chip_smp_req(struct pm8001_hba_info *pm8001_ha, kunmap_atomic(to); build_smp_cmd(pm8001_dev->device_id, smp_cmd.tag, &smp_cmd, pm8001_ha->smp_exp_mode, length); + pm8001_set_task_at_initiator(task); rc = pm8001_mpi_build_cmd(pm8001_ha, circularQ, opc, &smp_cmd, sizeof(smp_cmd), 0); if (rc) @@ -4538,6 +4541,7 @@ static int pm80xx_chip_ssp_io_req(struct pm8001_hba_info *pm8001_ha, ssp_cmd.esgl = 0; } } + pm8001_set_task_at_initiator(task); ret = pm8001_mpi_build_cmd(pm8001_ha, circularQ, opc, &ssp_cmd, sizeof(ssp_cmd), q_index); return ret; @@ -4774,6 +4778,7 @@ static int pm80xx_chip_sata_req(struct pm8001_hba_info *pm8001_ha, ccb->ccb_tag, opc, qc ? qc->tf.command : 0, // ata opcode ccb->device ? atomic_read(&ccb->device->running_req) : 0); + pm8001_set_task_at_initiator(task); ret = pm8001_mpi_build_cmd(pm8001_ha, circularQ, opc, &sata_cmd, sizeof(sata_cmd), q_index); return ret; -- 2.26.2