Re: [PATCH v3 2/5] scsi: aic94xx: validate the EDB index in asd_get_response_tasklet()

[Yehyeong Lee]

> have you actually tested this on real hardware?

No -- the hardware is twenty years old and I couldn't get a board. I
exercised the path by fault injection: built the function into a KUnit
harness, fed it a status block with those three bits zero, and KASAN
reported the read one entry before seq->edb_arr[] without the guard and
nothing with it. That isn't evidence the sequencer does this, and I'm not
presenting it as such.

> is there any evidence of the sequencer ever setting these three bits
> to a zero value?

I can't show that it does. I looked for something authoritative -- an
AIC-9410 done-list/status-block layout -- and there's no public Adaptec
spec; the sequencer is an opaque request_firmware() blob, and the only
in-tree description of this field is the commented-out size line just
above the decode. What the tree does settle is the decode: the index is
one-based, and the same seq->edb_arr[edb_id + edb_index] access is also
reached from the empty-buffer path (escb_tasklet_complete ->
asd_bytes_dmaed_tasklet), where edb < 0 is rejected before the access --
aic94xx_scb.c:410, the 0xc0 "no buffer" case. The response path reaches
the identical access with no such check. So it really turns on whether
the sequencer can set those bits to zero on a response: if you know it
can't, this is dead code like the second comparison and I'll drop it; if
the field is as unbounded as it looks, this is the guard the driver
already applies to the same array on its other producer.

> Why the second comparison?  As you say the value could only be -1..6
> and ASD_EDBS_PER_SCB is defined to 7.

You're right, it's dead -- (resp_sb->flags & 0x70) >> 4 is 0..7, so
edb_id is -1..6 and -1 is the only out-of-range value. I'll cut it to

	if (edb_id < 0)

here and in the aic94xx_tmf.c copy. (scb.c carries both halves; I can
make them match whichever you prefer.)