Re: [PATCH 3/3] scsi: aic94xx: validate the EDB index in asd_get_tmf_resp_tasklet()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] scsi: aic94xx: type confusion and OOB array access due to unchecked index_escb
--

commit 6e22ba829d4a35f4965fabb73864e8a26c77e6c0
Author: Yehyeong Lee <yhlee@isslab.korea.ac.kr>

scsi: aic94xx: validate the EDB index in asd_get_tmf_resp_tasklet()

This patch validates the empty data buffer index (edb_id) extracted from the
done list status block in asd_get_tmf_resp_tasklet(). It adds bounds checking
before indexing seq->edb_arr[] to prevent out-of-bounds access.

> diff --git a/drivers/scsi/aic94xx/aic94xx_tmf.c b/drivers/scsi/aic94xx/aic94xx_tmf.c
> index 28ac92b041fe3..1ee6009d83051 100644
> --- a/drivers/scsi/aic94xx/aic94xx_tmf.c
> +++ b/drivers/scsi/aic94xx/aic94xx_tmf.c
> @@ -282,6 +282,12 @@ static int asd_get_tmf_resp_tasklet(struct asd_ascb *ascb,
>  		return res;
>  	}
>  
> +	if (edb_id < 0 || edb_id >= ASD_EDBS_PER_SCB) {
> +		ASD_DPRINTK("edb_id is %d! resp_sb->flags is 0x%x\n",
> +			    edb_id, resp_sb->flags);

[Severity: High]
Does resp_sb->index_escb need validation as well?

Looking at asd_get_tmf_resp_tasklet(), the index_escb value is extracted and
passed directly to asd_tc_index_find():

drivers/scsi/aic94xx/aic94xx_tmf.c:asd_get_tmf_resp_tasklet() {
    ...
	escb = asd_tc_index_find(&asd_ha->seq,
				 (int)le16_to_cpu(resp_sb->index_escb));
    ...
}

Since index_escb comes from the untrusted firmware status block, could
an invalid index cause an out-of-bounds read in asd_tc_index_find() across
seq->tc_index_array?

Furthermore, could this lead to type confusion? The returned escb pointer
is used at the end of the function:

drivers/scsi/aic94xx/aic94xx_tmf.c:asd_get_tmf_resp_tasklet() {
    ...
	asd_invalidate_edb(escb, edb_id);
	return res;
}

If a normal SCB index was returned instead of an Empty SCB, could
asd_invalidate_edb() corrupt the state of normal pending tasks by assuming
the pointer points to an Empty SCB without verifying its opcode?

> +		return res;
> +	}
> +
>  	edb = asd_ha->seq.edb_arr[edb_id + escb->edb_index];
>  	ascb->tag = *(__be16 *)(edb->vaddr+4);
>  	fh = edb->vaddr + 16;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261015072612.4144043-1-yhlee@isslab.korea.ac.kr?part=3