Re: [PATCH V3] accel/amdxdna: Prevent PM resume deadlock in hwctx_sync_debug_bo()

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Concurrent destruction of hwctx causes a Use-After-Free because `dev_lock` is temporarily dropped during PM runtime resume without holding the SRCU read lock.
--

commit 3f02e0b8320e3b9382db4505bfac1b4c92265f29
Author: Lizhi Hou <lizhi.hou@amd.com>

accel/amdxdna: Prevent PM resume deadlock in hwctx_sync_debug_bo()

This commit addresses a potential deadlock by calling amdxdna_pm_resume_get()
before acquiring xdna->dev_lock in both amdxdna_hwctx_sync_debug_bo() and
amdxdna_drm_config_hwctx_ioctl().

> diff --git a/drivers/accel/amdxdna/amdxdna_ctx.c b/drivers/accel/amdxdna/amdxdna_ctx.c
> index 855da8c79a1cd..67a2abcf173ee 100644
> --- a/drivers/accel/amdxdna/amdxdna_ctx.c
> +++ b/drivers/accel/amdxdna/amdxdna_ctx.c
> @@ -382,16 +382,25 @@ int amdxdna_drm_config_hwctx_ioctl(struct drm_device *dev, void *data, struct dr
[ ... ]
> +	mutex_lock(&xdna->dev_lock);
>  	hwctx = xa_load(&client->hwctx_xa, args->handle);
>  	if (!hwctx) {
>  		XDNA_DBG(xdna, "PID %d failed to get hwctx %d", client->pid, args->handle);
>  		ret = -EINVAL;
> -		goto free_buf;
> +		goto unlock;
>  	}
>  
>  	ret = xdna->dev_info->ops->hwctx_config(hwctx, args->param_type, val, buf, buf_size);

[Severity: High]
This isn't a bug introduced by this patch, but does dropping the lock inside
this callback expose hwctx to a use-after-free?

If hwctx_config() (for example, aie2_hwctx_cu_config()) calls
amdxdna_pm_resume_get_locked(xdna), which temporarily unlocks and re-locks
xdna->dev_lock:

	mutex_unlock(&xdna->dev_lock);
	ret = amdxdna_pm_resume_get(xdna);
	mutex_lock(&xdna->dev_lock);

During this window, could a concurrent amdxdna_drm_destroy_hwctx_ioctl()
acquire dev_lock, erase hwctx from the xarray, and call
amdxdna_hwctx_destroy_rcu()?

Because the SRCU read lock (client->hwctx_srcu) is not held here,
synchronize_srcu() wouldn't block, and hwctx could be freed while still
in use after re-acquiring dev_lock.

It appears a similar issue might also exist in amdxdna_update_heap(), which
iterates over hwctx_xa without the SRCU read lock and drops both mm_lock
and dev_lock inside the loop body via amdxdna_hwctx_expand_heap().

>  
> +unlock:
> +	mutex_unlock(&xdna->dev_lock);
> +	amdxdna_pm_suspend_put(xdna);
>  free_buf:
>  	kfree(buf);
>  	return ret;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260616212429.3620645-1-lizhi.hou@amd.com?part=1