Re: [PATCH v5 14/23] lib: rspdm: Support SPDM get_version

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] rspdm: missing unsafe and Safety doc on spdm_authenticate()
--

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

lib: rspdm: Support SPDM get_version

This commit adds support for the GET_VERSION SPDM command and calls
state.get_version() from spdm_authenticate().

> diff --git a/lib/rspdm/lib.rs b/lib/rspdm/lib.rs
> index 27449ac1ca25d..e0fb7b49d4a07 100644
> --- a/lib/rspdm/lib.rs
> +++ b/lib/rspdm/lib.rs
> @@ -238,7 +238,30 @@ fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
>  ///
>  /// See `include/linux/spdm.h` for the full C API documentation.
>  #[export]
> -pub extern "C" fn spdm_authenticate(_state_ptr: *mut spdm_state) -> c_int {
> +pub extern "C" fn spdm_authenticate(state_ptr: *mut spdm_state) -> c_int {
> +    if state_ptr.is_null() {
> +        return -(bindings::EINVAL as c_int);
> +    }
> +
> +    // SAFETY: `state_ptr` was returned from `spdm_create()` which used `into_foreign()`
> +    // to create the pointer, and it remains valid until `spdm_destroy()` is called.
> +    // We only borrow here (rather than `from_foreign()`) so that ownership stays
> +    // with the foreign (C) caller.
> +    // The exclusive `&mut SpdmState` lives entirely inside the lock guard, so
> +    // concurrent FFI callers serialize on the mutex and can never form
> +    // aliased `&mut SpdmState` references.
> +    let mutex = unsafe {
> +        <Pin<KBox<Mutex<SpdmState<'_, CTransport>>>> as ForeignOwnable>::borrow(
> +            state_ptr.cast::<c_void>(),
> +        )
> +    };

[Severity: Medium]
Should spdm_authenticate() in lib/rspdm/lib.rs be declared as pub unsafe
extern "C" fn with a # Safety documentation section, similar to
spdm_create() in the same file?

Now that spdm_authenticate() passes a non-null state_ptr to
ForeignOwnable::borrow() and locks the returned mutex, calling it with an
invalid or dangling pointer (or concurrently with spdm_destroy()) causes
undefined behavior. The SAFETY comment here relies on the caller
guaranteeing that state_ptr was returned from spdm_create() and remains
valid until spdm_destroy() is called, which requires the function to be
marked unsafe.

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