Re: [PATCH v3 8/8] optee: Add support for lendable-dma-pool

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [High] optee: restore_direct_map_dyn_protmem() called when
  reclaim_protmem() fails
--

commit 1a801ca7487b0859c4bfdf62e4f1116b16a1b7fe
Author: Vincent Donnefort <vdonnefort@google.com>

optee: Add support for lendable-dma-pool

This commit uses dma_zap_direct_map() and dma_restore_direct_map() to
unmap dynamically allocated protected memory from the kernel linear map
before lending it to the secure world and restore it upon reclaim.

> diff --git a/drivers/tee/optee/protmem.c b/drivers/tee/optee/protmem.c
> index be3abf6e8aa6d..10e8f1540e5e8 100644
> --- a/drivers/tee/optee/protmem.c
> +++ b/drivers/tee/optee/protmem.c

[ ... ]

> @@ -76,6 +103,8 @@ static int init_dyn_protmem(struct optee_protmem_dyn_pool *rp)
>  	rp->gen_pool = NULL;
>  err_reclaim:
>  	rp->optee->ops->reclaim_protmem(rp->optee, rp->protmem);
> +err_restore_direct_map:
> +	restore_direct_map_dyn_protmem(rp);
>  err_put_shm:
>  	tee_shm_put(rp->protmem);
>  err_null_protmem:
> @@ -112,6 +141,7 @@ static void release_dyn_protmem(struct optee_protmem_dyn_pool *rp)
>  	rp->gen_pool = NULL;
>  
>  	rp->optee->ops->reclaim_protmem(rp->optee, rp->protmem);
> +	restore_direct_map_dyn_protmem(rp);
>  	rp->protmem->flags &= ~TEE_SHM_DYNAMIC;
>  
>  	WARN(refcount_read(&rp->protmem->refcount) != 1, "Unexpected refcount");

[Severity: High]
Can restore_direct_map_dyn_protmem() run while rp->protmem is still lent
to the secure world if reclaim_protmem() fails?

reclaim_protmem() can fail, for example if optee_get_msg_arg() returns
-ENOMEM in optee_smc_reclaim_protmem() or if the secure world has not
relinquished the memory handle in optee_ffa_reclaim_protmem().  Both
init_dyn_protmem() and release_dyn_protmem() ignore that return value and
unconditionally call restore_direct_map_dyn_protmem(rp).

On arm64, arch_dma_restore_direct_map() remaps the pages as cacheable in
the linear map and runs dcache_inval_poc() across the range, which
re-exposes the still-lent pages to CPU speculative reads and triggers a
synchronous external abort or SError on the cache maintenance before
tee_shm_put() returns the pages to the allocator.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261111110738.872947-1-vdonnefort@google.com?part=8