Re: [PATCH v5 20/23] rust: public_key: Add crypto public key bindings

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] rust: public_key: missing unsafe on PublicKey::from_raw()
- [Low] rust: public_key: wrong Kconfig guard on public_key module in lib.rs
- [Low] rust: public_key: missing inline attributes and split imports
--

commit d3b51b7112cb9e69d8cc7083f31a2a45bf366492
Author: Alistair Francis <alistair.francis@wdc.com>

rust: public_key: Add crypto public key bindings

This commit adds initial Rust bindings and a PublicKey wrapper around
the kernel crypto public key (asymmetric-keys) APIs, including
PublicKey::from_raw(), PublicKey::verify_signature(), and Drop cleanup via
public_key_free().

> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index f7db5975e7621..e8b652faa3b49 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -122,6 +122,8 @@
>  pub mod print;
>  pub mod processor;
>  pub mod ptr;
> +#[cfg(CONFIG_CRYPTO)]
> +pub mod public_key;

[Severity: Low]
Will gating the public_key module on CONFIG_CRYPTO break the build when
CONFIG_RUST=y and CONFIG_CRYPTO=y, while
CONFIG_ASYMMETRIC_PUBLIC_KEY_SUBTYPE is n or m?

In rust/kernel/public_key.rs, PublicKey::verify_signature() calls
bindings::public_key_verify_signature() and PublicKey::drop() calls
bindings::public_key_free(). Both functions are defined in
crypto/asymmetric_keys/public_key.c, which is built under the tristate
symbol CONFIG_ASYMMETRIC_PUBLIC_KEY_SUBTYPE rather than CONFIG_CRYPTO:

crypto/asymmetric_keys/Makefile:
    obj-$(CONFIG_ASYMMETRIC_PUBLIC_KEY_SUBTYPE) += public_key.o

When CONFIG_ASYMMETRIC_PUBLIC_KEY_SUBTYPE=n, include/crypto/public_key.h
defines public_key_verify_signature() as a static inline stub:

include/crypto/public_key.h:
    #if IS_REACHABLE(CONFIG_ASYMMETRIC_PUBLIC_KEY_SUBTYPE)
    int public_key_verify_signature(const struct public_key *pkey,
                                    const struct public_key_signature *sig);
    #else
    static inline
    int public_key_verify_signature(const struct public_key *pkey,
                                    const struct public_key_signature *sig)
    {
            return -EINVAL;
    }
    #endif

Without a Rust helper wrapper, bindgen does not generate
bindings::public_key_verify_signature, and public_key_free() has no stub at
all. Additionally, when CONFIG_ASYMMETRIC_PUBLIC_KEY_SUBTYPE=m, built-in
rust/kernel.o cannot link against public_key_verify_signature() and
public_key_free() in the loadable public_key.ko module.

Should this module instead be gated on
#[cfg(CONFIG_ASYMMETRIC_PUBLIC_KEY_SUBTYPE = "y")]?

> diff --git a/rust/kernel/public_key.rs b/rust/kernel/public_key.rs
> new file mode 100644
> index 0000000000000..e1d51333c1179
> --- /dev/null
> +++ b/rust/kernel/public_key.rs
> @@ -0,0 +1,69 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +// Copyright (C) 2026 Western Digital
> +
> +//! Wrappers around the Linux kernel crypto public key APIs.
> +//!
> +//! C headers: [`include/crypto/public_key.h`](srctree/include/crypto/public_key.h).
> +//!
> +//! Reference: <https://www.kernel.org/doc/html/latest/crypto/asymmetric-keys.html>
> +
> +use core::ptr::NonNull;
> +use kernel::prelude::*;
> +use kernel::{
> +    bindings,
> +    error::to_result,
> +    str::CStr, //
> +};
> +
> +/// Owned `struct public_key` wrapper.
> +pub struct PublicKey {
> +    key: NonNull<bindings::public_key>,
> +}
> +
> +impl PublicKey {
> +    /// Create a Rust `PublicKey` from a valid C `struct public_key` pointer.
> +    pub fn from_raw(key: NonNull<bindings::public_key>) -> Self {
> +        Self { key }
> +    }

[Severity: Medium]
Should PublicKey::from_raw() be marked unsafe?

NonNull<bindings::public_key> is Copy and can be constructed or duplicated
in safe Rust (for example via NonNull::dangling() or passing the same
NonNull pointer twice). However, PublicKey::verify_signature() passes
self.key.as_ptr() to bindings::public_key_verify_signature(), which
dereferences the pointer and its internal fields, and PublicKey::drop()
unconditionally passes self.key.as_ptr() to bindings::public_key_free(),
which calls kfree_sensitive(key->key), kfree(key->params), and kfree(key).

Although Certificate::take_public_key() in a later commit in this series
passes a valid pointer and clears cert->pub_, leaving PublicKey::from_raw()
as a safe public function makes the abstraction unsound for safe callers.
Could PublicKey document its type invariants in an Invariants section, and
could PublicKey::from_raw() be made an unsafe fn with a Safety section and
an invariant comment?

> +
> +    /// Verify a signature using this public key.
> +    pub fn verify_signature(
> +        &self,
> +        signature: &[u8],
> +        encoding: &'static CStr,
> +        hash_algo: &'static CStr,
> +        message: &[u8],
> +    ) -> Result {
> +        let sig = bindings::public_key_signature {
> +            s: signature.as_ptr().cast_mut(),
> +            s_size: signature.len() as u32,
> +            encoding: encoding.as_char_ptr().cast::<u8>(),
> +            hash_algo: hash_algo.as_char_ptr().cast::<u8>(),
> +            m: message.as_ptr().cast_mut(),
> +            m_size: message.len() as u32,
> +            ..Default::default()
> +        };
> +
> +        // SAFETY: `self.key` is a valid public key handle, and `sig` points to
> +        // valid signature/message data owned by the caller for the duration of
> +        // this call.
> +        unsafe {
> +            to_result(bindings::public_key_verify_signature(
> +                self.key.as_ptr(),
> +                &sig,
> +            ))
> +        }
> +    }
> +}
> +
> +impl Drop for PublicKey {
> +    fn drop(&mut self) {
> +        // SAFETY: `self.key` was either returned by `x509_cert_parse()` or
> +        // manually transferred from an `x509_certificate` and has not been
> +        // freed yet.
> +        unsafe {
> +            bindings::public_key_free(self.key.as_ptr());
> +        }
> +    }
> +}

[Severity: Low]
This isn't a bug, but per the Rust coding guidelines for the kernel crate,
could PublicKey::from_raw(), PublicKey::verify_signature(), and
PublicKey::drop() be annotated with #[inline], and could the two
use kernel::... blocks at the top of rust/kernel/public_key.rs be merged
into a single vertical import block (noting that CStr is already exported by
kernel::prelude::*)?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261027052536.1722081-1-alistair.francis@wdc.com?part=20