Re: [PATCH RFC v2 4/8] tee: optee: add RPMI dynamic shared-memory pool

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] tee: optee: UAF in optee_rpmi_shm_rht_rm() due to synchronous kfree()
- [Low] build: unused static function optee_rpmi_shm_pool_alloc breaks bisect
--

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

tee: optee: add RPMI dynamic shared-memory pool

This patch adds a page-based dynamic shared-memory pool for the RPMI backend
using the common TEE allocation and free helpers. It registers allocated
pages as RPMI parcels so OP-TEE can access them, and unregisters the parcels
before freeing their backing pages.

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

[ ... ]

> +static void optee_rpmi_pool_free(struct tee_shm_pool *pool, struct tee_shm *shm)
> +{
> +	tee_dyn_shm_free_helper(shm, optee_rpmi_shm_unregister);
> +}

[Severity: High]
Is there a risk of a use-after-free during bucket traversal here?

When tee_dyn_shm_free_helper() executes optee_rpmi_shm_unregister() in
drivers/tee/optee/rpmi_abi.c, it subsequently calls optee_rpmi_shm_rht_rm()
to remove the hash table entry. This same path is also triggered via
optee_rpmi_shm_unregister_supp().

In optee_rpmi_shm_rht_rm():

	if (entry)
		ret = rhashtable_remove_fast(&optee->rpmi.shm_rht,
					     &entry->node,
					     optee_rpmi_shm_rht_params);
	if (!ret)
		kfree(entry);

Because rhashtable uses RCU-based worker threads (rht_deferred_worker) to
traverse the hash table concurrently during automatic resizing, freeing
the entry synchronously via kfree() instead of kfree_rcu() might cause a
use-after-free if a rehash worker is actively traversing the removed node's
bucket list.

Note that this issue is reachable from unprivileged user space by repeatedly
registering and unregistering memory, driving the size of the table up and
down to trigger rehashing.

[ ... ]

> +static struct tee_shm_pool *optee_rpmi_shm_pool_alloc(void)
> +{
> +	struct tee_shm_pool *pool;
> +
> +	pool = kzalloc_obj(*pool);
> +	if (!pool)
> +		return ERR_PTR(-ENOMEM);
> +
> +	pool->ops = &optee_rpmi_pool_ops;
> +
> +	return pool;
> +}

[Severity: Low]
Could this unused function break git bisect?

The static function optee_rpmi_shm_pool_alloc() in
drivers/tee/optee/rpmi_abi.c is defined here but is never invoked in this
patch. Under CONFIG_WERROR=y, the compiler warning for an unused function
promotes to a build error.

While this is fixed later in the series by commit
3ef22394684ff6d8b2126c91fa4c8bb2d1960d68 (tee: optee: bind RPMI services and
negotiate backend capabilities), each individual patch in a series
should compile cleanly to ensure git bisect continues to work.

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