Re: [PATCH v1 2/3] misc: fastrpc: wake poll-mode waiters on SSR

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] misc/fastrpc: missing memory barrier and data race on ctx->is_work_done allows stale error codes
--

commit ec0bf5065c708ee8356d5c1feae24849edc520bd
Author: Jianping Li <jianping.li@oss.qualcomm.com>

misc: fastrpc: wake poll-mode waiters on SSR

This commit updates fastrpc_notify_users() to set ctx->is_work_done along
with ctx->retval. This ensures that threads spinning in polling mode will
observe the termination and exit, instead of waiting for a timeout when the
DSP goes down.

> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> index 05b2e7e4ad3b7..c54c450cb5714 100644
> --- a/drivers/misc/fastrpc.c
> +++ b/drivers/misc/fastrpc.c
> @@ -2663,6 +2663,7 @@ static void fastrpc_notify_users(struct fastrpc_user *user)
>  	spin_lock(&user->lock);
>  	list_for_each_entry(ctx, &user->pending, node) {
>  		ctx->retval = -EPIPE;
> +		ctx->is_work_done = true;

[Severity: Medium]
Is there a missing write memory barrier here?

If the stores to ctx->retval and ctx->is_work_done are reordered by the
compiler or the CPU, a concurrent thread executing poll_for_remote_response()
might read ctx->is_work_done as true but read a stale value for ctx->retval,
such as the initialization value -1 or -EPERM.

In poll_for_remote_response(), a concurrent thread evaluates this locklessly:

        ...
        (val == FASTRPC_POLL_RESPONSE) || ctx->is_work_done

Even if the polling thread uses dma_rmb() before reading ctx->retval, that
only ensures read-read hardware ordering. It does not prevent this writer
thread from reordering the stores. Would it be safer to use
smp_store_release() for ctx->is_work_done here and smp_load_acquire() in the
polling loop?

Additionally, does this introduce a plain C data race?

poll_for_remote_response() appears to locklessly write to ctx->is_work_done
on DSP polling completion. This could cause undefined behavior when racing
with the lockless reads and this new write in fastrpc_notify_users().

>  		complete(&ctx->work);
>  	}
>  	spin_unlock(&user->lock);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261007084447.922-1-jianping.li@oss.qualcomm.com?part=2