Re: [PATCH v2 6/8] optee: Add support for arm,ffa-lend-pool

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] tee: optee: premature lend pool detachment prevents direct map restore
--

commit 4b4b22cda5d128d440dc2543a21dd2f20cdf757f
Author: Vincent Donnefort <vdonnefort@google.com>

optee: Add support for arm,ffa-lend-pool
    
Hook OP-TEE dynamically allocated protected memory pools to the
"arm,ffa-lend-pool" driver. While the SMC transport platform device
resolves the pool through its DT "memory-region" property, the FF-A
transport lacks a device tree node and binds via ffa_lend_pool_attach().

> diff --git a/drivers/tee/optee/core.c b/drivers/tee/optee/core.c
> index a52c1f498b996..39f315cea830d 100644
> --- a/drivers/tee/optee/core.c
> +++ b/drivers/tee/optee/core.c
> @@ -201,6 +201,12 @@ void optee_remove_common(struct optee *optee)
>  	/* Unregister OP-TEE specific client devices on TEE bus */
>  	optee_unregister_devices();
>  
> +	/*
> +	 * Must follow optee_unregister_devices(). Clients require the lend pool
> +	 * linkage to successfully free their memory.
> +	 */
> +	optee_lend_pool_unregister(optee);
> +
>  	optee_notif_uninit(optee);
>  	optee_shm_arg_cache_uninit(optee);
>  	teedev_close_context(optee->ctx);

[Severity: High]
Does detaching the lend pool here before tee_device_unregister() risk
leaving CMA pages unmapped?

If a user-space process holds a DMA buffer open during driver unbind,
tee_device_unregister() (called shortly after this) will block waiting for
the teedev refcount to drop to zero.

When the user-space process finally closes the buffer, release_dyn_protmem()
will invoke optee_lend_pool_reclaimed() to restore the direct map.

If optee_lend_pool_unregister() has already detached the pool, will the
direct map restore fail? This could cause the CMA pages to be returned to
the allocator unmapped, leading to a fatal synchronous data abort when the
kernel later accesses them.

[ ... ]

> diff --git a/drivers/tee/optee/ffa_abi.c b/drivers/tee/optee/ffa_abi.c
> index 633715b98625c..e979dbc9a5475 100644
> --- a/drivers/tee/optee/ffa_abi.c
> +++ b/drivers/tee/optee/ffa_abi.c
> @@ -1172,6 +1180,7 @@ static int optee_ffa_probe(struct ffa_device *ffa_dev)
>  
>  err_unregister_devices:
>  	optee_unregister_devices();
> +	optee_lend_pool_unregister(optee);
>  	if (optee->ffa.bottom_half_value != U32_MAX)
>  		notif_ops->notify_relinquish(ffa_dev,

[Severity: High]
Does this error path in optee_ffa_probe() have the same issue with premature
detachment? 

It seems optee_lend_pool_unregister() runs before err_unreg_teedev calls
tee_device_unregister(). If a DMA allocation was active during a probe
failure, the delayed cleanup might fail to restore the direct map.

[ ... ]

> diff --git a/drivers/tee/optee/smc_abi.c b/drivers/tee/optee/smc_abi.c
> index b8a2bdac3208f..e46eb881cf903 100644
> --- a/drivers/tee/optee/smc_abi.c
> +++ b/drivers/tee/optee/smc_abi.c
> @@ -1932,6 +1942,7 @@ static int optee_probe(struct platform_device *pdev)
>  		optee_disable_shm_cache(optee);
>  	optee_smc_notif_uninit_irq(optee);
>  	optee_unregister_devices();
> +	optee_lend_pool_unregister(optee);
>  err_notif_uninit:
>  	optee_notif_uninit(optee);
>  err_close_ctx:

[Severity: High]
Does this error path in optee_probe() also detach the pool before
tee_device_unregister() ensures DMA buffers are closed?

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