Re: [PATCH] asymmetric_keys: check asymmetric_key_ids() for NULL before dereference

Hi,

Thanks for the report.

On Wed, Apr 29, 2026 at 7:17 PM Weiming Shi <[email protected]> wrote:
>
> asymmetric_key_ids() returns key->payload.data[asym_key_ids], which can
> be NULL for keys parsed by the PKCS#8 parser (pkcs8_parser.c explicitly
> stores NULL in prep->payload.data[asym_key_ids]).
>
> key_or_keyring_common() in restrict.c and find_asymmetric_key() in
> asymmetric_type.c both dereference this return value without checking
> for NULL. An unprivileged user can trigger a NULL pointer dereference
> in key_or_keyring_common() by creating a PKCS#8 key, restricting a
> keyring with key_or_keyring:<pkcs8_serial>, and adding an X.509 cert
> to the restricted keyring. CONFIG_PKCS8_PRIVATE_KEY_PARSER=y is

Could you add a simple bash script for this to the commit message?

> required.
>
>  Oops: general protection fault, probably for non-canonical address 0xdffffc0000000000
>  KASAN: null-ptr-deref in range [0x0000000000000000-0x0000000000000007]
>  RIP: 0010:key_or_keyring_common (crypto/asymmetric_keys/restrict.c:205 crypto/asymmetric_keys/restrict.c:279)
>  Call Trace:
>   <TASK>
>   __key_create_or_update (security/keys/key.c:884)
>   key_create_or_update (security/keys/key.c:1021)
>   __do_sys_add_key (security/keys/keyctl.c:134)
>   do_syscall_64 (arch/x86/entry/common.c:52 arch/x86/entry/common.c:83)
>   entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:130)
>   </TASK>
>  Kernel panic - not syncing: Fatal exception
>
> Add a NULL check in find_asymmetric_key(), mirroring the existing
> pattern in asymmetric_match_key_ids() and asymmetric_key_describe().
> In key_or_keyring_common(), skip the trusted key matching when it
> has no key IDs and fall through to the check_dest path.
>
> Fixes: 7d30198ee24f ("keys: X.509 public key issuer lookup without AKID")
> Reported-by: Xiang Mei <[email protected]>
> Signed-off-by: Weiming Shi <[email protected]>
> ---
>  crypto/asymmetric_keys/asymmetric_type.c | 2 ++
>  crypto/asymmetric_keys/restrict.c        | 9 ++++++---
>  2 files changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/crypto/asymmetric_keys/asymmetric_type.c b/crypto/asymmetric_keys/asymmetric_type.c
> --- a/crypto/asymmetric_keys/asymmetric_type.c
> +++ b/crypto/asymmetric_keys/asymmetric_type.c
> @@ -109,6 +109,8 @@ struct key *find_asymmetric_key(struct key *keyring,
>         if (id_0 && id_1) {
>                 const struct asymmetric_key_ids *kids = asymmetric_key_ids(key);
>
> +               if (!kids)
> +                       goto reject;
>                 if (!kids->id[1]) {
>                         pr_debug("First ID matches, but second is missing\n");
>                         goto reject;
> diff --git a/crypto/asymmetric_keys/restrict.c b/crypto/asymmetric_keys/restrict.c
> --- a/crypto/asymmetric_keys/restrict.c
> +++ b/crypto/asymmetric_keys/restrict.c
> @@ -243,10 +243,14 @@ static int key_or_keyring_common(struct key *dest_keyring,
>                         if (IS_ERR(key))
>                                 key = NULL;
>                 } else if (trusted->type == &key_type_asymmetric) {
> +                       const struct asymmetric_key_ids *kids;
>                         const struct asymmetric_key_id **signer_ids;
>
> -                       signer_ids = (const struct asymmetric_key_id **)
> -                               asymmetric_key_ids(trusted)->id;
> +                       kids = asymmetric_key_ids(trusted);
> +                       if (!kids)
> +                               goto skip_trusted;
> +
> +                       signer_ids = (const struct asymmetric_key_id **)kids->id;
>
>                         /*
>                          * The auth_ids come from the candidate key (the
> @@ -290,6 +294,7 @@ static int key_or_keyring_common(struct key *dest_keyring,
>                 }
>         }
>
> +skip_trusted:
>         if (check_dest && !key) {
>                 /* See if the destination has a key that signed this one. */
>                 key = find_asymmetric_key(dest_keyring, sig->auth_ids[0],
> --
> 2.39.0
>

Ignat