Re: [PATCH] accel/amdxdna: put the chained BO when its mapping fails

On 8/19/26 16:08, Taimuraz Kaitmazov wrote:
> amdxdna_cmd_set_error() looks up the first BO of a command chain, which
> takes a reference, and drops it at the end of the function. The mapping of
> that BO is established in between, and the failure path returns without the
> put, so the reference is leaked.
>
> Ordinary use does not reach it. The chain has been submitted before any of
> this runs, so aie2_cmdlist_fill_slot() has already called
> amdxdna_cmd_get_op() on that BO and amdxdna_gem_vmap() has cached its
> address. What makes it reachable is that the BO is resolved again by
> handle here, and the handle is userspace's to recycle: closing it after
> submission and importing a dma-buf whose exporter implements no vmap onto
> the same id leaves amdxdna_gem_get_obj() returning an object this cannot
> map, since prime_import() types every import AMDXDNA_BO_SHARE.
>
> Signed-off-by: Taimuraz Kaitmazov <[email protected]>
> ---
>   drivers/accel/amdxdna/amdxdna_ctx.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/accel/amdxdna/amdxdna_ctx.c b/drivers/accel/amdxdna/amdxdna_ctx.c
> index 855da8c79a1c..4e4a9cf5c327 100644
> --- a/drivers/accel/amdxdna/amdxdna_ctx.c
> +++ b/drivers/accel/amdxdna/amdxdna_ctx.c
> @@ -183,8 +183,10 @@ int amdxdna_cmd_set_error(struct amdxdna_gem_obj *abo,
>   		if (!abo)
>   			return -EINVAL;
>   		cmd = amdxdna_gem_vmap(abo);
> -		if (!cmd)
> +		if (!cmd) {
> +			amdxdna_gem_put_obj(abo);
>   			return -ENOMEM;
> +		}

Thanks for the fix. I will add 'Fixes' tag when I merge.

Reviewed-by: Lizhi Hou <[email protected]>


Lizhi

>   	}
>   
>   	memset(cmd->data, 0xff, abo->mem.size - sizeof(*cmd));