Re: [PATCH 3/4] accel/amdxdna: release the chained command BO when vmap fails

[Lizhi Hou]

On 9/12/26 01:10, Eva Crystal wrote:
> When amdxdna_cmd_set_error() follows a command chain it takes a
> reference on the BO named by the chain:
>
> 	abo = amdxdna_gem_get_obj(client, cc->data[0], AMDXDNA_BO_SHARE);
> 	if (!abo)
> 		return -EINVAL;
> 	cmd = amdxdna_gem_vmap(abo);
> 	if (!cmd)
> 		return -ENOMEM;
>
> and drops it at the end of the function under "if (cc)". The -ENOMEM
> path returns before reaching that, so the reference taken by
> amdxdna_gem_get_obj() is leaked and the GEM object is never freed.
>
> amdxdna_gem_vmap() fails only if drm_gem_vmap() fails, which needs
> memory pressure or an exporter that refuses the mapping, so this is a
> small leak on a rare path rather than something a caller can drive at
> will. It is still a leak, and the chain BO handle comes from a command
> buffer user space can write.
>
> Drop the reference before returning.
>
> Signed-off-by: Eva Crystal <0xiviel@xxxxxxxxx>
> ---
>   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 c24bf1c..7a61e83 100644
> --- a/drivers/accel/amdxdna/amdxdna_ctx.c
> +++ b/drivers/accel/amdxdna/amdxdna_ctx.c
> @@ -170,8 +170,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 providing the patch. This has been fixed by:

https://lore.kernel.org/all/20260819230852.287751-1-taimuraz@xxxxxxxxxxxxx/


Lizhi

>   	}
>   
>   	if (abo->mem.size < sizeof(*cmd)) {