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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] scsi: aic94xx: UAF race condition due to lockless escb access
--

commit 61c788d589a8ac9597298b6297ce45bf72a70621
Author: Yehyeong Lee <yhlee@isslab.korea.ac.kr>

scsi: aic94xx: check that an ESCB lookup returned an ESCB

This commit adds checks to verify that an ESCB lookup actually returns an
Empty SCB by inspecting the opcode.

> diff --git a/drivers/scsi/aic94xx/aic94xx_task.c b/drivers/scsi/aic94xx/aic94xx_task.c
> --- a/drivers/scsi/aic94xx/aic94xx_task.c
> +++ b/drivers/scsi/aic94xx/aic94xx_task.c
> @@ -170,6 +170,14 @@ static void asd_get_response_tasklet(struct asd_ascb *ascb,
>  		return;
>  	}
>  
> +	if (escb->scb->header.opcode != EMPTY_SCB) {

[Severity: High]
Does checking the escb->scb->header.opcode outside of the tc_index_lock
introduce a race condition?

If the hardware returns a bogus index corresponding to a live task, could
that task concurrently time out and be freed while we are evaluating the
opcode? This appears to introduce a use-after-free read when dereferencing
escb->scb since the tc_index_lock was already dropped earlier in
asd_get_response_tasklet().

> +		ASD_DPRINTK("index_escb %d is not an ESCB, opcode is 0x%x!\n",
> +			    le16_to_cpu(resp_sb->index_escb),
> +			    escb->scb->header.opcode);
> +		ts->stat = SAS_INTERRUPTED;
> +		return;
> +	}
> +

[ ... ]

> diff --git a/drivers/scsi/aic94xx/aic94xx_tmf.c b/drivers/scsi/aic94xx/aic94xx_tmf.c
> --- a/drivers/scsi/aic94xx/aic94xx_tmf.c
> +++ b/drivers/scsi/aic94xx/aic94xx_tmf.c
> @@ -282,6 +282,13 @@ static int asd_get_tmf_resp_tasklet(struct asd_ascb *ascb,
>  		return res;
>  	}
>  
> +	if (escb->scb->header.opcode != EMPTY_SCB) {

[Severity: High]
Can this lockless access to escb->scb in asd_get_tmf_resp_tasklet() also
risk a use-after-free if the escb is concurrently freed?

> +		ASD_DPRINTK("index_escb %d is not an ESCB, opcode is 0x%x!\n",
> +			    le16_to_cpu(resp_sb->index_escb),
> +			    escb->scb->header.opcode);
> +		return res;
> +	}

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