| Message ID | 1643289172-165636-3-git-send-email-john.garry@huawei.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | scsi: pm8001: Documentation and use-after-free fixes | expand |
On 1/27/22 22:12, John Garry wrote: > Currently a use-after-free may occur if a TMF sas_task is aborted before > we handle the IO completion in mpi_ssp_completion(). The abort occurs due > to timeout. > > When the timeout occurs, the SAS_TASK_STATE_ABORTED flag is set and the > sas_task is freed in pm8001_exec_internal_tmf_task(). > > However, if the IO completion occurs later, the IO completion still thinks > that the sas_task is available. Fix this by clearing the ccb->task if > the TMF times out - the IO completion handler does nothing if this pointer > is cleared. > > Signed-off-by: John Garry <john.garry@huawei.com> > --- > > Note: For hisi_sas driver we already do something similar. However there > we also flush the completion queue interrupt to ensure that there is no > race in clearing the task pointer. Please advise if/how something similar > can be done here. > > drivers/scsi/pm8001/pm8001_sas.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/drivers/scsi/pm8001/pm8001_sas.c b/drivers/scsi/pm8001/pm8001_sas.c > index 160ee8b228c9..32edda3e55c6 100644 > --- a/drivers/scsi/pm8001/pm8001_sas.c > +++ b/drivers/scsi/pm8001/pm8001_sas.c > @@ -769,8 +769,13 @@ static int pm8001_exec_internal_tmf_task(struct domain_device *dev, > res = -TMF_RESP_FUNC_FAILED; > /* Even TMF timed out, return direct. */ > if (task->task_state_flags & SAS_TASK_STATE_ABORTED) { > + struct pm8001_ccb_info *ccb = task->lldd_task; > + > pm8001_dbg(pm8001_ha, FAIL, "TMF task[%x]timeout.\n", > tmf->tmf); > + > + if (ccb) > + ccb->task = NULL; > goto ex_err; > } > Reviewed-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
On Thu, Jan 27, 2022 at 2:18 PM John Garry <john.garry@huawei.com> wrote: > > Currently a use-after-free may occur if a TMF sas_task is aborted before > we handle the IO completion in mpi_ssp_completion(). The abort occurs due > to timeout. > > When the timeout occurs, the SAS_TASK_STATE_ABORTED flag is set and the > sas_task is freed in pm8001_exec_internal_tmf_task(). > > However, if the IO completion occurs later, the IO completion still thinks > that the sas_task is available. Fix this by clearing the ccb->task if > the TMF times out - the IO completion handler does nothing if this pointer > is cleared. > > Signed-off-by: John Garry <john.garry@huawei.com> > --- > > Note: For hisi_sas driver we already do something similar. However there > we also flush the completion queue interrupt to ensure that there is no > race in clearing the task pointer. Please advise if/how something similar > can be done here. Not I'm aware of, but microchip guys know better. Thx John! Acked-by: Jack Wang <jinpu.wang@ionos.com> > > drivers/scsi/pm8001/pm8001_sas.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/drivers/scsi/pm8001/pm8001_sas.c b/drivers/scsi/pm8001/pm8001_sas.c > index 160ee8b228c9..32edda3e55c6 100644 > --- a/drivers/scsi/pm8001/pm8001_sas.c > +++ b/drivers/scsi/pm8001/pm8001_sas.c > @@ -769,8 +769,13 @@ static int pm8001_exec_internal_tmf_task(struct domain_device *dev, > res = -TMF_RESP_FUNC_FAILED; > /* Even TMF timed out, return direct. */ > if (task->task_state_flags & SAS_TASK_STATE_ABORTED) { > + struct pm8001_ccb_info *ccb = task->lldd_task; > + > pm8001_dbg(pm8001_ha, FAIL, "TMF task[%x]timeout.\n", > tmf->tmf); > + > + if (ccb) > + ccb->task = NULL; > goto ex_err; > } > > -- > 2.26.2 >
diff --git a/drivers/scsi/pm8001/pm8001_sas.c b/drivers/scsi/pm8001/pm8001_sas.c index 160ee8b228c9..32edda3e55c6 100644 --- a/drivers/scsi/pm8001/pm8001_sas.c +++ b/drivers/scsi/pm8001/pm8001_sas.c @@ -769,8 +769,13 @@ static int pm8001_exec_internal_tmf_task(struct domain_device *dev, res = -TMF_RESP_FUNC_FAILED; /* Even TMF timed out, return direct. */ if (task->task_state_flags & SAS_TASK_STATE_ABORTED) { + struct pm8001_ccb_info *ccb = task->lldd_task; + pm8001_dbg(pm8001_ha, FAIL, "TMF task[%x]timeout.\n", tmf->tmf); + + if (ccb) + ccb->task = NULL; goto ex_err; }
Currently a use-after-free may occur if a TMF sas_task is aborted before we handle the IO completion in mpi_ssp_completion(). The abort occurs due to timeout. When the timeout occurs, the SAS_TASK_STATE_ABORTED flag is set and the sas_task is freed in pm8001_exec_internal_tmf_task(). However, if the IO completion occurs later, the IO completion still thinks that the sas_task is available. Fix this by clearing the ccb->task if the TMF times out - the IO completion handler does nothing if this pointer is cleared. Signed-off-by: John Garry <john.garry@huawei.com> --- Note: For hisi_sas driver we already do something similar. However there we also flush the completion queue interrupt to ensure that there is no race in clearing the task pointer. Please advise if/how something similar can be done here. drivers/scsi/pm8001/pm8001_sas.c | 5 +++++ 1 file changed, 5 insertions(+)