Re: [PATCH v2 1/2] accel/amdxdna: check the command chain payload before using it

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] amdxdna_cmd_set_error() writes the error state to the wrong chain command BO by hardcoding index 0.
--

commit 5dc0906b3cc41016d562034c7287f924681a97d7
Author: Taimuraz Kaitmazov <taimuraz@kaitmazov.com>

accel/amdxdna: check the command chain payload before using it

This commit updates amdxdna_cmd_get_payload() to make the size parameter
mandatory and introduces amdxdna_cmd_get_chain() to ensure the command
count fits within the BO before returning the chain payload.

> diff --git a/drivers/accel/amdxdna/amdxdna_ctx.c b/drivers/accel/amdxdna/amdxdna_ctx.c
> index 888e857ec5582..caf7295a2b5b9 100644
> --- a/drivers/accel/amdxdna/amdxdna_ctx.c
> +++ b/drivers/accel/amdxdna/amdxdna_ctx.c

[ ... ]

> @@ -177,8 +199,13 @@ int amdxdna_cmd_set_error(struct amdxdna_gem_obj *abo,
>  	cmd->header |= FIELD_PREP(AMDXDNA_CMD_STATE, error_state);
>  
>  	if (amdxdna_cmd_get_op(abo) == ERT_CMD_CHAIN) {
> -		cc = amdxdna_cmd_get_payload(abo, NULL);
> -		cc->error_index = (cmd_idx < cc->command_count) ? cmd_idx : 0;
> +		u32 ccnt;
> +
> +		cc = amdxdna_cmd_get_chain(abo, &ccnt);
> +		if (!cc || !ccnt)
> +			return -EINVAL;
> +
> +		cc->error_index = (cmd_idx < ccnt) ? cmd_idx : 0;
>  		abo = amdxdna_gem_get_obj(client, cc->data[0], AMDXDNA_BO_SHARE);

[Severity: High]
This isn't a bug introduced by this patch, but does this logic write the
error state to the wrong command BO?

In amdxdna_cmd_set_error(), the code correctly assigns cc->error_index based
on cmd_idx, but then it hardcodes cc->data[0] when looking up the BO. If a 
chained command fails at an index greater than 0, won't this clobber the
first command in the chain with error data and completely ignore the actual
failing command?

>  		if (!abo)
>  			return -EINVAL;
>  		cmd = amdxdna_gem_vmap(abo);
>  		if (!cmd) {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260826153121.133507-1-taimuraz@kaitmazov.com?part=1