[PATCH v3 5/5] scsi: aic94xx: check that an ESCB lookup returned an ESCB

[Yehyeong Lee]

asd_get_response_tasklet() and asd_get_tmf_resp_tasklet() look an Empty
SCB up by the index_escb field of the done list status block and then use
it as one:

	escb = asd_tc_index_find(&asd_ha->seq,
				 (int)le16_to_cpu(resp_sb->index_escb));
	...
	edb = asd_ha->seq.edb_arr[edb_id + escb->edb_index];
	...
	asd_invalidate_edb(escb, edb_id);

Nothing checks that what came back is an Empty SCB. There is one index
space for every aSCB the driver allocates -- asd_ascb_alloc() is the only
caller of asd_tc_index_get() -- so an in-range index can just as well
name a live task, a control phy SCB or an internal command.

That matters because struct scb is a union, and asd_invalidate_edb()
reads its escb member without looking at the opcode:

	struct empty_scb *escb = &ascb->scb->escb;
	struct sg_el     *eb   = &escb->eb[edb_id];
	...
	eb->flags |= ELEMENT_NOT_VALID;
	escb->num_valid--;

On an SCB that is not an Empty SCB those two stores land in whichever
union member it really is -- for an initiate_ssp_task that region holds
the SSP frame header and the scatter-gather elements of a command that is
still in flight.

Check the opcode, the way asd_dl_tasklet_handler() already does when it
decides whether a done list entry belongs to an Empty SCB:

	} else if (ascb->scb->header.opcode == EMPTY_SCB) {

Do it inside the critical section that already holds tc_index_lock for
the lookup, and clear escb on a mismatch so that the existing "no escb
for this dl" path handles it. Reading the opcode after the unlock would
be reading an aSCB the lookup no longer pins: asd_ascb_free() clears the
array slot under tc_index_lock and only then calls dma_pool_free() on
ascb->dma_scb.vaddr, which is ascb->scb, and kmem_cache_free() on the
aSCB itself. So a slot seen non-NULL while the lock is held belongs to an
aSCB whose free has not reached its own critical section yet, and the
opcode is safe to read; the same read after the unlock is not.
asd_tc_index_find() is documented as needing that lock anyway.

Empty SCBs themselves are not freed while the adapter is up --
asd_free_escbs() runs from the teardown path -- so this only concerns an
index that names something other than an ESCB, which is the case the
check is for.

Both callers already bail on a NULL lookup, so neither needs a new exit:
asd_get_response_tasklet() reports SAS_INTERRUPTED there as of the
earlier patch in this series, and asd_get_tmf_resp_tasklet() returns the
TMF_RESP_FUNC_FAILED that res already holds. The non-ESCB case shares
that diagnostic rather than adding one.

This is not a claim of a reachable type confusion. The index originates
with the driver and the sequencer echoes it back, and whether it can echo
a different one is not something the tree answers. The check is cheap and
the idiom is already in the driver.

Fixes: 2908d778ab3e ("[SCSI] aic94xx: new driver")
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Yehyeong Lee <yhlee@xxxxxxxxxxxxxxxxxx>
---
 drivers/scsi/aic94xx/aic94xx_task.c | 2 ++
 drivers/scsi/aic94xx/aic94xx_tmf.c  | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/drivers/scsi/aic94xx/aic94xx_task.c b/drivers/scsi/aic94xx/aic94xx_task.c
index bb5094ca5de15..00addeef1230a 100644
--- a/drivers/scsi/aic94xx/aic94xx_task.c
+++ b/drivers/scsi/aic94xx/aic94xx_task.c
@@ -162,6 +162,8 @@ static void asd_get_response_tasklet(struct asd_ascb *ascb,
 	spin_lock_irqsave(&asd_ha->seq.tc_index_lock, flags);
 	escb = asd_tc_index_find(&asd_ha->seq,
 				 (int)le16_to_cpu(resp_sb->index_escb));
+	if (escb && escb->scb->header.opcode != EMPTY_SCB)
+		escb = NULL;	  /* the index landed on something else */
 	spin_unlock_irqrestore(&asd_ha->seq.tc_index_lock, flags);
 
 	if (!escb) {
diff --git a/drivers/scsi/aic94xx/aic94xx_tmf.c b/drivers/scsi/aic94xx/aic94xx_tmf.c
index 1ee6009d83051..c05446d66b7bb 100644
--- a/drivers/scsi/aic94xx/aic94xx_tmf.c
+++ b/drivers/scsi/aic94xx/aic94xx_tmf.c
@@ -275,6 +275,8 @@ static int asd_get_tmf_resp_tasklet(struct asd_ascb *ascb,
 	spin_lock_irqsave(&asd_ha->seq.tc_index_lock, flags);
 	escb = asd_tc_index_find(&asd_ha->seq,
 				 (int)le16_to_cpu(resp_sb->index_escb));
+	if (escb && escb->scb->header.opcode != EMPTY_SCB)
+		escb = NULL;	  /* the index landed on something else */
 	spin_unlock_irqrestore(&asd_ha->seq.tc_index_lock, flags);
 
 	if (!escb) {
-- 
2.43.0