Re: [PATCH RFC v2 8/8] tee: optee: support RPMI asynchronous notification doorbells

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] tee: optee: UAF in optee_rpmi_notif_callback() due to unsynchronized teardown
--

commit 78a8a655d3ae3b299cafbd7aaef4c7ef599402d8
Author: Amirreza Zarrabi <amirreza.zarrabi@oss.qualcomm.com>

    tee: optee: support RPMI asynchronous notification doorbells
    
This commit adds support for RPMI asynchronous notifications in OP-TEE. It
allocates an RPMI signal, sets up a callback to queue a bottom-half on a
private workqueue, and provides initialization and teardown routines.

> diff --git a/drivers/tee/optee/rpmi_abi.c b/drivers/tee/optee/rpmi_abi.c
> index db541f3de4250..ffe44d81e8571 100644
> --- a/drivers/tee/optee/rpmi_abi.c
> +++ b/drivers/tee/optee/rpmi_abi.c

[ ... ]

> +static void optee_rpmi_notif_callback(struct rpmi_tee_device *rdev, u32 signal,
> +				      void *cb_data)
> +{
> +	struct optee *optee = cb_data;
> +
> +	queue_work(optee->rpmi.notif_wq, &optee->rpmi.notif_work);

[Severity: High]
Can this callback execute after the optee structure has been freed and the
workqueue destroyed?

If an RPMI notification is dispatched just prior to notify_relinquish(), it
appears this callback could run concurrently with or after the teardown
sequence in optee_rpmi_async_notif_uninit(). This would lead to a
use-after-free on the optee structure and a potential NULL pointer
dereference or crash when passing the destroyed workqueue to queue_work().

> +}
> +
> +/* Relinquish is not a barrier for an already-selected transport callback. */
> +static void optee_rpmi_async_notif_uninit(struct optee *optee)
> +{
> +	struct optee_rpmi *rpmi = &optee->rpmi;
> +	struct rpmi_tee_device *rdev = rpmi->rdev;
> +	int ret;
> +
> +	if (!rpmi->notif_wq)
> +		return;
> +
> +	ret = optee_stop_async_notif(optee->ctx);
> +	if (ret)
> +		dev_warn(&rdev->dev, "stop notifications failed: %d\n", ret);
> +
> +	ret = rdev->ops->notifier_ops->notify_relinquish(rdev, rpmi->signal);
> +	if (ret && ret != -EOPNOTSUPP)
> +		dev_warn(&rdev->dev,
> +			 "relinquish notification failed: %d\n", ret);
> +
> +	destroy_workqueue(rpmi->notif_wq);
> +
> +	rpmi->notif_wq = NULL;

[Severity: High]
Does this teardown sequence need to synchronize with in-flight callbacks
before destroying the workqueue and returning?

Because notify_relinquish() does not wait for already-selected callbacks to
finish, destroying the workqueue here and subsequently freeing the optee
structure in optee_rpmi_remove() could race with an executing callback.

> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261005-rpmi-tee-service-grp-dev-v2-0-72f222e23ec1@oss.qualcomm.com?part=8