Re: [PATCH v4 06/11] KEYS: trusted: TEE: Disable sysfs bind attributes

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [High] KEYS: trusted: premature register_key_type() before
  init_trusted() setup [1]
--

commit 3bd98fb38b1322342ed23c63a9f83d8e5a79dd02
Author: Jarkko Sakkinen <jarkko@kernel.org>

KEYS: trusted: TEE: Disable sysfs bind attributes

This commit sets suppress_bind_attrs on trusted_key_driver to prevent
userspace from unbinding the driver via sysfs and unregistering
key_type_trusted while the module is loaded.

> diff --git a/security/keys/trusted-keys/trusted_tee.c b/security/keys/trusted-keys/trusted_tee.c
> index 6e465c8bef5e5..227efb668aa95 100644
> --- a/security/keys/trusted-keys/trusted_tee.c
> +++ b/security/keys/trusted-keys/trusted_tee.c
> @@ -263,7 +263,8 @@ static struct tee_client_driver trusted_key_driver = {
>  	.remove		= trusted_key_remove,
>  	.id_table	= trusted_key_id_table,
>  	.driver		= {
> -		.name		= DRIVER_NAME,
> +		.name			= DRIVER_NAME,
> +		.suppress_bind_attrs	= true,
>  	},
>  };

[Severity: High]
This problem wasn't introduced by this patch, but can a concurrent
add_key() syscall invoke trusted_instantiate() before the backend is fully
initialized [1]?

Each backend's ->init() callback (including trusted_key_probe() for
trusted_key_driver) calls register_key_type(&key_type_trusted) before
returning to init_trusted(), which only then populates the static calls
and backend state:

CPU0                              CPU1
init_trusted()                    sys_add_key()
  ops->init()
    register_key_type(&key_type_trusted)
                                    key_type_lookup("trusted")
                                    ...
                                      trusted_instantiate()
                                        static_call(trusted_key_unseal)()
  static_call_update(trusted_key_seal, ...)
  static_call_update(trusted_key_unseal, ...)
  static_call_update(trusted_key_get_random, ...)
  migratable = ...

Calling an uninitialized DEFINE_STATIC_CALL_NULL target dereferences a
NULL function pointer when CONFIG_HAVE_STATIC_CALL is disabled, or runs
__static_call_nop() and leaves the return value uninitialized when it is
enabled.  In addition, trusted_key_probe() calls register_key_type()
before setting pvt_data.dev, and if TEE probing is deferred until after
init_trusted() returns, the static_call_update() calls in init_trusted()
never run at all.

[1] https://sashiko.dev/#/bug/linux-384065c3-cac2-4397-8b30-74edea3d1ff3

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261107131512.3603168-1-jarkko@kernel.org?part=6