Re: [PATCH] optee: Do not send requests to supplicant during shutdown

[Jens Wiklander <jens.wiklander@linaro.org>]

[-- Attachment #1: Type: text/plain, Size: 2656 bytes --]

On Wed, Jan 19, 2022 at 06:49:33PM +0100, Lars Persson wrote:
> The addition of a shutdown hook by commit f25889f93184 ("optee:
> fix tee out of memory failure seen during kexec reboot") introduced a
> kernel shutdown regression that can be triggered after running the
> xtest suites.
> 
> Once the shutdown hook is called it is not possible to communicate any
> more with the supplicant process because the system is not scheduling
> task any longer. Thus if the optee driver shutdown path receives a
> supplicant RPC request from the OP-TEE we will deadlock the kernel's
> shutdown.
> 
> This unexpected event will in fact occur after the xtest suite has
> been run. It seems some cached SHM kept alive a context object which
> in turn kept alive a session towards a PTA or TA. Closing the session
> results in a socket RPC command being sent back from OP-TEE.
> 
> This sequence of events is captured by a 5.15 kernel annotated with
> extra prints:
> 
>  Calling OPTEE_SMC_DISABLE_SHM_CACHE
>  OPTEE_SMC_DISABLE_SHM_CACHE returned 0
>   freeing SHM ptr 0xFFFFFF8001079380
>  Calling OPTEE_SMC_DISABLE_SHM_CACHE
>  OPTEE_SMC_DISABLE_SHM_CACHE returned 0
>   freeing SHM ptr 0xFFFFFF8001CC5580
>  Calling OPTEE_SMC_DISABLE_SHM_CACHE
>  OPTEE_SMC_DISABLE_SHM_CACHE returned 0
>   freeing SHM ptr 0xFFFFFF8006308A80
>  Calling OPTEE_SMC_DISABLE_SHM_CACHE
>  OPTEE_SMC_DISABLE_SHM_CACHE returned 0
>   freeing SHM ptr 0xFFFFFF8006308B00
>  optee: optee_handle_rpc: a0=0XFFFF0000 a1=0XA0 a2=0X0
>  optee: optee_handle_rpc: a0=0XFFFF0005 a1=0XFFFFFF80 a2=0X61E6500
>  optee: handle_rpc_func_cmd: cmd = 0XA
>  optee_supp_thrd_req: func=0XA
> 
> Introduce a shutdown state in the optee device object to return an
> immediate error to all RPC requests in the shutdown path.
> 
> Fixes: f25889f93184 ("optee: fix tee out of memory failure seen during kexec reboot
> Signed-off-by: Lars Persson <larper@axis.com>
> ---
>  drivers/tee/optee/optee_private.h | 1 +
>  drivers/tee/optee/smc_abi.c       | 5 ++++-
>  drivers/tee/optee/supp.c          | 8 ++++++++
>  3 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/tee/optee/optee_private.h b/drivers/tee/optee/optee_private.h
> index 46f74ab07c7e..83380974ff44 100644
> --- a/drivers/tee/optee/optee_private.h
> +++ b/drivers/tee/optee/optee_private.h
> @@ -164,6 +164,7 @@ struct optee {
>  	bool   scan_bus_done;
>  	struct workqueue_struct *scan_bus_wq;
>  	struct work_struct scan_bus_work;
> +	bool shutting_down;

Please move this to right after "bool scan_bus_done" above to avoid
unnecessary extra padding.

Thanks,
Jens