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

[James Bottomley <James.Bottomley@HansenPartnership.com>]

On Fri, 2026-10-16 at 09:14 +0900, Yehyeong Lee wrote:
> > 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.

There is a long tail of old server class hardware that has this chip as
its on-board controller, so we try not to touch the driver to avoid
causing bugs for them which will only show up months to years later
when they install stable backports.  The problem you outline here is
purely theoretical: if the sequencer put a zero in this field; however,
there's never been any actual observation of any sequencer doing that,
so there's no evidence a bug actually exists here.  Added to which the
fact that the change is untested on real hardware seems to create a set
of risks in applying it without much offset in the way of benefit.

Regards,

James