* [PATCH 0/2] scsi: libsas: Drop SAS_TASK_AT_INITIATOR
@ 2022-02-10 10:43 John Garry
2022-02-10 10:43 ` [PATCH 1/2] scsi: isci: Drop SAS_TASK_AT_INITIATOR check in isci_task_abort_task() John Garry
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: John Garry @ 2022-02-10 10:43 UTC (permalink / raw)
To: jejb, martin.petersen, artur.paszkiewicz, jinpu.wang, damien.lemoal
Cc: chenxiang66, linux-scsi, linux-kernel, linuxarm, hch, John Garry
Apart from some isci driver code, flag SAS_TASK_AT_INITIATOR is only set,
so drop usage in that driver and then everywhere else.
This solves a use-after-free in the pm8001 queue path.
John Garry (2):
scsi: isci: Drop SAS_TASK_AT_INITIATOR check in isci_task_abort_task()
scsi: libsas: Drop SAS_TASK_AT_INITIATOR
drivers/scsi/aic94xx/aic94xx_task.c | 9 ---------
drivers/scsi/hisi_sas/hisi_sas_main.c | 8 +-------
drivers/scsi/hisi_sas/hisi_sas_v1_hw.c | 3 +--
drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 3 +--
drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 3 +--
drivers/scsi/isci/request.c | 15 +++++----------
drivers/scsi/isci/request.h | 5 ++++-
drivers/scsi/isci/task.c | 23 +++++++++--------------
drivers/scsi/mvsas/mv_sas.c | 6 +-----
drivers/scsi/pm8001/pm8001_hwi.c | 7 -------
drivers/scsi/pm8001/pm8001_sas.c | 4 ----
drivers/scsi/pm8001/pm80xx_hwi.c | 5 -----
include/scsi/libsas.h | 1 -
13 files changed, 23 insertions(+), 69 deletions(-)
--
2.26.2
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/2] scsi: isci: Drop SAS_TASK_AT_INITIATOR check in isci_task_abort_task()
2022-02-10 10:43 [PATCH 0/2] scsi: libsas: Drop SAS_TASK_AT_INITIATOR John Garry
@ 2022-02-10 10:43 ` John Garry
2022-02-10 10:43 ` [PATCH 2/2] scsi: libsas: Drop SAS_TASK_AT_INITIATOR John Garry
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: John Garry @ 2022-02-10 10:43 UTC (permalink / raw)
To: jejb, martin.petersen, artur.paszkiewicz, jinpu.wang, damien.lemoal
Cc: chenxiang66, linux-scsi, linux-kernel, linuxarm, hch, John Garry
In the queue path, move around when we assign sas_task->lldd_task such
that this pointer and the SAS_TASK_AT_INITIATOR flag are set atomically.
It is also not required to clear SAS_TASK_AT_INITIATOR in
isci_task_execute_task() error path as it is also cleared immediately
after in isci_task_refuse() call.
Now the following items may be considered:
- SAS_TASK_STATE_DONE and SAS_TASK_AT_INITIATOR are mutually exclusive
apart from possibly when SAS_TASK_STATE_DONE is set in
sas_scsi_find_task(), but that is after .lldd_abort_task, i.e. the
considered callback, is called.
- If isci_task_refuse() is called in the queue path, then
sas_task->lldd_task and SAS_TASK_AT_INITIATOR are cleared atomically
in isci_task_refuse().
- In the completion path, SAS_TASK_STATE_DONE is set and
SAS_TASK_AT_INITIATOR is cleared atomically before the sas_task.lldd_task
is cleared later.
So in isci_task_abort_task() if SAS_TASK_STATE_DONE is not set and
sas_task.lldd_task is still set, then SAS_TASK_AT_INITIATOR must be set -
so we can drop this check on SAS_TASK_AT_INITIATOR.
Signed-off-by: John Garry <john.garry@huawei.com>
---
drivers/scsi/isci/request.c | 12 ++++--------
drivers/scsi/isci/request.h | 5 ++++-
drivers/scsi/isci/task.c | 13 ++++++-------
3 files changed, 14 insertions(+), 16 deletions(-)
diff --git a/drivers/scsi/isci/request.c b/drivers/scsi/isci/request.c
index fcaa84a3c210..b48ec64f745a 100644
--- a/drivers/scsi/isci/request.c
+++ b/drivers/scsi/isci/request.c
@@ -3406,9 +3406,9 @@ static struct isci_request *isci_request_from_tag(struct isci_host *ihost, u16 t
return ireq;
}
-static struct isci_request *isci_io_request_from_tag(struct isci_host *ihost,
- struct sas_task *task,
- u16 tag)
+struct isci_request *isci_io_request_from_tag(struct isci_host *ihost,
+ struct sas_task *task,
+ u16 tag)
{
struct isci_request *ireq;
@@ -3434,16 +3434,12 @@ struct isci_request *isci_tmf_request_from_tag(struct isci_host *ihost,
}
int isci_request_execute(struct isci_host *ihost, struct isci_remote_device *idev,
- struct sas_task *task, u16 tag)
+ struct sas_task *task, struct isci_request * ireq)
{
enum sci_status status;
- struct isci_request *ireq;
unsigned long flags;
int ret = 0;
- /* do common allocation and init of request object. */
- ireq = isci_io_request_from_tag(ihost, task, tag);
-
status = isci_io_request_build(ihost, ireq, idev);
if (status != SCI_SUCCESS) {
dev_dbg(&ihost->pdev->dev,
diff --git a/drivers/scsi/isci/request.h b/drivers/scsi/isci/request.h
index aff95317fcf4..40b71b3fd03e 100644
--- a/drivers/scsi/isci/request.h
+++ b/drivers/scsi/isci/request.h
@@ -291,7 +291,10 @@ struct isci_request *isci_tmf_request_from_tag(struct isci_host *ihost,
struct isci_tmf *isci_tmf,
u16 tag);
int isci_request_execute(struct isci_host *ihost, struct isci_remote_device *idev,
- struct sas_task *task, u16 tag);
+ struct sas_task *task, struct isci_request * ireq);
+struct isci_request *isci_io_request_from_tag(struct isci_host *ihost,
+ struct sas_task *task,
+ u16 tag);
enum sci_status
sci_task_request_construct(struct isci_host *ihost,
struct isci_remote_device *idev,
diff --git a/drivers/scsi/isci/task.c b/drivers/scsi/isci/task.c
index 3fd88d72a0c0..14738702d4c9 100644
--- a/drivers/scsi/isci/task.c
+++ b/drivers/scsi/isci/task.c
@@ -162,18 +162,18 @@ int isci_task_execute_task(struct sas_task *task, gfp_t gfp_flags)
SAS_TASK_UNDELIVERED,
SAS_SAM_STAT_TASK_ABORTED);
} else {
+ struct isci_request *ireq;
+
task->task_state_flags |= SAS_TASK_AT_INITIATOR;
+ /* do common allocation and init of request object. */
+ ireq = isci_io_request_from_tag(ihost, task, tag);
spin_unlock_irqrestore(&task->task_state_lock, flags);
/* build and send the request. */
- status = isci_request_execute(ihost, idev, task, tag);
+ /* do common allocation and init of request object. */
+ status = isci_request_execute(ihost, idev, task, ireq);
if (status != SCI_SUCCESS) {
- spin_lock_irqsave(&task->task_state_lock, flags);
- /* Did not really start this command. */
- task->task_state_flags &= ~SAS_TASK_AT_INITIATOR;
- spin_unlock_irqrestore(&task->task_state_lock, flags);
-
if (test_bit(IDEV_GONE, &idev->flags)) {
/* Indicate that the device
* is gone.
@@ -498,7 +498,6 @@ int isci_task_abort_task(struct sas_task *task)
/* If task is already done, the request isn't valid */
if (!(task->task_state_flags & SAS_TASK_STATE_DONE) &&
- (task->task_state_flags & SAS_TASK_AT_INITIATOR) &&
old_request) {
idev = isci_get_device(task->dev->lldd_dev);
target_done_already = test_bit(IREQ_COMPLETE_IN_TARGET,
--
2.26.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/2] scsi: libsas: Drop SAS_TASK_AT_INITIATOR
2022-02-10 10:43 [PATCH 0/2] scsi: libsas: Drop SAS_TASK_AT_INITIATOR John Garry
2022-02-10 10:43 ` [PATCH 1/2] scsi: isci: Drop SAS_TASK_AT_INITIATOR check in isci_task_abort_task() John Garry
@ 2022-02-10 10:43 ` John Garry
2022-02-10 11:46 ` Damien Le Moal
2022-02-11 21:57 ` [PATCH 0/2] " Martin K. Petersen
2022-02-15 3:19 ` Martin K. Petersen
3 siblings, 1 reply; 6+ messages in thread
From: John Garry @ 2022-02-10 10:43 UTC (permalink / raw)
To: jejb, martin.petersen, artur.paszkiewicz, jinpu.wang, damien.lemoal
Cc: chenxiang66, linux-scsi, linux-kernel, linuxarm, hch, John Garry
This flag is now only ever set, so delete it.
This also avoids a use-after-free in the pm8001 queue path, as reported
in the following:
https://lore.kernel.org/linux-scsi/c3cb7228-254e-9584-182b-007ac5e6fe0a@huawei.com/T/#m28c94c6d3ff582ec4a9fa54819180740e8bd4cfb
https://lore.kernel.org/linux-scsi/0cc0c435-b4f2-9c76-258d-865ba50a29dd@huawei.com/
Signed-off-by: John Garry <john.garry@huawei.com>
---
drivers/scsi/aic94xx/aic94xx_task.c | 9 ---------
drivers/scsi/hisi_sas/hisi_sas_main.c | 8 +-------
drivers/scsi/hisi_sas/hisi_sas_v1_hw.c | 3 +--
drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 3 +--
drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 3 +--
drivers/scsi/isci/request.c | 3 +--
drivers/scsi/isci/task.c | 10 +++-------
drivers/scsi/mvsas/mv_sas.c | 6 +-----
drivers/scsi/pm8001/pm8001_hwi.c | 7 -------
drivers/scsi/pm8001/pm8001_sas.c | 4 ----
drivers/scsi/pm8001/pm80xx_hwi.c | 5 -----
include/scsi/libsas.h | 1 -
12 files changed, 9 insertions(+), 53 deletions(-)
diff --git a/drivers/scsi/aic94xx/aic94xx_task.c b/drivers/scsi/aic94xx/aic94xx_task.c
index c6b63eae28f5..ed119a3f6f2e 100644
--- a/drivers/scsi/aic94xx/aic94xx_task.c
+++ b/drivers/scsi/aic94xx/aic94xx_task.c
@@ -322,7 +322,6 @@ static void asd_task_tasklet_complete(struct asd_ascb *ascb,
spin_lock_irqsave(&task->task_state_lock, flags);
task->task_state_flags &= ~SAS_TASK_STATE_PENDING;
- task->task_state_flags &= ~SAS_TASK_AT_INITIATOR;
task->task_state_flags |= SAS_TASK_STATE_DONE;
if (unlikely((task->task_state_flags & SAS_TASK_STATE_ABORTED))) {
struct completion *completion = ascb->completion;
@@ -532,7 +531,6 @@ int asd_execute_task(struct sas_task *task, gfp_t gfp_flags)
struct sas_task *t = task;
struct asd_ascb *ascb = NULL, *a;
struct asd_ha_struct *asd_ha = task->dev->port->ha->lldd_ha;
- unsigned long flags;
res = asd_can_queue(asd_ha, 1);
if (res)
@@ -575,10 +573,6 @@ int asd_execute_task(struct sas_task *task, gfp_t gfp_flags)
}
if (res)
goto out_err_unmap;
-
- spin_lock_irqsave(&t->task_state_lock, flags);
- t->task_state_flags |= SAS_TASK_AT_INITIATOR;
- spin_unlock_irqrestore(&t->task_state_lock, flags);
}
list_del_init(&alist);
@@ -597,9 +591,6 @@ int asd_execute_task(struct sas_task *task, gfp_t gfp_flags)
if (a == b)
break;
t = a->uldd_task;
- spin_lock_irqsave(&t->task_state_lock, flags);
- t->task_state_flags &= ~SAS_TASK_AT_INITIATOR;
- spin_unlock_irqrestore(&t->task_state_lock, flags);
switch (t->task_proto) {
case SAS_PROTOCOL_SATA:
case SAS_PROTOCOL_STP:
diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c b/drivers/scsi/hisi_sas/hisi_sas_main.c
index ebf5ec38891b..1873707ca599 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_main.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
@@ -405,7 +405,6 @@ void hisi_sas_task_deliver(struct hisi_hba *hisi_hba,
struct hisi_sas_cmd_hdr *cmd_hdr_base;
int dlvry_queue_slot, dlvry_queue;
struct sas_task *task = slot->task;
- unsigned long flags;
int wr_q_index;
spin_lock(&dq->lock);
@@ -457,10 +456,6 @@ void hisi_sas_task_deliver(struct hisi_hba *hisi_hba,
break;
}
- spin_lock_irqsave(&task->task_state_lock, flags);
- task->task_state_flags |= SAS_TASK_AT_INITIATOR;
- spin_unlock_irqrestore(&task->task_state_lock, flags);
-
WRITE_ONCE(slot->ready, 1);
spin_lock(&dq->lock);
@@ -1035,8 +1030,7 @@ static void hisi_sas_do_release_task(struct hisi_hba *hisi_hba, struct sas_task
ts->resp = SAS_TASK_COMPLETE;
ts->stat = SAS_ABORTED_TASK;
spin_lock_irqsave(&task->task_state_lock, flags);
- task->task_state_flags &=
- ~(SAS_TASK_STATE_PENDING | SAS_TASK_AT_INITIATOR);
+ task->task_state_flags &= ~SAS_TASK_STATE_PENDING;
if (!slot->is_internal && task->task_proto != SAS_PROTOCOL_SMP)
task->task_state_flags |= SAS_TASK_STATE_DONE;
spin_unlock_irqrestore(&task->task_state_lock, flags);
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
index 3059d19e4368..6914e992a02e 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
@@ -1200,8 +1200,7 @@ static void slot_complete_v1_hw(struct hisi_hba *hisi_hba,
sas_dev = device->lldd_dev;
spin_lock_irqsave(&task->task_state_lock, flags);
- task->task_state_flags &=
- ~(SAS_TASK_STATE_PENDING | SAS_TASK_AT_INITIATOR);
+ task->task_state_flags &= ~SAS_TASK_STATE_PENDING;
task->task_state_flags |= SAS_TASK_STATE_DONE;
spin_unlock_irqrestore(&task->task_state_lock, flags);
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
index 64ed3e472e65..eaaf9e8b4ca4 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
@@ -2344,8 +2344,7 @@ static void slot_complete_v2_hw(struct hisi_hba *hisi_hba,
sas_dev = device->lldd_dev;
spin_lock_irqsave(&task->task_state_lock, flags);
- task->task_state_flags &=
- ~(SAS_TASK_STATE_PENDING | SAS_TASK_AT_INITIATOR);
+ task->task_state_flags &= ~SAS_TASK_STATE_PENDING;
spin_unlock_irqrestore(&task->task_state_lock, flags);
memset(ts, 0, sizeof(*ts));
diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
index a01a3a7b706b..52e306c1ece2 100644
--- a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
+++ b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
@@ -2217,8 +2217,7 @@ static void slot_complete_v3_hw(struct hisi_hba *hisi_hba,
sas_dev = device->lldd_dev;
spin_lock_irqsave(&task->task_state_lock, flags);
- task->task_state_flags &=
- ~(SAS_TASK_STATE_PENDING | SAS_TASK_AT_INITIATOR);
+ task->task_state_flags &= ~SAS_TASK_STATE_PENDING ;
spin_unlock_irqrestore(&task->task_state_lock, flags);
memset(ts, 0, sizeof(*ts));
diff --git a/drivers/scsi/isci/request.c b/drivers/scsi/isci/request.c
index b48ec64f745a..b45e4e81dd19 100644
--- a/drivers/scsi/isci/request.c
+++ b/drivers/scsi/isci/request.c
@@ -2934,8 +2934,7 @@ static void isci_request_io_request_complete(struct isci_host *ihost,
if (test_bit(IREQ_COMPLETE_IN_TARGET, &request->flags)) {
/* Normal notification (task_done) */
task->task_state_flags |= SAS_TASK_STATE_DONE;
- task->task_state_flags &= ~(SAS_TASK_AT_INITIATOR |
- SAS_TASK_STATE_PENDING);
+ task->task_state_flags &= ~SAS_TASK_STATE_PENDING;
}
spin_unlock_irqrestore(&task->task_state_lock, task_flags);
diff --git a/drivers/scsi/isci/task.c b/drivers/scsi/isci/task.c
index 14738702d4c9..c82d07978532 100644
--- a/drivers/scsi/isci/task.c
+++ b/drivers/scsi/isci/task.c
@@ -91,8 +91,7 @@ static void isci_task_refuse(struct isci_host *ihost, struct sas_task *task,
/* Normal notification (task_done) */
task->task_state_flags |= SAS_TASK_STATE_DONE;
- task->task_state_flags &= ~(SAS_TASK_AT_INITIATOR |
- SAS_TASK_STATE_PENDING);
+ task->task_state_flags &= ~SAS_TASK_STATE_PENDING;
task->lldd_task = NULL;
spin_unlock_irqrestore(&task->task_state_lock, flags);
@@ -164,7 +163,6 @@ int isci_task_execute_task(struct sas_task *task, gfp_t gfp_flags)
} else {
struct isci_request *ireq;
- task->task_state_flags |= SAS_TASK_AT_INITIATOR;
/* do common allocation and init of request object. */
ireq = isci_io_request_from_tag(ihost, task, tag);
spin_unlock_irqrestore(&task->task_state_lock, flags);
@@ -531,8 +529,7 @@ int isci_task_abort_task(struct sas_task *task)
*/
spin_lock_irqsave(&task->task_state_lock, flags);
task->task_state_flags |= SAS_TASK_STATE_DONE;
- task->task_state_flags &= ~(SAS_TASK_AT_INITIATOR |
- SAS_TASK_STATE_PENDING);
+ task->task_state_flags &= ~SAS_TASK_STATE_PENDING;
spin_unlock_irqrestore(&task->task_state_lock, flags);
ret = TMF_RESP_FUNC_COMPLETE;
@@ -580,8 +577,7 @@ int isci_task_abort_task(struct sas_task *task)
test_bit(IDEV_GONE, &idev->flags));
spin_lock_irqsave(&task->task_state_lock, flags);
- task->task_state_flags &= ~(SAS_TASK_AT_INITIATOR |
- SAS_TASK_STATE_PENDING);
+ task->task_state_flags &= ~SAS_TASK_STATE_PENDING;
task->task_state_flags |= SAS_TASK_STATE_DONE;
spin_unlock_irqrestore(&task->task_state_lock, flags);
diff --git a/drivers/scsi/mvsas/mv_sas.c b/drivers/scsi/mvsas/mv_sas.c
index 1e52bc7febfa..a8d1f3dd607a 100644
--- a/drivers/scsi/mvsas/mv_sas.c
+++ b/drivers/scsi/mvsas/mv_sas.c
@@ -815,9 +815,6 @@ static int mvs_task_prep(struct sas_task *task, struct mvs_info *mvi, int is_tmf
slot->port = tei.port;
task->lldd_task = slot;
list_add_tail(&slot->entry, &tei.port->list);
- spin_lock(&task->task_state_lock);
- task->task_state_flags |= SAS_TASK_AT_INITIATOR;
- spin_unlock(&task->task_state_lock);
mvi_dev->running_req++;
++(*pass);
@@ -1721,8 +1718,7 @@ int mvs_slot_complete(struct mvs_info *mvi, u32 rx_desc, u32 flags)
mvi_dev = dev->lldd_dev;
spin_lock(&task->task_state_lock);
- task->task_state_flags &=
- ~(SAS_TASK_STATE_PENDING | SAS_TASK_AT_INITIATOR);
+ task->task_state_flags &= ~SAS_TASK_STATE_PENDING;
task->task_state_flags |= SAS_TASK_STATE_DONE;
/* race condition*/
aborted = task->task_state_flags & SAS_TASK_STATE_ABORTED;
diff --git a/drivers/scsi/pm8001/pm8001_hwi.c b/drivers/scsi/pm8001/pm8001_hwi.c
index 9ec310b795c3..4683fee87b84 100644
--- a/drivers/scsi/pm8001/pm8001_hwi.c
+++ b/drivers/scsi/pm8001/pm8001_hwi.c
@@ -1561,7 +1561,6 @@ void pm8001_work_fn(struct work_struct *work)
atomic_dec(&pm8001_dev->running_req);
spin_lock_irqsave(&t->task_state_lock, flags1);
t->task_state_flags &= ~SAS_TASK_STATE_PENDING;
- t->task_state_flags &= ~SAS_TASK_AT_INITIATOR;
t->task_state_flags |= SAS_TASK_STATE_DONE;
if (unlikely((t->task_state_flags & SAS_TASK_STATE_ABORTED))) {
spin_unlock_irqrestore(&t->task_state_lock, flags1);
@@ -2105,7 +2104,6 @@ mpi_ssp_completion(struct pm8001_hba_info *pm8001_ha, void *piomb)
psspPayload->ssp_resp_iu.status);
spin_lock_irqsave(&t->task_state_lock, flags);
t->task_state_flags &= ~SAS_TASK_STATE_PENDING;
- t->task_state_flags &= ~SAS_TASK_AT_INITIATOR;
t->task_state_flags |= SAS_TASK_STATE_DONE;
if (unlikely((t->task_state_flags & SAS_TASK_STATE_ABORTED))) {
spin_unlock_irqrestore(&t->task_state_lock, flags);
@@ -2273,7 +2271,6 @@ static void mpi_ssp_event(struct pm8001_hba_info *pm8001_ha, void *piomb)
}
spin_lock_irqsave(&t->task_state_lock, flags);
t->task_state_flags &= ~SAS_TASK_STATE_PENDING;
- t->task_state_flags &= ~SAS_TASK_AT_INITIATOR;
t->task_state_flags |= SAS_TASK_STATE_DONE;
if (unlikely((t->task_state_flags & SAS_TASK_STATE_ABORTED))) {
spin_unlock_irqrestore(&t->task_state_lock, flags);
@@ -2665,7 +2662,6 @@ mpi_sata_completion(struct pm8001_hba_info *pm8001_ha, void *piomb)
}
spin_lock_irqsave(&t->task_state_lock, flags);
t->task_state_flags &= ~SAS_TASK_STATE_PENDING;
- t->task_state_flags &= ~SAS_TASK_AT_INITIATOR;
t->task_state_flags |= SAS_TASK_STATE_DONE;
if (unlikely((t->task_state_flags & SAS_TASK_STATE_ABORTED))) {
spin_unlock_irqrestore(&t->task_state_lock, flags);
@@ -3022,7 +3018,6 @@ mpi_smp_completion(struct pm8001_hba_info *pm8001_ha, void *piomb)
}
spin_lock_irqsave(&t->task_state_lock, flags);
t->task_state_flags &= ~SAS_TASK_STATE_PENDING;
- t->task_state_flags &= ~SAS_TASK_AT_INITIATOR;
t->task_state_flags |= SAS_TASK_STATE_DONE;
if (unlikely((t->task_state_flags & SAS_TASK_STATE_ABORTED))) {
spin_unlock_irqrestore(&t->task_state_lock, flags);
@@ -3696,7 +3691,6 @@ int pm8001_mpi_task_abort_resp(struct pm8001_hba_info *pm8001_ha, void *piomb)
}
spin_lock_irqsave(&t->task_state_lock, flags);
t->task_state_flags &= ~SAS_TASK_STATE_PENDING;
- t->task_state_flags &= ~SAS_TASK_AT_INITIATOR;
t->task_state_flags |= SAS_TASK_STATE_DONE;
spin_unlock_irqrestore(&t->task_state_lock, flags);
pm8001_ccb_task_free(pm8001_ha, t, ccb, tag);
@@ -4336,7 +4330,6 @@ static int pm8001_chip_sata_req(struct pm8001_hba_info *pm8001_ha,
ts->resp = SAS_TASK_COMPLETE;
ts->stat = SAS_SAM_STAT_GOOD;
task->task_state_flags &= ~SAS_TASK_STATE_PENDING;
- task->task_state_flags &= ~SAS_TASK_AT_INITIATOR;
task->task_state_flags |= SAS_TASK_STATE_DONE;
if (unlikely((task->task_state_flags &
SAS_TASK_STATE_ABORTED))) {
diff --git a/drivers/scsi/pm8001/pm8001_sas.c b/drivers/scsi/pm8001/pm8001_sas.c
index 32edda3e55c6..8c12fbb9c476 100644
--- a/drivers/scsi/pm8001/pm8001_sas.c
+++ b/drivers/scsi/pm8001/pm8001_sas.c
@@ -487,9 +487,6 @@ static int pm8001_task_exec(struct sas_task *task,
goto err_out_tag;
}
/* 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;
@@ -983,7 +980,6 @@ void pm8001_open_reject_retry(
atomic_dec(&pm8001_dev->running_req);
spin_lock_irqsave(&task->task_state_lock, flags1);
task->task_state_flags &= ~SAS_TASK_STATE_PENDING;
- task->task_state_flags &= ~SAS_TASK_AT_INITIATOR;
task->task_state_flags |= SAS_TASK_STATE_DONE;
if (unlikely((task->task_state_flags
& SAS_TASK_STATE_ABORTED))) {
diff --git a/drivers/scsi/pm8001/pm80xx_hwi.c b/drivers/scsi/pm8001/pm80xx_hwi.c
index 9d20f8009b89..b83500ef3d86 100644
--- a/drivers/scsi/pm8001/pm80xx_hwi.c
+++ b/drivers/scsi/pm8001/pm80xx_hwi.c
@@ -2178,7 +2178,6 @@ mpi_ssp_completion(struct pm8001_hba_info *pm8001_ha, void *piomb)
psspPayload->ssp_resp_iu.status);
spin_lock_irqsave(&t->task_state_lock, flags);
t->task_state_flags &= ~SAS_TASK_STATE_PENDING;
- t->task_state_flags &= ~SAS_TASK_AT_INITIATOR;
t->task_state_flags |= SAS_TASK_STATE_DONE;
if (unlikely((t->task_state_flags & SAS_TASK_STATE_ABORTED))) {
spin_unlock_irqrestore(&t->task_state_lock, flags);
@@ -2362,7 +2361,6 @@ static void mpi_ssp_event(struct pm8001_hba_info *pm8001_ha, void *piomb)
}
spin_lock_irqsave(&t->task_state_lock, flags);
t->task_state_flags &= ~SAS_TASK_STATE_PENDING;
- t->task_state_flags &= ~SAS_TASK_AT_INITIATOR;
t->task_state_flags |= SAS_TASK_STATE_DONE;
if (unlikely((t->task_state_flags & SAS_TASK_STATE_ABORTED))) {
spin_unlock_irqrestore(&t->task_state_lock, flags);
@@ -2787,7 +2785,6 @@ mpi_sata_completion(struct pm8001_hba_info *pm8001_ha,
}
spin_lock_irqsave(&t->task_state_lock, flags);
t->task_state_flags &= ~SAS_TASK_STATE_PENDING;
- t->task_state_flags &= ~SAS_TASK_AT_INITIATOR;
t->task_state_flags |= SAS_TASK_STATE_DONE;
if (unlikely((t->task_state_flags & SAS_TASK_STATE_ABORTED))) {
spin_unlock_irqrestore(&t->task_state_lock, flags);
@@ -3199,7 +3196,6 @@ mpi_smp_completion(struct pm8001_hba_info *pm8001_ha, void *piomb)
}
spin_lock_irqsave(&t->task_state_lock, flags);
t->task_state_flags &= ~SAS_TASK_STATE_PENDING;
- t->task_state_flags &= ~SAS_TASK_AT_INITIATOR;
t->task_state_flags |= SAS_TASK_STATE_DONE;
if (unlikely((t->task_state_flags & SAS_TASK_STATE_ABORTED))) {
spin_unlock_irqrestore(&t->task_state_lock, flags);
@@ -4722,7 +4718,6 @@ static int pm80xx_chip_sata_req(struct pm8001_hba_info *pm8001_ha,
ts->resp = SAS_TASK_COMPLETE;
ts->stat = SAS_SAM_STAT_GOOD;
task->task_state_flags &= ~SAS_TASK_STATE_PENDING;
- task->task_state_flags &= ~SAS_TASK_AT_INITIATOR;
task->task_state_flags |= SAS_TASK_STATE_DONE;
if (unlikely((task->task_state_flags &
SAS_TASK_STATE_ABORTED))) {
diff --git a/include/scsi/libsas.h b/include/scsi/libsas.h
index 698f2032807b..549232d66b40 100644
--- a/include/scsi/libsas.h
+++ b/include/scsi/libsas.h
@@ -617,7 +617,6 @@ struct sas_task_slow {
#define SAS_TASK_STATE_DONE 2
#define SAS_TASK_STATE_ABORTED 4
#define SAS_TASK_NEED_DEV_RESET 8
-#define SAS_TASK_AT_INITIATOR 16
extern struct sas_task *sas_alloc_task(gfp_t flags);
extern struct sas_task *sas_alloc_slow_task(gfp_t flags);
--
2.26.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] scsi: libsas: Drop SAS_TASK_AT_INITIATOR
2022-02-10 10:43 ` [PATCH 2/2] scsi: libsas: Drop SAS_TASK_AT_INITIATOR John Garry
@ 2022-02-10 11:46 ` Damien Le Moal
0 siblings, 0 replies; 6+ messages in thread
From: Damien Le Moal @ 2022-02-10 11:46 UTC (permalink / raw)
To: John Garry, jejb, martin.petersen, artur.paszkiewicz, jinpu.wang
Cc: chenxiang66, linux-scsi, linux-kernel, linuxarm, hch
On 2/10/22 19:43, John Garry wrote:
> This flag is now only ever set, so delete it.
>
> This also avoids a use-after-free in the pm8001 queue path, as reported
> in the following:
> https://lore.kernel.org/linux-scsi/c3cb7228-254e-9584-182b-007ac5e6fe0a@huawei.com/T/#m28c94c6d3ff582ec4a9fa54819180740e8bd4cfb
> https://lore.kernel.org/linux-scsi/0cc0c435-b4f2-9c76-258d-865ba50a29dd@huawei.com/
>
> Signed-off-by: John Garry <john.garry@huawei.com>
No mor use-after-free, finally :)
Reviewed-by: Damien Le Moal <damien.lemoal@wdc.com>
> ---
> drivers/scsi/aic94xx/aic94xx_task.c | 9 ---------
> drivers/scsi/hisi_sas/hisi_sas_main.c | 8 +-------
> drivers/scsi/hisi_sas/hisi_sas_v1_hw.c | 3 +--
> drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 3 +--
> drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 3 +--
> drivers/scsi/isci/request.c | 3 +--
> drivers/scsi/isci/task.c | 10 +++-------
> drivers/scsi/mvsas/mv_sas.c | 6 +-----
> drivers/scsi/pm8001/pm8001_hwi.c | 7 -------
> drivers/scsi/pm8001/pm8001_sas.c | 4 ----
> drivers/scsi/pm8001/pm80xx_hwi.c | 5 -----
> include/scsi/libsas.h | 1 -
> 12 files changed, 9 insertions(+), 53 deletions(-)
>
> diff --git a/drivers/scsi/aic94xx/aic94xx_task.c b/drivers/scsi/aic94xx/aic94xx_task.c
> index c6b63eae28f5..ed119a3f6f2e 100644
> --- a/drivers/scsi/aic94xx/aic94xx_task.c
> +++ b/drivers/scsi/aic94xx/aic94xx_task.c
> @@ -322,7 +322,6 @@ static void asd_task_tasklet_complete(struct asd_ascb *ascb,
>
> spin_lock_irqsave(&task->task_state_lock, flags);
> task->task_state_flags &= ~SAS_TASK_STATE_PENDING;
> - task->task_state_flags &= ~SAS_TASK_AT_INITIATOR;
> task->task_state_flags |= SAS_TASK_STATE_DONE;
> if (unlikely((task->task_state_flags & SAS_TASK_STATE_ABORTED))) {
> struct completion *completion = ascb->completion;
> @@ -532,7 +531,6 @@ int asd_execute_task(struct sas_task *task, gfp_t gfp_flags)
> struct sas_task *t = task;
> struct asd_ascb *ascb = NULL, *a;
> struct asd_ha_struct *asd_ha = task->dev->port->ha->lldd_ha;
> - unsigned long flags;
>
> res = asd_can_queue(asd_ha, 1);
> if (res)
> @@ -575,10 +573,6 @@ int asd_execute_task(struct sas_task *task, gfp_t gfp_flags)
> }
> if (res)
> goto out_err_unmap;
> -
> - spin_lock_irqsave(&t->task_state_lock, flags);
> - t->task_state_flags |= SAS_TASK_AT_INITIATOR;
> - spin_unlock_irqrestore(&t->task_state_lock, flags);
> }
> list_del_init(&alist);
>
> @@ -597,9 +591,6 @@ int asd_execute_task(struct sas_task *task, gfp_t gfp_flags)
> if (a == b)
> break;
> t = a->uldd_task;
> - spin_lock_irqsave(&t->task_state_lock, flags);
> - t->task_state_flags &= ~SAS_TASK_AT_INITIATOR;
> - spin_unlock_irqrestore(&t->task_state_lock, flags);
> switch (t->task_proto) {
> case SAS_PROTOCOL_SATA:
> case SAS_PROTOCOL_STP:
> diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c b/drivers/scsi/hisi_sas/hisi_sas_main.c
> index ebf5ec38891b..1873707ca599 100644
> --- a/drivers/scsi/hisi_sas/hisi_sas_main.c
> +++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
> @@ -405,7 +405,6 @@ void hisi_sas_task_deliver(struct hisi_hba *hisi_hba,
> struct hisi_sas_cmd_hdr *cmd_hdr_base;
> int dlvry_queue_slot, dlvry_queue;
> struct sas_task *task = slot->task;
> - unsigned long flags;
> int wr_q_index;
>
> spin_lock(&dq->lock);
> @@ -457,10 +456,6 @@ void hisi_sas_task_deliver(struct hisi_hba *hisi_hba,
> break;
> }
>
> - spin_lock_irqsave(&task->task_state_lock, flags);
> - task->task_state_flags |= SAS_TASK_AT_INITIATOR;
> - spin_unlock_irqrestore(&task->task_state_lock, flags);
> -
> WRITE_ONCE(slot->ready, 1);
>
> spin_lock(&dq->lock);
> @@ -1035,8 +1030,7 @@ static void hisi_sas_do_release_task(struct hisi_hba *hisi_hba, struct sas_task
> ts->resp = SAS_TASK_COMPLETE;
> ts->stat = SAS_ABORTED_TASK;
> spin_lock_irqsave(&task->task_state_lock, flags);
> - task->task_state_flags &=
> - ~(SAS_TASK_STATE_PENDING | SAS_TASK_AT_INITIATOR);
> + task->task_state_flags &= ~SAS_TASK_STATE_PENDING;
> if (!slot->is_internal && task->task_proto != SAS_PROTOCOL_SMP)
> task->task_state_flags |= SAS_TASK_STATE_DONE;
> spin_unlock_irqrestore(&task->task_state_lock, flags);
> diff --git a/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
> index 3059d19e4368..6914e992a02e 100644
> --- a/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
> +++ b/drivers/scsi/hisi_sas/hisi_sas_v1_hw.c
> @@ -1200,8 +1200,7 @@ static void slot_complete_v1_hw(struct hisi_hba *hisi_hba,
> sas_dev = device->lldd_dev;
>
> spin_lock_irqsave(&task->task_state_lock, flags);
> - task->task_state_flags &=
> - ~(SAS_TASK_STATE_PENDING | SAS_TASK_AT_INITIATOR);
> + task->task_state_flags &= ~SAS_TASK_STATE_PENDING;
> task->task_state_flags |= SAS_TASK_STATE_DONE;
> spin_unlock_irqrestore(&task->task_state_lock, flags);
>
> diff --git a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
> index 64ed3e472e65..eaaf9e8b4ca4 100644
> --- a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
> +++ b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
> @@ -2344,8 +2344,7 @@ static void slot_complete_v2_hw(struct hisi_hba *hisi_hba,
> sas_dev = device->lldd_dev;
>
> spin_lock_irqsave(&task->task_state_lock, flags);
> - task->task_state_flags &=
> - ~(SAS_TASK_STATE_PENDING | SAS_TASK_AT_INITIATOR);
> + task->task_state_flags &= ~SAS_TASK_STATE_PENDING;
> spin_unlock_irqrestore(&task->task_state_lock, flags);
>
> memset(ts, 0, sizeof(*ts));
> diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
> index a01a3a7b706b..52e306c1ece2 100644
> --- a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
> +++ b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
> @@ -2217,8 +2217,7 @@ static void slot_complete_v3_hw(struct hisi_hba *hisi_hba,
> sas_dev = device->lldd_dev;
>
> spin_lock_irqsave(&task->task_state_lock, flags);
> - task->task_state_flags &=
> - ~(SAS_TASK_STATE_PENDING | SAS_TASK_AT_INITIATOR);
> + task->task_state_flags &= ~SAS_TASK_STATE_PENDING ;
> spin_unlock_irqrestore(&task->task_state_lock, flags);
>
> memset(ts, 0, sizeof(*ts));
> diff --git a/drivers/scsi/isci/request.c b/drivers/scsi/isci/request.c
> index b48ec64f745a..b45e4e81dd19 100644
> --- a/drivers/scsi/isci/request.c
> +++ b/drivers/scsi/isci/request.c
> @@ -2934,8 +2934,7 @@ static void isci_request_io_request_complete(struct isci_host *ihost,
> if (test_bit(IREQ_COMPLETE_IN_TARGET, &request->flags)) {
> /* Normal notification (task_done) */
> task->task_state_flags |= SAS_TASK_STATE_DONE;
> - task->task_state_flags &= ~(SAS_TASK_AT_INITIATOR |
> - SAS_TASK_STATE_PENDING);
> + task->task_state_flags &= ~SAS_TASK_STATE_PENDING;
> }
> spin_unlock_irqrestore(&task->task_state_lock, task_flags);
>
> diff --git a/drivers/scsi/isci/task.c b/drivers/scsi/isci/task.c
> index 14738702d4c9..c82d07978532 100644
> --- a/drivers/scsi/isci/task.c
> +++ b/drivers/scsi/isci/task.c
> @@ -91,8 +91,7 @@ static void isci_task_refuse(struct isci_host *ihost, struct sas_task *task,
>
> /* Normal notification (task_done) */
> task->task_state_flags |= SAS_TASK_STATE_DONE;
> - task->task_state_flags &= ~(SAS_TASK_AT_INITIATOR |
> - SAS_TASK_STATE_PENDING);
> + task->task_state_flags &= ~SAS_TASK_STATE_PENDING;
> task->lldd_task = NULL;
> spin_unlock_irqrestore(&task->task_state_lock, flags);
>
> @@ -164,7 +163,6 @@ int isci_task_execute_task(struct sas_task *task, gfp_t gfp_flags)
> } else {
> struct isci_request *ireq;
>
> - task->task_state_flags |= SAS_TASK_AT_INITIATOR;
> /* do common allocation and init of request object. */
> ireq = isci_io_request_from_tag(ihost, task, tag);
> spin_unlock_irqrestore(&task->task_state_lock, flags);
> @@ -531,8 +529,7 @@ int isci_task_abort_task(struct sas_task *task)
> */
> spin_lock_irqsave(&task->task_state_lock, flags);
> task->task_state_flags |= SAS_TASK_STATE_DONE;
> - task->task_state_flags &= ~(SAS_TASK_AT_INITIATOR |
> - SAS_TASK_STATE_PENDING);
> + task->task_state_flags &= ~SAS_TASK_STATE_PENDING;
> spin_unlock_irqrestore(&task->task_state_lock, flags);
>
> ret = TMF_RESP_FUNC_COMPLETE;
> @@ -580,8 +577,7 @@ int isci_task_abort_task(struct sas_task *task)
> test_bit(IDEV_GONE, &idev->flags));
>
> spin_lock_irqsave(&task->task_state_lock, flags);
> - task->task_state_flags &= ~(SAS_TASK_AT_INITIATOR |
> - SAS_TASK_STATE_PENDING);
> + task->task_state_flags &= ~SAS_TASK_STATE_PENDING;
> task->task_state_flags |= SAS_TASK_STATE_DONE;
> spin_unlock_irqrestore(&task->task_state_lock, flags);
>
> diff --git a/drivers/scsi/mvsas/mv_sas.c b/drivers/scsi/mvsas/mv_sas.c
> index 1e52bc7febfa..a8d1f3dd607a 100644
> --- a/drivers/scsi/mvsas/mv_sas.c
> +++ b/drivers/scsi/mvsas/mv_sas.c
> @@ -815,9 +815,6 @@ static int mvs_task_prep(struct sas_task *task, struct mvs_info *mvi, int is_tmf
> slot->port = tei.port;
> task->lldd_task = slot;
> list_add_tail(&slot->entry, &tei.port->list);
> - spin_lock(&task->task_state_lock);
> - task->task_state_flags |= SAS_TASK_AT_INITIATOR;
> - spin_unlock(&task->task_state_lock);
>
> mvi_dev->running_req++;
> ++(*pass);
> @@ -1721,8 +1718,7 @@ int mvs_slot_complete(struct mvs_info *mvi, u32 rx_desc, u32 flags)
> mvi_dev = dev->lldd_dev;
>
> spin_lock(&task->task_state_lock);
> - task->task_state_flags &=
> - ~(SAS_TASK_STATE_PENDING | SAS_TASK_AT_INITIATOR);
> + task->task_state_flags &= ~SAS_TASK_STATE_PENDING;
> task->task_state_flags |= SAS_TASK_STATE_DONE;
> /* race condition*/
> aborted = task->task_state_flags & SAS_TASK_STATE_ABORTED;
> diff --git a/drivers/scsi/pm8001/pm8001_hwi.c b/drivers/scsi/pm8001/pm8001_hwi.c
> index 9ec310b795c3..4683fee87b84 100644
> --- a/drivers/scsi/pm8001/pm8001_hwi.c
> +++ b/drivers/scsi/pm8001/pm8001_hwi.c
> @@ -1561,7 +1561,6 @@ void pm8001_work_fn(struct work_struct *work)
> atomic_dec(&pm8001_dev->running_req);
> spin_lock_irqsave(&t->task_state_lock, flags1);
> t->task_state_flags &= ~SAS_TASK_STATE_PENDING;
> - t->task_state_flags &= ~SAS_TASK_AT_INITIATOR;
> t->task_state_flags |= SAS_TASK_STATE_DONE;
> if (unlikely((t->task_state_flags & SAS_TASK_STATE_ABORTED))) {
> spin_unlock_irqrestore(&t->task_state_lock, flags1);
> @@ -2105,7 +2104,6 @@ mpi_ssp_completion(struct pm8001_hba_info *pm8001_ha, void *piomb)
> psspPayload->ssp_resp_iu.status);
> spin_lock_irqsave(&t->task_state_lock, flags);
> t->task_state_flags &= ~SAS_TASK_STATE_PENDING;
> - t->task_state_flags &= ~SAS_TASK_AT_INITIATOR;
> t->task_state_flags |= SAS_TASK_STATE_DONE;
> if (unlikely((t->task_state_flags & SAS_TASK_STATE_ABORTED))) {
> spin_unlock_irqrestore(&t->task_state_lock, flags);
> @@ -2273,7 +2271,6 @@ static void mpi_ssp_event(struct pm8001_hba_info *pm8001_ha, void *piomb)
> }
> spin_lock_irqsave(&t->task_state_lock, flags);
> t->task_state_flags &= ~SAS_TASK_STATE_PENDING;
> - t->task_state_flags &= ~SAS_TASK_AT_INITIATOR;
> t->task_state_flags |= SAS_TASK_STATE_DONE;
> if (unlikely((t->task_state_flags & SAS_TASK_STATE_ABORTED))) {
> spin_unlock_irqrestore(&t->task_state_lock, flags);
> @@ -2665,7 +2662,6 @@ mpi_sata_completion(struct pm8001_hba_info *pm8001_ha, void *piomb)
> }
> spin_lock_irqsave(&t->task_state_lock, flags);
> t->task_state_flags &= ~SAS_TASK_STATE_PENDING;
> - t->task_state_flags &= ~SAS_TASK_AT_INITIATOR;
> t->task_state_flags |= SAS_TASK_STATE_DONE;
> if (unlikely((t->task_state_flags & SAS_TASK_STATE_ABORTED))) {
> spin_unlock_irqrestore(&t->task_state_lock, flags);
> @@ -3022,7 +3018,6 @@ mpi_smp_completion(struct pm8001_hba_info *pm8001_ha, void *piomb)
> }
> spin_lock_irqsave(&t->task_state_lock, flags);
> t->task_state_flags &= ~SAS_TASK_STATE_PENDING;
> - t->task_state_flags &= ~SAS_TASK_AT_INITIATOR;
> t->task_state_flags |= SAS_TASK_STATE_DONE;
> if (unlikely((t->task_state_flags & SAS_TASK_STATE_ABORTED))) {
> spin_unlock_irqrestore(&t->task_state_lock, flags);
> @@ -3696,7 +3691,6 @@ int pm8001_mpi_task_abort_resp(struct pm8001_hba_info *pm8001_ha, void *piomb)
> }
> spin_lock_irqsave(&t->task_state_lock, flags);
> t->task_state_flags &= ~SAS_TASK_STATE_PENDING;
> - t->task_state_flags &= ~SAS_TASK_AT_INITIATOR;
> t->task_state_flags |= SAS_TASK_STATE_DONE;
> spin_unlock_irqrestore(&t->task_state_lock, flags);
> pm8001_ccb_task_free(pm8001_ha, t, ccb, tag);
> @@ -4336,7 +4330,6 @@ static int pm8001_chip_sata_req(struct pm8001_hba_info *pm8001_ha,
> ts->resp = SAS_TASK_COMPLETE;
> ts->stat = SAS_SAM_STAT_GOOD;
> task->task_state_flags &= ~SAS_TASK_STATE_PENDING;
> - task->task_state_flags &= ~SAS_TASK_AT_INITIATOR;
> task->task_state_flags |= SAS_TASK_STATE_DONE;
> if (unlikely((task->task_state_flags &
> SAS_TASK_STATE_ABORTED))) {
> diff --git a/drivers/scsi/pm8001/pm8001_sas.c b/drivers/scsi/pm8001/pm8001_sas.c
> index 32edda3e55c6..8c12fbb9c476 100644
> --- a/drivers/scsi/pm8001/pm8001_sas.c
> +++ b/drivers/scsi/pm8001/pm8001_sas.c
> @@ -487,9 +487,6 @@ static int pm8001_task_exec(struct sas_task *task,
> goto err_out_tag;
> }
> /* 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;
> @@ -983,7 +980,6 @@ void pm8001_open_reject_retry(
> atomic_dec(&pm8001_dev->running_req);
> spin_lock_irqsave(&task->task_state_lock, flags1);
> task->task_state_flags &= ~SAS_TASK_STATE_PENDING;
> - task->task_state_flags &= ~SAS_TASK_AT_INITIATOR;
> task->task_state_flags |= SAS_TASK_STATE_DONE;
> if (unlikely((task->task_state_flags
> & SAS_TASK_STATE_ABORTED))) {
> diff --git a/drivers/scsi/pm8001/pm80xx_hwi.c b/drivers/scsi/pm8001/pm80xx_hwi.c
> index 9d20f8009b89..b83500ef3d86 100644
> --- a/drivers/scsi/pm8001/pm80xx_hwi.c
> +++ b/drivers/scsi/pm8001/pm80xx_hwi.c
> @@ -2178,7 +2178,6 @@ mpi_ssp_completion(struct pm8001_hba_info *pm8001_ha, void *piomb)
> psspPayload->ssp_resp_iu.status);
> spin_lock_irqsave(&t->task_state_lock, flags);
> t->task_state_flags &= ~SAS_TASK_STATE_PENDING;
> - t->task_state_flags &= ~SAS_TASK_AT_INITIATOR;
> t->task_state_flags |= SAS_TASK_STATE_DONE;
> if (unlikely((t->task_state_flags & SAS_TASK_STATE_ABORTED))) {
> spin_unlock_irqrestore(&t->task_state_lock, flags);
> @@ -2362,7 +2361,6 @@ static void mpi_ssp_event(struct pm8001_hba_info *pm8001_ha, void *piomb)
> }
> spin_lock_irqsave(&t->task_state_lock, flags);
> t->task_state_flags &= ~SAS_TASK_STATE_PENDING;
> - t->task_state_flags &= ~SAS_TASK_AT_INITIATOR;
> t->task_state_flags |= SAS_TASK_STATE_DONE;
> if (unlikely((t->task_state_flags & SAS_TASK_STATE_ABORTED))) {
> spin_unlock_irqrestore(&t->task_state_lock, flags);
> @@ -2787,7 +2785,6 @@ mpi_sata_completion(struct pm8001_hba_info *pm8001_ha,
> }
> spin_lock_irqsave(&t->task_state_lock, flags);
> t->task_state_flags &= ~SAS_TASK_STATE_PENDING;
> - t->task_state_flags &= ~SAS_TASK_AT_INITIATOR;
> t->task_state_flags |= SAS_TASK_STATE_DONE;
> if (unlikely((t->task_state_flags & SAS_TASK_STATE_ABORTED))) {
> spin_unlock_irqrestore(&t->task_state_lock, flags);
> @@ -3199,7 +3196,6 @@ mpi_smp_completion(struct pm8001_hba_info *pm8001_ha, void *piomb)
> }
> spin_lock_irqsave(&t->task_state_lock, flags);
> t->task_state_flags &= ~SAS_TASK_STATE_PENDING;
> - t->task_state_flags &= ~SAS_TASK_AT_INITIATOR;
> t->task_state_flags |= SAS_TASK_STATE_DONE;
> if (unlikely((t->task_state_flags & SAS_TASK_STATE_ABORTED))) {
> spin_unlock_irqrestore(&t->task_state_lock, flags);
> @@ -4722,7 +4718,6 @@ static int pm80xx_chip_sata_req(struct pm8001_hba_info *pm8001_ha,
> ts->resp = SAS_TASK_COMPLETE;
> ts->stat = SAS_SAM_STAT_GOOD;
> task->task_state_flags &= ~SAS_TASK_STATE_PENDING;
> - task->task_state_flags &= ~SAS_TASK_AT_INITIATOR;
> task->task_state_flags |= SAS_TASK_STATE_DONE;
> if (unlikely((task->task_state_flags &
> SAS_TASK_STATE_ABORTED))) {
> diff --git a/include/scsi/libsas.h b/include/scsi/libsas.h
> index 698f2032807b..549232d66b40 100644
> --- a/include/scsi/libsas.h
> +++ b/include/scsi/libsas.h
> @@ -617,7 +617,6 @@ struct sas_task_slow {
> #define SAS_TASK_STATE_DONE 2
> #define SAS_TASK_STATE_ABORTED 4
> #define SAS_TASK_NEED_DEV_RESET 8
> -#define SAS_TASK_AT_INITIATOR 16
>
> extern struct sas_task *sas_alloc_task(gfp_t flags);
> extern struct sas_task *sas_alloc_slow_task(gfp_t flags);
--
Damien Le Moal
Western Digital Research
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 0/2] scsi: libsas: Drop SAS_TASK_AT_INITIATOR
2022-02-10 10:43 [PATCH 0/2] scsi: libsas: Drop SAS_TASK_AT_INITIATOR John Garry
2022-02-10 10:43 ` [PATCH 1/2] scsi: isci: Drop SAS_TASK_AT_INITIATOR check in isci_task_abort_task() John Garry
2022-02-10 10:43 ` [PATCH 2/2] scsi: libsas: Drop SAS_TASK_AT_INITIATOR John Garry
@ 2022-02-11 21:57 ` Martin K. Petersen
2022-02-15 3:19 ` Martin K. Petersen
3 siblings, 0 replies; 6+ messages in thread
From: Martin K. Petersen @ 2022-02-11 21:57 UTC (permalink / raw)
To: John Garry
Cc: jejb, martin.petersen, artur.paszkiewicz, jinpu.wang,
damien.lemoal, chenxiang66, linux-scsi, linux-kernel, linuxarm,
hch
John,
> Apart from some isci driver code, flag SAS_TASK_AT_INITIATOR is only
> set, so drop usage in that driver and then everywhere else.
>
> This solves a use-after-free in the pm8001 queue path.
Applied to 5.18/scsi-staging, thanks!
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 0/2] scsi: libsas: Drop SAS_TASK_AT_INITIATOR
2022-02-10 10:43 [PATCH 0/2] scsi: libsas: Drop SAS_TASK_AT_INITIATOR John Garry
` (2 preceding siblings ...)
2022-02-11 21:57 ` [PATCH 0/2] " Martin K. Petersen
@ 2022-02-15 3:19 ` Martin K. Petersen
3 siblings, 0 replies; 6+ messages in thread
From: Martin K. Petersen @ 2022-02-15 3:19 UTC (permalink / raw)
To: jejb, artur.paszkiewicz, damien.lemoal, John Garry, jinpu.wang
Cc: Martin K . Petersen, linux-scsi, linuxarm, hch, linux-kernel,
chenxiang66
On Thu, 10 Feb 2022 18:43:22 +0800, John Garry wrote:
> Apart from some isci driver code, flag SAS_TASK_AT_INITIATOR is only set,
> so drop usage in that driver and then everywhere else.
>
> This solves a use-after-free in the pm8001 queue path.
>
> John Garry (2):
> scsi: isci: Drop SAS_TASK_AT_INITIATOR check in isci_task_abort_task()
> scsi: libsas: Drop SAS_TASK_AT_INITIATOR
>
> [...]
Applied to 5.18/scsi-queue, thanks!
[1/2] scsi: isci: Drop SAS_TASK_AT_INITIATOR check in isci_task_abort_task()
https://git.kernel.org/mkp/scsi/c/c39d5aa457f2
[2/2] scsi: libsas: Drop SAS_TASK_AT_INITIATOR
https://git.kernel.org/mkp/scsi/c/26fc0ea74fcb
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-02-15 3:19 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-10 10:43 [PATCH 0/2] scsi: libsas: Drop SAS_TASK_AT_INITIATOR John Garry
2022-02-10 10:43 ` [PATCH 1/2] scsi: isci: Drop SAS_TASK_AT_INITIATOR check in isci_task_abort_task() John Garry
2022-02-10 10:43 ` [PATCH 2/2] scsi: libsas: Drop SAS_TASK_AT_INITIATOR John Garry
2022-02-10 11:46 ` Damien Le Moal
2022-02-11 21:57 ` [PATCH 0/2] " Martin K. Petersen
2022-02-15 3:19 ` Martin K. Petersen
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.