Re: [PATCH V1 12/20] accel/amdxdna: Implement AIE4 kernel queue lifecycle and memory layout

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] amdxdna: System-wide DoS via dev_lock when sleeping with SRCU read lock held
--

commit 132c3fc1d7de0e87a5c560f0114f2517ab24e40c
Author: David Zhang <yidong.zhang@amd.com>

accel/amdxdna: Implement AIE4 kernel queue lifecycle and memory layout

This commit initializes the kernel-mode submission required buffers, workqueue,
and hardware contexts for AIE4. It updates the queue definition, adds a job
workqueue for pending and running jobs, initializes direct and indirect packets,
and implements hardware context lifecycle operations.

> diff --git a/drivers/accel/amdxdna/aie4_ctx.c b/drivers/accel/amdxdna/aie4_ctx.c
> index 5a2fc19bad206..3927c9fef05f9 100644
> --- a/drivers/accel/amdxdna/aie4_ctx.c
> +++ b/drivers/accel/amdxdna/aie4_ctx.c

[ ... ]

> @@ -301,10 +456,12 @@ static inline bool check_cmd_done(struct amdxdna_hwctx *hwctx, u64 seq)
>  int aie4_cmd_wait(struct amdxdna_hwctx *hwctx, u64 seq, u32 timeout)
>  {
>  	unsigned long wait_jifs = MAX_SCHEDULE_TIMEOUT;
> -	struct amdxdna_hwctx_priv *priv = hwctx->priv;
> -	struct cert_comp *cert_comp = priv->cert_comp;
> +	struct cert_comp *cert_comp = aie4_get_cert_comp(hwctx);
>  	long ret;
>  
> +	if (!cert_comp)
> +		return -EAGAIN;
> +
>  	if (timeout)
>  		wait_jifs = msecs_to_jiffies(timeout);
>  
>  	ret = wait_event_interruptible_timeout(cert_comp->waitq,
>  					       (check_cmd_done(hwctx, seq)),
>  					       wait_jifs);

[Severity: High]
Could this indefinite sleep cause a system-wide denial of service by deadlocking
the device?

If a user calls the wait ioctl with an infinite timeout, the caller acquires
srcu_read_lock(&client->hwctx_srcu) and sleeps indefinitely in
aie4_cmd_wait() waiting for hardware completion.

Concurrently, if another thread attempts to destroy the same hardware context,
it will acquire the global xdna->dev_lock and call amdxdna_hwctx_destroy_rcu().
This function blocks on synchronize_srcu(&client->hwctx_srcu), waiting for the
wait ioctl to drop the SRCU read lock.

However, the hardware destruction that would wake up the wait queue is called 
after synchronize_srcu():

amdxdna_hwctx_destroy_rcu() {
    ...
    synchronize_srcu(&client->hwctx_srcu);
    
    /* At this point, user is not able to submit new commands */
    xdna->dev_info->ops->hwctx_fini(hwctx);
}

This creates a circular dependency where aie4_cmd_wait() will never wake up
because hwctx_fini() cannot run, leaving xdna->dev_lock held forever. Could we
handle the destruction order differently or abort the wait before calling
synchronize_srcu()?

>  	if (!ret)
>  		ret = -ETIME;
>  
> +	aie4_put_cert_comp(cert_comp);
> +
>  	return ret <= 0 ? ret : 0;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260930033233.1727265-1-yidong.zhang@amd.com?part=12