Linux Security Modules development
 help / color / mirror / Atom feed
* Re: [GIT PULL] lsm/lsm-pr-20260202
From: Paul Moore @ 2026-02-02 17:39 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: linux-security-module, linux-mm, linux-kernel
In-Reply-To: <1840fe300392f962e0bd444941c24969@paul-moore.com>

On Mon, Feb 2, 2026 at 12:37 PM Paul Moore <paul@paul-moore.com> wrote:
>
> Hi Linus,
>
> A small LSM patch to address a regression found in the v6.19-rcX releases
> where the /proc/sys/vm/mmap_min_addr tunable disappeared when
> CONFIG_SECURITY was not selected.  Long term we plan to work with the MM
> folks to get the core parts of this moved over to the MM subsystem, but
> in the meantime we need to fix this regression prior to the v6.19
> release.

I forgot to add, you'll notice a forced push on that branch, but that
was simply to add some additional reviewed-by/tested-by tags this
morning that I thought were worthwhile given we are currently at -rc8.

> --
> The following changes since commit 63804fed149a6750ffd28610c5c1c98cce6bd377:
>
>   Linux 6.19-rc7 (2026-01-25 14:11:24 -0800)
>
> are available in the Git repository at:
>
>   https://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/lsm.git
>     tags/lsm-pr-20260202
>
> for you to fetch changes up to bdde21d3e77da55121885fd2ef42bc6a15ac2f0c:
>
>   lsm: preserve /proc/sys/vm/mmap_min_addr when !CONFIG_SECURITY
>     (2026-01-29 13:56:53 -0500)
>
> ----------------------------------------------------------------
> lsm/stable-6.19 PR 20260202
> ----------------------------------------------------------------
>
> Paul Moore (1):
>       lsm: preserve /proc/sys/vm/mmap_min_addr when !CONFIG_SECURITY
>
>  security/lsm.h      |    9 ---------
>  security/lsm_init.c |    7 +------
>  security/min_addr.c |    5 ++---
>  3 files changed, 3 insertions(+), 18 deletions(-)

-- 
paul-moore.com

^ permalink raw reply

* [GIT PULL] lsm/lsm-pr-20260202
From: Paul Moore @ 2026-02-02 17:37 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: linux-security-module, linux-mm, linux-kernel

Hi Linus,

A small LSM patch to address a regression found in the v6.19-rcX releases
where the /proc/sys/vm/mmap_min_addr tunable disappeared when
CONFIG_SECURITY was not selected.  Long term we plan to work with the MM
folks to get the core parts of this moved over to the MM subsystem, but
in the meantime we need to fix this regression prior to the v6.19
release.

Paul

--
The following changes since commit 63804fed149a6750ffd28610c5c1c98cce6bd377:

  Linux 6.19-rc7 (2026-01-25 14:11:24 -0800)

are available in the Git repository at:

  https://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/lsm.git
    tags/lsm-pr-20260202

for you to fetch changes up to bdde21d3e77da55121885fd2ef42bc6a15ac2f0c:

  lsm: preserve /proc/sys/vm/mmap_min_addr when !CONFIG_SECURITY
    (2026-01-29 13:56:53 -0500)

----------------------------------------------------------------
lsm/stable-6.19 PR 20260202
----------------------------------------------------------------

Paul Moore (1):
      lsm: preserve /proc/sys/vm/mmap_min_addr when !CONFIG_SECURITY

 security/lsm.h      |    9 ---------
 security/lsm_init.c |    7 +------
 security/min_addr.c |    5 ++---
 3 files changed, 3 insertions(+), 18 deletions(-)

--
paul-moore.com

^ permalink raw reply

* Re: [PATCH] lsm: preserve /proc/sys/vm/mmap_min_addr when !CONFIG_SECURITY
From: Paul Moore @ 2026-02-02 16:19 UTC (permalink / raw)
  To: Lorenzo Stoakes; +Cc: linux-security-module, linux-mm
In-Reply-To: <38c9c9bf-3912-4e16-b15e-60890390e8dc@lucifer.local>

On Mon, Feb 2, 2026 at 5:53 AM Lorenzo Stoakes
<lorenzo.stoakes@oracle.com> wrote:
> On Thu, Jan 29, 2026 at 05:51:33PM -0500, Paul Moore wrote:
> > While reworking the LSM initialization code the
> > /proc/sys/vm/mmap_min_addr handler was inadvertently caught up in the
> > change and the procfs entry wasn't setup when CONFIG_SECURITY was not
> > selected at kernel build time.  This patch restores the previous behavior
> > and ensures that the procfs entry is setup regardless of the
> > CONFIG_SECURITY state.
> >
> > Future work will improve upon this, likely by moving the procfs handler
> > into the mm subsystem, but this patch should resolve the immediate
> > regression.
> >
> > Fixes: 4ab5efcc2829 ("lsm: consolidate all of the LSM framework initcalls")
> > Reported-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> > Signed-off-by: Paul Moore <paul@paul-moore.com>
>
> (Sorry was at fosdem from fri)
>
> LGTM and tested locally confirming it works, thanks so much for the quick
> turnaround! Feel free to add:
>
> Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> Tested-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
>
> Cheers, Lorenzo

Thanks, for the original report, testing, and extra set of eyes! added
and updated lsm/stable-6.19, I'll be sending this to Linus shortly.

-- 
paul-moore.com

^ permalink raw reply

* Re: [PATCH v13 1/4] rust: types: Add Ownable/Owned types
From: Andreas Hindborg @ 2026-02-02 13:04 UTC (permalink / raw)
  To: Gary Guo, Gary Guo, Oliver Mangold
  Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Björn Roy Baron,
	Alice Ryhl, Trevor Gross, Benno Lossin, Danilo Krummrich,
	Greg Kroah-Hartman, Dave Ertman, Ira Weiny, Leon Romanovsky,
	Rafael J. Wysocki, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Alexander Viro,
	Christian Brauner, Jan Kara, Lorenzo Stoakes, Liam R. Howlett,
	Viresh Kumar, Nishanth Menon, Stephen Boyd, Bjorn Helgaas,
	Krzysztof Wilczyński, Paul Moore, Serge Hallyn, Asahi Lina,
	rust-for-linux, linux-kernel, linux-block, dri-devel,
	linux-fsdevel, linux-mm, linux-pm, linux-pci,
	linux-security-module
In-Reply-To: <DG4H66NZ5ME0.3M9CQY1ER4Q0X@garyguo.net>

"Gary Guo" <gary@garyguo.net> writes:

> On Mon Feb 2, 2026 at 9:37 AM GMT, Andreas Hindborg wrote:
>> Gary Guo <gary@garyguo.net> writes:
>>
>>> On Mon, 17 Nov 2025 10:07:40 +0000
>>> Oliver Mangold <oliver.mangold@pm.me> wrote:
>>>
>>>> From: Asahi Lina <lina+kernel@asahilina.net>

<cut>

>>>> +impl<T: Ownable> Owned<T> {
>>>> +    /// Creates a new instance of [`Owned`].
>>>> +    ///
>>>> +    /// It takes over ownership of the underlying object.
>>>> +    ///
>>>> +    /// # Safety
>>>> +    ///
>>>> +    /// Callers must ensure that:
>>>> +    /// - `ptr` points to a valid instance of `T`.
>>>> +    /// - Ownership of the underlying `T` can be transferred to the `Self<T>` (i.e. operations
>>>> +    ///   which require ownership will be safe).
>>>> +    /// - No other Rust references to the underlying object exist. This implies that the underlying
>>>> +    ///   object is not accessed through `ptr` anymore after the function call (at least until the
>>>> +    ///   the `Self<T>` is dropped.
>>>
>>> Is this correct? If `Self<T>` is dropped then `T::release` is called so
>>> the pointer should also not be accessed further?
>>
>> I can't follow you point here. Are you saying that the requirement is
>> wrong because `T::release` will access the object by reference? If so,
>> that is part of `Owned<_>::drop`, which is explicitly mentioned in the
>> comment (until .. dropped).
>
> I meant that the `Self<T>` is dropped, the object is destroyed so it should also
> not be accessed further. Perhaps just remove the "(at least ...)" part from
> comment.

Right, got it. The "until.." is in place to allow reuse of the
allocation. There is no requirement here to drop `T` via the `release`
method when an `Owned<T>` is dropped. Implementers are free to implement
schemes that reuse the object without drop and re-init. This can be used
in object caches such as for the block request cache.

>
>>
>>>
>>>> +    /// - The C code follows the usual shared reference requirements. That is, the kernel will never
>>>> +    ///   mutate or free the underlying object (excluding interior mutability that follows the usual
>>>> +    ///   rules) while Rust owns it.
>>>
>>> The concept "interior mutability" doesn't really exist on the C side.
>>> Also, use of interior mutability (by UnsafeCell) would be incorrect if
>>> the type is implemented in the rust side (as this requires a
>>> UnsafePinned).
>>>
>>> Interior mutability means things can be mutated behind a shared
>>> reference -- however in this case, we have a mutable reference (either
>>> `Pin<&mut Self>` or `&mut Self`)!
>>>
>>> Perhaps together with the next line, they could be just phrased like
>>> this?
>>>
>>> - The underlying object must not be accessed (read or mutated) through
>>>   any pointer other than the created `Owned<T>`.
>>>   Opt-out is still possbile similar to a mutable reference (e.g. by
>>>   using p`Opaque`]).
>>>
>>> I think we should just tell the user "this is just a unique reference
>>> similar to &mut". They should be able to deduce that all the `!Unpin`
>>> that opts out from uniqueness of mutable reference applies here too.
>>
>> I agree. I would suggest updating the struct documentation:
>>
>>     @@ -108,7 +108,7 @@ pub unsafe trait Ownable {
>>         unsafe fn release(this: NonNull<Self>);
>>     }
>>
>>     -/// An owned reference to an owned `T`.
>>     +/// An mutable reference to an owned `T`.
>>     ///
>>     /// The [`Ownable`] is automatically freed or released when an instance of [`Owned`] is
>>     /// dropped.
>>
>> And then the safety requirement as
>>
>>  An `Owned<T>` is a mutable reference to the underlying object. As such,
>>  the object must not be accessed (read or mutated) through any pointer
>>  other than the created `Owned<T>`. Opt-out is still possbile similar to
>>  a mutable reference (e.g. by using [`Opaque`]).
>
> Sounds good to me.

OK.

>
>>
>>
>>>> +    /// - In case `T` implements [`Unpin`] the previous requirement is extended from shared to
>>>> +    ///   mutable reference requirements. That is, the kernel will not mutate or free the underlying
>>>> +    ///   object and is okay with it being modified by Rust code.
>>>
>>> - If `T` implements [`Unpin`], the structure must not be mutated for
>>>   the entire lifetime of `Owned<T>`.
>>
>> Would it be OK to just write "If `T: Unpin`, the ..."?
>>
>> Again, opt out is possible, right?
>>
>
> When the "mutable reference" framing above I think you can just drop this part.

Agreed.

>
>>>
>>>> +    pub unsafe fn from_raw(ptr: NonNull<T>) -> Self {
>>>
>>> This needs a (rather trivial) INVARIANT comment.
>>
>> OK.
>>
>>>
>>>> +        Self {
>>>> +            ptr,
>>>> +        }
>>>> +    }
>>>> +
>>>> +    /// Consumes the [`Owned`], returning a raw pointer.
>>>> +    ///
>>>> +    /// This function does not actually relinquish ownership of the object. After calling this
>>>
>>> Perhaps "relinquish" isn't the best word here? In my mental model
>>> this function is pretty much relinquishing ownership as `Owned<T>` no
>>> longer exists. It just doesn't release the object.
>>
>> How about this:
>>
>>
>>     /// Consumes the [`Owned`], returning a raw pointer.
>>     ///
>>     /// This function does not drop the underlying `T`. When this function returns, ownership of the
>>     /// underlying `T` is with the caller.
>
> SGTM.

OK.


Best regards,
Andreas Hindborg



^ permalink raw reply

* Re: [PATCH v13 1/4] rust: types: Add Ownable/Owned types
From: Gary Guo @ 2026-02-02 12:29 UTC (permalink / raw)
  To: Andreas Hindborg, Gary Guo, Oliver Mangold
  Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Björn Roy Baron,
	Alice Ryhl, Trevor Gross, Benno Lossin, Danilo Krummrich,
	Greg Kroah-Hartman, Dave Ertman, Ira Weiny, Leon Romanovsky,
	Rafael J. Wysocki, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Alexander Viro,
	Christian Brauner, Jan Kara, Lorenzo Stoakes, Liam R. Howlett,
	Viresh Kumar, Nishanth Menon, Stephen Boyd, Bjorn Helgaas,
	Krzysztof Wilczyński, Paul Moore, Serge Hallyn, Asahi Lina,
	rust-for-linux, linux-kernel, linux-block, dri-devel,
	linux-fsdevel, linux-mm, linux-pm, linux-pci,
	linux-security-module
In-Reply-To: <87343jqydo.fsf@t14s.mail-host-address-is-not-set>

On Mon Feb 2, 2026 at 9:37 AM GMT, Andreas Hindborg wrote:
> Gary Guo <gary@garyguo.net> writes:
>
>> On Mon, 17 Nov 2025 10:07:40 +0000
>> Oliver Mangold <oliver.mangold@pm.me> wrote:
>>
>>> From: Asahi Lina <lina+kernel@asahilina.net>
>>> 
>>> By analogy to `AlwaysRefCounted` and `ARef`, an `Ownable` type is a
>>> (typically C FFI) type that *may* be owned by Rust, but need not be. Unlike
>>> `AlwaysRefCounted`, this mechanism expects the reference to be unique
>>> within Rust, and does not allow cloning.
>>> 
>>> Conceptually, this is similar to a `KBox<T>`, except that it delegates
>>> resource management to the `T` instead of using a generic allocator.
>>> 
>>> [ om:
>>>   - Split code into separate file and `pub use` it from types.rs.
>>>   - Make from_raw() and into_raw() public.
>>>   - Remove OwnableMut, and make DerefMut dependent on Unpin instead.
>>>   - Usage example/doctest for Ownable/Owned.
>>>   - Fixes to documentation and commit message.
>>> ]
>>> 
>>> Link: https://lore.kernel.org/all/20250202-rust-page-v1-1-e3170d7fe55e@asahilina.net/
>>> Signed-off-by: Asahi Lina <lina+kernel@asahilina.net>
>>> Co-developed-by: Oliver Mangold <oliver.mangold@pm.me>
>>> Signed-off-by: Oliver Mangold <oliver.mangold@pm.me>
>>> Co-developed-by: Andreas Hindborg <a.hindborg@kernel.org>
>>> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
>>> Reviewed-by: Boqun Feng <boqun.feng@gmail.com>
>>> ---
>>>  rust/kernel/lib.rs       |   1 +
>>>  rust/kernel/owned.rs     | 195 +++++++++++++++++++++++++++++++++++++++++++++++
>>>  rust/kernel/sync/aref.rs |   5 ++
>>>  rust/kernel/types.rs     |   2 +
>>>  4 files changed, 203 insertions(+)
>>> 
>>> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
>>> index 3dd7bebe7888..e0ee04330dd0 100644
>>> --- a/rust/kernel/lib.rs
>>> +++ b/rust/kernel/lib.rs
>>> @@ -112,6 +112,7 @@
>>>  pub mod of;
>>>  #[cfg(CONFIG_PM_OPP)]
>>>  pub mod opp;
>>> +pub mod owned;
>>>  pub mod page;
>>>  #[cfg(CONFIG_PCI)]
>>>  pub mod pci;
>>> diff --git a/rust/kernel/owned.rs b/rust/kernel/owned.rs
>>> new file mode 100644
>>> index 000000000000..a2cdd2cb8a10
>>> --- /dev/null
>>> +++ b/rust/kernel/owned.rs
>>> @@ -0,0 +1,195 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +
>>> +//! Unique owned pointer types for objects with custom drop logic.
>>> +//!
>>> +//! These pointer types are useful for C-allocated objects which by API-contract
>>> +//! are owned by Rust, but need to be freed through the C API.
>>> +
>>> +use core::{
>>> +    mem::ManuallyDrop,
>>> +    ops::{Deref, DerefMut},
>>> +    pin::Pin,
>>> +    ptr::NonNull,
>>> +};
>>> +
>>> +/// Type allocated and destroyed on the C side, but owned by Rust.
>>
>> The example given in the documentation below shows a valid way of
>> defining a type that's handled on the Rust side, so I think this
>> message is somewhat inaccurate.
>>
>> Perhaps something like
>>
>> 	Types that specify their own way of performing allocation and
>> 	destruction. Typically, this trait is implemented on types from
>> 	the C side.
>
> Thanks, I'll use this.
>
>>
>> ?
>>
>>> +///
>>> +/// Implementing this trait allows types to be referenced via the [`Owned<Self>`] pointer type. This
>>> +/// is useful when it is desirable to tie the lifetime of the reference to an owned object, rather
>>> +/// than pass around a bare reference. [`Ownable`] types can define custom drop logic that is
>>> +/// executed when the owned reference [`Owned<Self>`] pointing to the object is dropped.
>>> +///
>>> +/// Note: The underlying object is not required to provide internal reference counting, because it
>>> +/// represents a unique, owned reference. If reference counting (on the Rust side) is required,
>>> +/// [`AlwaysRefCounted`](crate::types::AlwaysRefCounted) should be implemented.
>>> +///
>>> +/// # Safety
>>> +///
>>> +/// Implementers must ensure that the [`release()`](Self::release) function frees the underlying
>>> +/// object in the correct way for a valid, owned object of this type.
>>> +///
>>> +/// # Examples
>>> +///
>>> +/// A minimal example implementation of [`Ownable`] and its usage with [`Owned`] looks like this:
>>> +///
>>> +/// ```
>>> +/// # #![expect(clippy::disallowed_names)]
>>> +/// # use core::cell::Cell;
>>> +/// # use core::ptr::NonNull;
>>> +/// # use kernel::sync::global_lock;
>>> +/// # use kernel::alloc::{flags, kbox::KBox, AllocError};
>>> +/// # use kernel::types::{Owned, Ownable};
>>> +///
>>> +/// // Let's count the allocations to see if freeing works.
>>> +/// kernel::sync::global_lock! {
>>> +///     // SAFETY: we call `init()` right below, before doing anything else.
>>> +///     unsafe(uninit) static FOO_ALLOC_COUNT: Mutex<usize> = 0;
>>> +/// }
>>> +/// // SAFETY: We call `init()` only once, here.
>>> +/// unsafe { FOO_ALLOC_COUNT.init() };
>>> +///
>>> +/// struct Foo {
>>> +/// }
>>> +///
>>> +/// impl Foo {
>>> +///     fn new() -> Result<Owned<Self>, AllocError> {
>>> +///         // We are just using a `KBox` here to handle the actual allocation, as our `Foo` is
>>> +///         // not actually a C-allocated object.
>>> +///         let result = KBox::new(
>>> +///             Foo {},
>>> +///             flags::GFP_KERNEL,
>>> +///         )?;
>>> +///         let result = NonNull::new(KBox::into_raw(result))
>>> +///             .expect("Raw pointer to newly allocation KBox is null, this should never happen.");
>>> +///         // Count new allocation
>>> +///         *FOO_ALLOC_COUNT.lock() += 1;
>>> +///         // SAFETY: We just allocated the `Self`, thus it is valid and there cannot be any other
>>> +///         // Rust references. Calling `into_raw()` makes us responsible for ownership and we won't
>>> +///         // use the raw pointer anymore. Thus we can transfer ownership to the `Owned`.
>>> +///         Ok(unsafe { Owned::from_raw(result) })
>>> +///     }
>>> +/// }
>>> +///
>>> +/// // SAFETY: What out `release()` function does is safe of any valid `Self`.
>>
>> I can't parse this sentence. Is "out" supposed to be a different word?
>
> I think it should be "our".
>
>>
>>> +/// unsafe impl Ownable for Foo {
>>> +///     unsafe fn release(this: NonNull<Self>) {
>>> +///         // The `Foo` will be dropped when `KBox` goes out of scope.
>>
>> I would just write `drop(unsafe { ... })` to make drop explicit instead
>> of commenting about the implicit drop.
>
> Agree, that is easier to read.
>
>>
>>> +///         // SAFETY: The [`KBox<Self>`] is still alive. We can pass ownership to the [`KBox`], as
>>> +///         // by requirement on calling this function, the `Self` will no longer be used by the
>>> +///         // caller.
>>> +///         unsafe { KBox::from_raw(this.as_ptr()) };
>>> +///         // Count released allocation
>>> +///         *FOO_ALLOC_COUNT.lock() -= 1;
>>> +///     }
>>> +/// }
>>> +///
>>> +/// {
>>> +///    let foo = Foo::new().expect("Failed to allocate a Foo. This shouldn't happen");
>>> +///    assert!(*FOO_ALLOC_COUNT.lock() == 1);
>>> +/// }
>>> +/// // `foo` is out of scope now, so we expect no live allocations.
>>> +/// assert!(*FOO_ALLOC_COUNT.lock() == 0);
>>> +/// ```
>>> +pub unsafe trait Ownable {
>>> +    /// Releases the object.
>>> +    ///
>>> +    /// # Safety
>>> +    ///
>>> +    /// Callers must ensure that:
>>> +    /// - `this` points to a valid `Self`.
>>> +    /// - `*this` is no longer used after this call.
>>> +    unsafe fn release(this: NonNull<Self>);
>>> +}
>>> +
>>> +/// An owned reference to an owned `T`.
>>> +///
>>> +/// The [`Ownable`] is automatically freed or released when an instance of [`Owned`] is
>>> +/// dropped.
>>> +///
>>> +/// # Invariants
>>> +///
>>> +/// - The [`Owned<T>`] has exclusive access to the instance of `T`.
>>> +/// - The instance of `T` will stay alive at least as long as the [`Owned<T>`] is alive.
>>> +pub struct Owned<T: Ownable> {
>>> +    ptr: NonNull<T>,
>>> +}
>>> +
>>> +// SAFETY: It is safe to send an [`Owned<T>`] to another thread when the underlying `T` is [`Send`],
>>> +// because of the ownership invariant. Sending an [`Owned<T>`] is equivalent to sending the `T`.
>>> +unsafe impl<T: Ownable + Send> Send for Owned<T> {}
>>> +
>>> +// SAFETY: It is safe to send [`&Owned<T>`] to another thread when the underlying `T` is [`Sync`],
>>> +// because of the ownership invariant. Sending an [`&Owned<T>`] is equivalent to sending the `&T`.
>>> +unsafe impl<T: Ownable + Sync> Sync for Owned<T> {}
>>> +
>>> +impl<T: Ownable> Owned<T> {
>>> +    /// Creates a new instance of [`Owned`].
>>> +    ///
>>> +    /// It takes over ownership of the underlying object.
>>> +    ///
>>> +    /// # Safety
>>> +    ///
>>> +    /// Callers must ensure that:
>>> +    /// - `ptr` points to a valid instance of `T`.
>>> +    /// - Ownership of the underlying `T` can be transferred to the `Self<T>` (i.e. operations
>>> +    ///   which require ownership will be safe).
>>> +    /// - No other Rust references to the underlying object exist. This implies that the underlying
>>> +    ///   object is not accessed through `ptr` anymore after the function call (at least until the
>>> +    ///   the `Self<T>` is dropped.
>>
>> Is this correct? If `Self<T>` is dropped then `T::release` is called so
>> the pointer should also not be accessed further?
>
> I can't follow you point here. Are you saying that the requirement is
> wrong because `T::release` will access the object by reference? If so,
> that is part of `Owned<_>::drop`, which is explicitly mentioned in the
> comment (until .. dropped).

I meant that the `Self<T>` is dropped, the object is destroyed so it should also
not be accessed further. Perhaps just remove the "(at least ...)" part from
comment.

>
>>
>>> +    /// - The C code follows the usual shared reference requirements. That is, the kernel will never
>>> +    ///   mutate or free the underlying object (excluding interior mutability that follows the usual
>>> +    ///   rules) while Rust owns it.
>>
>> The concept "interior mutability" doesn't really exist on the C side.
>> Also, use of interior mutability (by UnsafeCell) would be incorrect if
>> the type is implemented in the rust side (as this requires a
>> UnsafePinned).
>>
>> Interior mutability means things can be mutated behind a shared
>> reference -- however in this case, we have a mutable reference (either
>> `Pin<&mut Self>` or `&mut Self`)!
>>
>> Perhaps together with the next line, they could be just phrased like
>> this?
>>
>> - The underlying object must not be accessed (read or mutated) through
>>   any pointer other than the created `Owned<T>`.
>>   Opt-out is still possbile similar to a mutable reference (e.g. by
>>   using p`Opaque`]). 
>>
>> I think we should just tell the user "this is just a unique reference
>> similar to &mut". They should be able to deduce that all the `!Unpin`
>> that opts out from uniqueness of mutable reference applies here too.
>
> I agree. I would suggest updating the struct documentation:
>
>     @@ -108,7 +108,7 @@ pub unsafe trait Ownable {
>         unsafe fn release(this: NonNull<Self>);
>     }
>
>     -/// An owned reference to an owned `T`.
>     +/// An mutable reference to an owned `T`.
>     ///
>     /// The [`Ownable`] is automatically freed or released when an instance of [`Owned`] is
>     /// dropped.
>
> And then the safety requirement as
>
>  An `Owned<T>` is a mutable reference to the underlying object. As such,
>  the object must not be accessed (read or mutated) through any pointer
>  other than the created `Owned<T>`. Opt-out is still possbile similar to
>  a mutable reference (e.g. by using [`Opaque`]).

Sounds good to me.

>
>
>>> +    /// - In case `T` implements [`Unpin`] the previous requirement is extended from shared to
>>> +    ///   mutable reference requirements. That is, the kernel will not mutate or free the underlying
>>> +    ///   object and is okay with it being modified by Rust code.
>>
>> - If `T` implements [`Unpin`], the structure must not be mutated for
>>   the entire lifetime of `Owned<T>`.
>
> Would it be OK to just write "If `T: Unpin`, the ..."?
>
> Again, opt out is possible, right?
>

When the "mutable reference" framing above I think you can just drop this part.

>>
>>> +    pub unsafe fn from_raw(ptr: NonNull<T>) -> Self {
>>
>> This needs a (rather trivial) INVARIANT comment.
>
> OK.
>
>>
>>> +        Self {
>>> +            ptr,
>>> +        }
>>> +    }
>>> +
>>> +    /// Consumes the [`Owned`], returning a raw pointer.
>>> +    ///
>>> +    /// This function does not actually relinquish ownership of the object. After calling this
>>
>> Perhaps "relinquish" isn't the best word here? In my mental model
>> this function is pretty much relinquishing ownership as `Owned<T>` no
>> longer exists. It just doesn't release the object.
>
> How about this:
>
>
>     /// Consumes the [`Owned`], returning a raw pointer.
>     ///
>     /// This function does not drop the underlying `T`. When this function returns, ownership of the
>     /// underlying `T` is with the caller.

SGTM.

>
>
> Thanks for the comments!

Best,
Gary

>
>
> Best regards,
> Andreas Hindborg


^ permalink raw reply

* Re: [PATCH] lsm: preserve /proc/sys/vm/mmap_min_addr when !CONFIG_SECURITY
From: Lorenzo Stoakes @ 2026-02-02 10:53 UTC (permalink / raw)
  To: Paul Moore; +Cc: linux-security-module, linux-mm
In-Reply-To: <20260129225132.420484-2-paul@paul-moore.com>

On Thu, Jan 29, 2026 at 05:51:33PM -0500, Paul Moore wrote:
> While reworking the LSM initialization code the
> /proc/sys/vm/mmap_min_addr handler was inadvertently caught up in the
> change and the procfs entry wasn't setup when CONFIG_SECURITY was not
> selected at kernel build time.  This patch restores the previous behavior
> and ensures that the procfs entry is setup regardless of the
> CONFIG_SECURITY state.
>
> Future work will improve upon this, likely by moving the procfs handler
> into the mm subsystem, but this patch should resolve the immediate
> regression.
>
> Fixes: 4ab5efcc2829 ("lsm: consolidate all of the LSM framework initcalls")
> Reported-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> Signed-off-by: Paul Moore <paul@paul-moore.com>

(Sorry was at fosdem from fri)

LGTM and tested locally confirming it works, thanks so much for the quick
turnaround! Feel free to add:

Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
Tested-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>

Cheers, Lorenzo

> ---
>  security/lsm.h      | 9 ---------
>  security/lsm_init.c | 7 +------
>  security/min_addr.c | 5 ++---
>  3 files changed, 3 insertions(+), 18 deletions(-)
>
> diff --git a/security/lsm.h b/security/lsm.h
> index 81aadbc61685..db77cc83e158 100644
> --- a/security/lsm.h
> +++ b/security/lsm.h
> @@ -37,15 +37,6 @@ int lsm_task_alloc(struct task_struct *task);
>
>  /* LSM framework initializers */
>
> -#ifdef CONFIG_MMU
> -int min_addr_init(void);
> -#else
> -static inline int min_addr_init(void)
> -{
> -	return 0;
> -}
> -#endif /* CONFIG_MMU */
> -
>  #ifdef CONFIG_SECURITYFS
>  int securityfs_init(void);
>  #else
> diff --git a/security/lsm_init.c b/security/lsm_init.c
> index 05bd52e6b1f2..573e2a7250c4 100644
> --- a/security/lsm_init.c
> +++ b/security/lsm_init.c
> @@ -489,12 +489,7 @@ int __init security_init(void)
>   */
>  static int __init security_initcall_pure(void)
>  {
> -	int rc_adr, rc_lsm;
> -
> -	rc_adr = min_addr_init();
> -	rc_lsm = lsm_initcall(pure);
> -
> -	return (rc_adr ? rc_adr : rc_lsm);
> +	return lsm_initcall(pure);
>  }
>  pure_initcall(security_initcall_pure);
>
> diff --git a/security/min_addr.c b/security/min_addr.c
> index 0fde5ec9abc8..56e4f9d25929 100644
> --- a/security/min_addr.c
> +++ b/security/min_addr.c
> @@ -5,8 +5,6 @@
>  #include <linux/sysctl.h>
>  #include <linux/minmax.h>
>
> -#include "lsm.h"
> -
>  /* amount of vm to protect from userspace access by both DAC and the LSM*/
>  unsigned long mmap_min_addr;
>  /* amount of vm to protect from userspace using CAP_SYS_RAWIO (DAC) */
> @@ -54,10 +52,11 @@ static const struct ctl_table min_addr_sysctl_table[] = {
>  	},
>  };
>
> -int __init min_addr_init(void)
> +static int __init mmap_min_addr_init(void)
>  {
>  	register_sysctl_init("vm", min_addr_sysctl_table);
>  	update_mmap_min_addr();
>
>  	return 0;
>  }
> +pure_initcall(mmap_min_addr_init);
> --
> 2.52.0
>

^ permalink raw reply

* Re: [PATCH v13 3/4] rust: Add missing SAFETY documentation for `ARef` example
From: Andreas Hindborg @ 2026-02-02  9:52 UTC (permalink / raw)
  To: Daniel Almeida, Oliver Mangold
  Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Alice Ryhl, Trevor Gross, Benno Lossin,
	Danilo Krummrich, Greg Kroah-Hartman, Dave Ertman, Ira Weiny,
	Leon Romanovsky, Rafael J. Wysocki, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
	Alexander Viro, Christian Brauner, Jan Kara, Lorenzo Stoakes,
	Liam R. Howlett, Viresh Kumar, Nishanth Menon, Stephen Boyd,
	Bjorn Helgaas, Krzysztof Wilczyński, Paul Moore,
	Serge Hallyn, Asahi Lina, rust-for-linux, linux-kernel,
	linux-block, dri-devel, linux-fsdevel, linux-mm, linux-pm,
	linux-pci, linux-security-module
In-Reply-To: <06C410F4-8534-43B4-8DE1-039F70B26E5A@collabora.com>

Daniel Almeida <daniel.almeida@collabora.com> writes:

>> On 17 Nov 2025, at 07:08, Oliver Mangold <oliver.mangold@pm.me> wrote:
>> 
>> SAFETY comment in rustdoc example was just 'TODO'. Fixed.
>> 
>> Signed-off-by: Oliver Mangold <oliver.mangold@pm.me>
>> Co-developed-by: Andreas Hindborg <a.hindborg@kernel.org>
>> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
>> ---
>> rust/kernel/sync/aref.rs | 10 ++++++----
>> 1 file changed, 6 insertions(+), 4 deletions(-)
>> 
>> diff --git a/rust/kernel/sync/aref.rs b/rust/kernel/sync/aref.rs
>> index 4226119d5ac9..937dcf6ed5de 100644
>> --- a/rust/kernel/sync/aref.rs
>> +++ b/rust/kernel/sync/aref.rs
>> @@ -129,12 +129,14 @@ pub unsafe fn from_raw(ptr: NonNull<T>) -> Self {
>>     /// # Examples
>>     ///
>>     /// ```
>> -    /// use core::ptr::NonNull;
>> -    /// use kernel::sync::aref::{ARef, RefCounted};
>> +    /// # use core::ptr::NonNull;
>> +    /// # use kernel::sync::aref::{ARef, RefCounted};
>>     ///
>>     /// struct Empty {}
>>     ///
>> -    /// # // SAFETY: TODO.
>> +    /// // SAFETY: The `RefCounted` implementation for `Empty` does not count references and
>> +    /// // never frees the underlying object. Thus we can act as having a refcount on the object
>
> nit: perhaps saying “an increment on the refcount” is clearer?

OK


    /// // SAFETY: The `RefCounted` implementation for `Empty` does not count references and never
    /// // frees the underlying object. Thus we can act as owning an increment on the refcount for
    /// // the object that we pass to the newly created `ARef`.

>
>> +    /// // that we pass to the newly created `ARef`.
>>     /// unsafe impl RefCounted for Empty {
>>     ///     fn inc_ref(&self) {}
>>     ///     unsafe fn dec_ref(_obj: NonNull<Self>) {}
>> @@ -142,7 +144,7 @@ pub unsafe fn from_raw(ptr: NonNull<T>) -> Self {
>>     ///
>>     /// let mut data = Empty {};
>>     /// let ptr = NonNull::<Empty>::new(&mut data).unwrap();
>> -    /// # // SAFETY: TODO.
>> +    /// // SAFETY: We keep `data` around longer than the `ARef`.
>>     /// let data_ref: ARef<Empty> = unsafe { ARef::from_raw(ptr) };
>>     /// let raw_ptr: NonNull<Empty> = ARef::into_raw(data_ref);
>>     ///
>> 
>> -- 
>> 2.51.2
>> 
>> 
>> 
>
> Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>

Thanks.


Best regards,
Andreas Hindborg



^ permalink raw reply

* Re: [PATCH v13 1/4] rust: types: Add Ownable/Owned types
From: Andreas Hindborg @ 2026-02-02  9:14 UTC (permalink / raw)
  To: Daniel Almeida, Oliver Mangold
  Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Alice Ryhl, Trevor Gross, Benno Lossin,
	Danilo Krummrich, Greg Kroah-Hartman, Dave Ertman, Ira Weiny,
	Leon Romanovsky, Rafael J. Wysocki, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
	Alexander Viro, Christian Brauner, Jan Kara, Lorenzo Stoakes,
	Liam R. Howlett, Viresh Kumar, Nishanth Menon, Stephen Boyd,
	Bjorn Helgaas, Krzysztof Wilczyński, Paul Moore,
	Serge Hallyn, Asahi Lina, rust-for-linux, linux-kernel,
	linux-block, dri-devel, linux-fsdevel, linux-mm, linux-pm,
	linux-pci, linux-security-module
In-Reply-To: <C95B13F7-B3F5-4508-A862-EAD22EF56FE2@collabora.com>

Hi Daniel,

I'll send the next iteration of this series.

Daniel Almeida <daniel.almeida@collabora.com> writes:

> Hi Oliver,
>
>> On 17 Nov 2025, at 07:07, Oliver Mangold <oliver.mangold@pm.me> wrote:
>> 
>> From: Asahi Lina <lina+kernel@asahilina.net>
>> 
>> By analogy to `AlwaysRefCounted` and `ARef`, an `Ownable` type is a
>> (typically C FFI) type that *may* be owned by Rust, but need not be. Unlike
>> `AlwaysRefCounted`, this mechanism expects the reference to be unique
>> within Rust, and does not allow cloning.
>> 
>> Conceptually, this is similar to a `KBox<T>`, except that it delegates
>> resource management to the `T` instead of using a generic allocator.
>> 
>> [ om:
>>  - Split code into separate file and `pub use` it from types.rs.
>>  - Make from_raw() and into_raw() public.
>>  - Remove OwnableMut, and make DerefMut dependent on Unpin instead.
>>  - Usage example/doctest for Ownable/Owned.
>>  - Fixes to documentation and commit message.
>> ]
>> 
>> Link: https://lore.kernel.org/all/20250202-rust-page-v1-1-e3170d7fe55e@asahilina.net/
>> Signed-off-by: Asahi Lina <lina+kernel@asahilina.net>
>> Co-developed-by: Oliver Mangold <oliver.mangold@pm.me>
>> Signed-off-by: Oliver Mangold <oliver.mangold@pm.me>
>> Co-developed-by: Andreas Hindborg <a.hindborg@kernel.org>
>> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
>> Reviewed-by: Boqun Feng <boqun.feng@gmail.com>
>> ---
>> rust/kernel/lib.rs       |   1 +
>> rust/kernel/owned.rs     | 195 +++++++++++++++++++++++++++++++++++++++++++++++
>> rust/kernel/sync/aref.rs |   5 ++
>> rust/kernel/types.rs     |   2 +
>> 4 files changed, 203 insertions(+)
>> 
>> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
>> index 3dd7bebe7888..e0ee04330dd0 100644
>> --- a/rust/kernel/lib.rs
>> +++ b/rust/kernel/lib.rs
>> @@ -112,6 +112,7 @@
>> pub mod of;
>> #[cfg(CONFIG_PM_OPP)]
>> pub mod opp;
>> +pub mod owned;
>> pub mod page;
>> #[cfg(CONFIG_PCI)]
>> pub mod pci;
>> diff --git a/rust/kernel/owned.rs b/rust/kernel/owned.rs
>> new file mode 100644
>> index 000000000000..a2cdd2cb8a10
>> --- /dev/null
>> +++ b/rust/kernel/owned.rs
>> @@ -0,0 +1,195 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +
>> +//! Unique owned pointer types for objects with custom drop logic.
>> +//!
>> +//! These pointer types are useful for C-allocated objects which by API-contract
>> +//! are owned by Rust, but need to be freed through the C API.
>> +
>> +use core::{
>> +    mem::ManuallyDrop,
>> +    ops::{Deref, DerefMut},
>> +    pin::Pin,
>> +    ptr::NonNull,
>> +};
>> +
>> +/// Type allocated and destroyed on the C side, but owned by Rust.
>> +///
>> +/// Implementing this trait allows types to be referenced via the [`Owned<Self>`] pointer type. This
>> +/// is useful when it is desirable to tie the lifetime of the reference to an owned object, rather
>> +/// than pass around a bare reference. [`Ownable`] types can define custom drop logic that is
>> +/// executed when the owned reference [`Owned<Self>`] pointing to the object is dropped.
>> +///
>> +/// Note: The underlying object is not required to provide internal reference counting, because it
>> +/// represents a unique, owned reference. If reference counting (on the Rust side) is required,
>> +/// [`AlwaysRefCounted`](crate::types::AlwaysRefCounted) should be implemented.
>> +///
>> +/// # Safety
>> +///
>> +/// Implementers must ensure that the [`release()`](Self::release) function frees the underlying
>> +/// object in the correct way for a valid, owned object of this type.
>> +///
>> +/// # Examples
>> +///
>> +/// A minimal example implementation of [`Ownable`] and its usage with [`Owned`] looks like this:
>> +///
>> +/// ```
>> +/// # #![expect(clippy::disallowed_names)]
>> +/// # use core::cell::Cell;
>> +/// # use core::ptr::NonNull;
>> +/// # use kernel::sync::global_lock;
>> +/// # use kernel::alloc::{flags, kbox::KBox, AllocError};
>> +/// # use kernel::types::{Owned, Ownable};
>> +///
>> +/// // Let's count the allocations to see if freeing works.
>> +/// kernel::sync::global_lock! {
>> +///     // SAFETY: we call `init()` right below, before doing anything else.
>> +///     unsafe(uninit) static FOO_ALLOC_COUNT: Mutex<usize> = 0;
>> +/// }
>> +/// // SAFETY: We call `init()` only once, here.
>> +/// unsafe { FOO_ALLOC_COUNT.init() };
>> +///
>> +/// struct Foo {
>> +/// }
>
> nit: this can be simply:
>
> struct Foo;

Got it.

>
>> +///
>> +/// impl Foo {
>> +///     fn new() -> Result<Owned<Self>, AllocError> {
>> +///         // We are just using a `KBox` here to handle the actual allocation, as our `Foo` is
>> +///         // not actually a C-allocated object.
>> +///         let result = KBox::new(
>> +///             Foo {},
>> +///             flags::GFP_KERNEL,
>> +///         )?;
>> +///         let result = NonNull::new(KBox::into_raw(result))
>> +///             .expect("Raw pointer to newly allocation KBox is null, this should never happen.");
>> +///         // Count new allocation
>> +///         *FOO_ALLOC_COUNT.lock() += 1;
>> +///         // SAFETY: We just allocated the `Self`, thus it is valid and there cannot be any other
>> +///         // Rust references. Calling `into_raw()` makes us responsible for ownership and we won't
>> +///         // use the raw pointer anymore. Thus we can transfer ownership to the `Owned`.
>> +///         Ok(unsafe { Owned::from_raw(result) })
>> +///     }
>> +/// }
>> +///
>> +/// // SAFETY: What out `release()` function does is safe of any valid `Self`.
>> +/// unsafe impl Ownable for Foo {
>> +///     unsafe fn release(this: NonNull<Self>) {
>> +///         // The `Foo` will be dropped when `KBox` goes out of scope.
>> +///         // SAFETY: The [`KBox<Self>`] is still alive. We can pass ownership to the [`KBox`], as
>> +///         // by requirement on calling this function, the `Self` will no longer be used by the
>> +///         // caller.
>> +///         unsafe { KBox::from_raw(this.as_ptr()) };
>> +///         // Count released allocation
>> +///         *FOO_ALLOC_COUNT.lock() -= 1;
>> +///     }
>> +/// }
>> +///
>> +/// {
>> +///    let foo = Foo::new().expect("Failed to allocate a Foo. This shouldn't happen");
>> +///    assert!(*FOO_ALLOC_COUNT.lock() == 1);
>> +/// }
>> +/// // `foo` is out of scope now, so we expect no live allocations.
>> +/// assert!(*FOO_ALLOC_COUNT.lock() == 0);
>> +/// ```
>> +pub unsafe trait Ownable {
>> +    /// Releases the object.
>> +    ///
>> +    /// # Safety
>> +    ///
>> +    /// Callers must ensure that:
>> +    /// - `this` points to a valid `Self`.
>> +    /// - `*this` is no longer used after this call.
>> +    unsafe fn release(this: NonNull<Self>);
>> +}
>> +
>> +/// An owned reference to an owned `T`.
>> +///
>> +/// The [`Ownable`] is automatically freed or released when an instance of [`Owned`] is
>> +/// dropped.
>> +///
>> +/// # Invariants
>> +///
>> +/// - The [`Owned<T>`] has exclusive access to the instance of `T`.
>> +/// - The instance of `T` will stay alive at least as long as the [`Owned<T>`] is alive.
>> +pub struct Owned<T: Ownable> {
>> +    ptr: NonNull<T>,
>> +}
>> +
>> +// SAFETY: It is safe to send an [`Owned<T>`] to another thread when the underlying `T` is [`Send`],
>> +// because of the ownership invariant. Sending an [`Owned<T>`] is equivalent to sending the `T`.
>> +unsafe impl<T: Ownable + Send> Send for Owned<T> {}
>> +
>> +// SAFETY: It is safe to send [`&Owned<T>`] to another thread when the underlying `T` is [`Sync`],
>> +// because of the ownership invariant. Sending an [`&Owned<T>`] is equivalent to sending the `&T`.
>> +unsafe impl<T: Ownable + Sync> Sync for Owned<T> {}
>> +
>> +impl<T: Ownable> Owned<T> {
>
> Can you make sure that impl Owned<T> follows the struct declaration?
>
> IOW: please move the Send and Sync impls to be after the impl above.

I don't really see the point, but moving them is no problem, so let's do that.

>
>> +    /// Creates a new instance of [`Owned`].
>> +    ///
>> +    /// It takes over ownership of the underlying object.
>> +    ///
>> +    /// # Safety
>> +    ///
>> +    /// Callers must ensure that:
>> +    /// - `ptr` points to a valid instance of `T`.
>> +    /// - Ownership of the underlying `T` can be transferred to the `Self<T>` (i.e. operations
>> +    ///   which require ownership will be safe).
>> +    /// - No other Rust references to the underlying object exist. This implies that the underlying
>> +    ///   object is not accessed through `ptr` anymore after the function call (at least until the
>> +    ///   the `Self<T>` is dropped.
>
> It looks like this can be written more succinctly as:
>
> "This implies that the underlying object is not accessed through `ptr` anymore until `Self<T>` is dropped."

I'll rephrase with that text.

>
>> +    /// - The C code follows the usual shared reference requirements. That is, the kernel will never
>> +    ///   mutate or free the underlying object (excluding interior mutability that follows the usual
>> +    ///   rules) while Rust owns it.
>> +    /// - In case `T` implements [`Unpin`] the previous requirement is extended from shared to
>> +    ///   mutable reference requirements. That is, the kernel will not mutate or free the underlying
>> +    ///   object and is okay with it being modified by Rust code.
>> +    pub unsafe fn from_raw(ptr: NonNull<T>) -> Self {
>> +        Self {
>> +            ptr,
>> +        }
>> +    }
>> +
>> +    /// Consumes the [`Owned`], returning a raw pointer.
>> +    ///
>> +    /// This function does not actually relinquish ownership of the object. After calling this
>> +    /// function, the caller is responsible for ownership previously managed
>> +    /// by the [`Owned`].
>> +    pub fn into_raw(me: Self) -> NonNull<T> {
>> +        ManuallyDrop::new(me).ptr
>> +    }
>> +
>> +    /// Get a pinned mutable reference to the data owned by this `Owned<T>`.
>> +    pub fn get_pin_mut(&mut self) -> Pin<&mut T> {
>> +        // SAFETY: The type invariants guarantee that the object is valid, and that we can safely
>> +        // return a mutable reference to it.
>> +        let unpinned = unsafe { self.ptr.as_mut() };
>> +
>> +        // SAFETY: We never hand out unpinned mutable references to the data in
>> +        // `Self`, unless the contained type is `Unpin`.
>> +        unsafe { Pin::new_unchecked(unpinned) }
>> +    }
>> +}
>> +
>> +impl<T: Ownable> Deref for Owned<T> {
>> +    type Target = T;
>> +
>> +    fn deref(&self) -> &Self::Target {
>> +        // SAFETY: The type invariants guarantee that the object is valid.
>> +        unsafe { self.ptr.as_ref() }
>> +    }
>> +}
>> +
>> +impl<T: Ownable + Unpin> DerefMut for Owned<T> {
>> +    fn deref_mut(&mut self) -> &mut Self::Target {
>> +        // SAFETY: The type invariants guarantee that the object is valid, and that we can safely
>> +        // return a mutable reference to it.
>> +        unsafe { self.ptr.as_mut() }
>> +    }
>> +}
>> +
>> +impl<T: Ownable> Drop for Owned<T> {
>> +    fn drop(&mut self) {
>> +        // SAFETY: The type invariants guarantee that the `Owned` owns the object we're about to
>> +        // release.
>> +        unsafe { T::release(self.ptr) };
>> +    }
>> +}
>> diff --git a/rust/kernel/sync/aref.rs b/rust/kernel/sync/aref.rs
>> index 0d24a0432015..e175aefe8615 100644
>> --- a/rust/kernel/sync/aref.rs
>> +++ b/rust/kernel/sync/aref.rs
>> @@ -29,6 +29,11 @@
>> /// Rust code, the recommendation is to use [`Arc`](crate::sync::Arc) to create reference-counted
>> /// instances of a type.
>> ///
>> +/// Note: Implementing this trait allows types to be wrapped in an [`ARef<Self>`]. It requires an
>> +/// internal reference count and provides only shared references. If unique references are required
>> +/// [`Ownable`](crate::types::Ownable) should be implemented which allows types to be wrapped in an
>> +/// [`Owned<Self>`](crate::types::Owned).
>> +///
>> /// # Safety
>> ///
>> /// Implementers must ensure that increments to the reference count keep the object alive in memory
>> diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
>> index dc0a02f5c3cf..7bc07c38cd6c 100644
>> --- a/rust/kernel/types.rs
>> +++ b/rust/kernel/types.rs
>> @@ -11,6 +11,8 @@
>> };
>> use pin_init::{PinInit, Wrapper, Zeroable};
>> 
>> +pub use crate::owned::{Ownable, Owned};
>> +
>> pub use crate::sync::aref::{ARef, AlwaysRefCounted};
>> 
>> /// Used to transfer ownership to and from foreign (non-Rust) languages.
>> 
>> -- 
>> 2.51.2
>> 
>> 
>> 
>
> With the changes above,
>
> Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>

Thanks!


^ permalink raw reply

* Re: [PATCH v13 2/4] rust: `AlwaysRefCounted` is renamed to `RefCounted`.
From: Andreas Hindborg @ 2026-02-02  9:46 UTC (permalink / raw)
  To: Daniel Almeida, Oliver Mangold
  Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Alice Ryhl, Trevor Gross, Benno Lossin,
	Danilo Krummrich, Greg Kroah-Hartman, Dave Ertman, Ira Weiny,
	Leon Romanovsky, Rafael J. Wysocki, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
	Alexander Viro, Christian Brauner, Jan Kara, Lorenzo Stoakes,
	Liam R. Howlett, Viresh Kumar, Nishanth Menon, Stephen Boyd,
	Bjorn Helgaas, Krzysztof Wilczyński, Paul Moore,
	Serge Hallyn, Asahi Lina, rust-for-linux, linux-kernel,
	linux-block, dri-devel, linux-fsdevel, linux-mm, linux-pm,
	linux-pci, linux-security-module
In-Reply-To: <D2C11077-2001-4E45-8039-C999E6CA5606@collabora.com>

Daniel Almeida <daniel.almeida@collabora.com> writes:

>> On 17 Nov 2025, at 07:07, Oliver Mangold <oliver.mangold@pm.me> wrote:
>> 
>> `AlwaysRefCounted` will become a marker trait to indicate that it is
>> allowed to obtain an `ARef<T>` from a `&T`, which cannot be allowed for
>> types which are also Ownable.
>> 
>> Signed-off-by: Oliver Mangold <oliver.mangold@pm.me>
>> Co-developed-by: Andreas Hindborg <a.hindborg@kernel.org>
>> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
>> Suggested-by: Alice Ryhl <aliceryhl@google.com>
>> ---
>> rust/kernel/auxiliary.rs        |  7 +++++-
>> rust/kernel/block/mq/request.rs | 15 +++++++------
>> rust/kernel/cred.rs             | 13 ++++++++++--
>> rust/kernel/device.rs           | 13 ++++++++----
>> rust/kernel/device/property.rs  |  7 +++++-
>> rust/kernel/drm/device.rs       | 10 ++++++---
>> rust/kernel/drm/gem/mod.rs      | 10 ++++++---
>> rust/kernel/fs/file.rs          | 16 ++++++++++----
>> rust/kernel/mm.rs               | 15 +++++++++----
>> rust/kernel/mm/mmput_async.rs   |  9 ++++++--
>> rust/kernel/opp.rs              | 10 ++++++---
>> rust/kernel/owned.rs            |  2 +-
>> rust/kernel/pci.rs              | 10 ++++++---
>> rust/kernel/pid_namespace.rs    | 12 +++++++++--
>> rust/kernel/platform.rs         |  7 +++++-
>> rust/kernel/sync/aref.rs        | 47 ++++++++++++++++++++++++++---------------
>> rust/kernel/task.rs             | 10 ++++++---
>> rust/kernel/types.rs            |  2 +-
>> 18 files changed, 154 insertions(+), 61 deletions(-)
>> 
>> diff --git a/rust/kernel/auxiliary.rs b/rust/kernel/auxiliary.rs
>> index 7a3b0b9c418e..7f5b16053c11 100644
>> --- a/rust/kernel/auxiliary.rs
>> +++ b/rust/kernel/auxiliary.rs
>> @@ -10,6 +10,7 @@
>>     driver,
>>     error::{from_result, to_result, Result},
>>     prelude::*,
>> +    sync::aref::{AlwaysRefCounted, RefCounted},
>>     types::Opaque,
>>     ThisModule,
>> };
>> @@ -239,7 +240,7 @@ extern "C" fn release(dev: *mut bindings::device) {
>> kernel::impl_device_context_into_aref!(Device);
>> 
>> // SAFETY: Instances of `Device` are always reference-counted.
>> -unsafe impl crate::sync::aref::AlwaysRefCounted for Device {
>> +unsafe impl RefCounted for Device {
>>     fn inc_ref(&self) {
>>         // SAFETY: The existence of a shared reference guarantees that the refcount is non-zero.
>>         unsafe { bindings::get_device(self.as_ref().as_raw()) };
>> @@ -258,6 +259,10 @@ unsafe fn dec_ref(obj: NonNull<Self>) {
>>     }
>> }
>> 
>> +// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef<Device>` from a
>> +// `&Device`.
>> +unsafe impl AlwaysRefCounted for Device {}
>> +
>> impl<Ctx: device::DeviceContext> AsRef<device::Device<Ctx>> for Device<Ctx> {
>>     fn as_ref(&self) -> &device::Device<Ctx> {
>>         // SAFETY: By the type invariant of `Self`, `self.as_raw()` is a pointer to a valid
>> diff --git a/rust/kernel/block/mq/request.rs b/rust/kernel/block/mq/request.rs
>> index c5f1f6b1ccfb..b6165f96b4ce 100644
>> --- a/rust/kernel/block/mq/request.rs
>> +++ b/rust/kernel/block/mq/request.rs
>> @@ -8,7 +8,7 @@
>>     bindings,
>>     block::mq::Operations,
>>     error::Result,
>> -    sync::{atomic::Relaxed, Refcount},
>> +    sync::{aref::RefCounted, atomic::Relaxed, Refcount},
>>     types::{ARef, AlwaysRefCounted, Opaque},
>> };
>> use core::{marker::PhantomData, ptr::NonNull};
>> @@ -225,11 +225,10 @@ unsafe impl<T: Operations> Send for Request<T> {}
>> // mutate `self` are internally synchronized`
>> unsafe impl<T: Operations> Sync for Request<T> {}
>> 
>> -// SAFETY: All instances of `Request<T>` are reference counted. This
>> -// implementation of `AlwaysRefCounted` ensure that increments to the ref count
>> -// keeps the object alive in memory at least until a matching reference count
>> -// decrement is executed.
>> -unsafe impl<T: Operations> AlwaysRefCounted for Request<T> {
>> +// SAFETY: All instances of `Request<T>` are reference counted. This implementation of `RefCounted`
>> +// ensure that increments to the ref count keeps the object alive in memory at least until a
>> +// matching reference count decrement is executed.
>> +unsafe impl<T: Operations> RefCounted for Request<T> {
>>     fn inc_ref(&self) {
>>         self.wrapper_ref().refcount().inc();
>>     }
>> @@ -251,3 +250,7 @@ unsafe fn dec_ref(obj: core::ptr::NonNull<Self>) {
>>         }
>>     }
>> }
>> +
>> +// SAFETY: We currently do not implement `Ownable`, thus it is okay to obtain an `ARef<Request>`
>> +// from a `&Request` (but this will change in the future).
>> +unsafe impl<T: Operations> AlwaysRefCounted for Request<T> {}
>> diff --git a/rust/kernel/cred.rs b/rust/kernel/cred.rs
>> index ffa156b9df37..20ef0144094b 100644
>> --- a/rust/kernel/cred.rs
>> +++ b/rust/kernel/cred.rs
>> @@ -8,7 +8,12 @@
>> //!
>> //! Reference: <https://www.kernel.org/doc/html/latest/security/credentials.html>
>> 
>> -use crate::{bindings, sync::aref::AlwaysRefCounted, task::Kuid, types::Opaque};
>> +use crate::{
>> +    bindings,
>> +    sync::aref::RefCounted,
>> +    task::Kuid,
>> +    types::{AlwaysRefCounted, Opaque},
>> +};
>> 
>> /// Wraps the kernel's `struct cred`.
>> ///
>> @@ -76,7 +81,7 @@ pub fn euid(&self) -> Kuid {
>> }
>> 
>> // SAFETY: The type invariants guarantee that `Credential` is always ref-counted.
>> -unsafe impl AlwaysRefCounted for Credential {
>> +unsafe impl RefCounted for Credential {
>>     #[inline]
>>     fn inc_ref(&self) {
>>         // SAFETY: The existence of a shared reference means that the refcount is nonzero.
>> @@ -90,3 +95,7 @@ unsafe fn dec_ref(obj: core::ptr::NonNull<Credential>) {
>>         unsafe { bindings::put_cred(obj.cast().as_ptr()) };
>>     }
>> }
>> +
>> +// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef<Credential>` from a
>> +// `&Credential`.
>> +unsafe impl AlwaysRefCounted for Credential {}
>> diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
>> index a849b7dde2fd..a69ee32997c1 100644
>> --- a/rust/kernel/device.rs
>> +++ b/rust/kernel/device.rs
>> @@ -5,9 +5,10 @@
>> //! C header: [`include/linux/device.h`](srctree/include/linux/device.h)
>> 
>> use crate::{
>> -    bindings, fmt,
>> -    sync::aref::ARef,
>> -    types::{ForeignOwnable, Opaque},
>> +    bindings,
>> +    fmt,
>> +    sync::aref::RefCounted,
>> +    types::{ARef, AlwaysRefCounted, ForeignOwnable, Opaque},
>> };
>> use core::{marker::PhantomData, ptr};
>> 
>> @@ -407,7 +408,7 @@ pub fn fwnode(&self) -> Option<&property::FwNode> {
>> kernel::impl_device_context_into_aref!(Device);
>> 
>> // SAFETY: Instances of `Device` are always reference-counted.
>> -unsafe impl crate::sync::aref::AlwaysRefCounted for Device {
>> +unsafe impl RefCounted for Device {
>>     fn inc_ref(&self) {
>>         // SAFETY: The existence of a shared reference guarantees that the refcount is non-zero.
>>         unsafe { bindings::get_device(self.as_raw()) };
>> @@ -419,6 +420,10 @@ unsafe fn dec_ref(obj: ptr::NonNull<Self>) {
>>     }
>> }
>> 
>> +// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef<Device>` from a
>> +// `&Device`.
>> +unsafe impl AlwaysRefCounted for Device {}
>> +
>> // SAFETY: As by the type invariant `Device` can be sent to any thread.
>> unsafe impl Send for Device {}
>> 
>> diff --git a/rust/kernel/device/property.rs b/rust/kernel/device/property.rs
>> index 3a332a8c53a9..a8bb824ad0ec 100644
>> --- a/rust/kernel/device/property.rs
>> +++ b/rust/kernel/device/property.rs
>> @@ -14,6 +14,7 @@
>>     fmt,
>>     prelude::*,
>>     str::{CStr, CString},
>> +    sync::aref::{AlwaysRefCounted, RefCounted},
>>     types::{ARef, Opaque},
>> };
>> 
>> @@ -359,7 +360,7 @@ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
>> }
>> 
>> // SAFETY: Instances of `FwNode` are always reference-counted.
>> -unsafe impl crate::types::AlwaysRefCounted for FwNode {
>> +unsafe impl RefCounted for FwNode {
>>     fn inc_ref(&self) {
>>         // SAFETY: The existence of a shared reference guarantees that the
>>         // refcount is non-zero.
>> @@ -373,6 +374,10 @@ unsafe fn dec_ref(obj: ptr::NonNull<Self>) {
>>     }
>> }
>> 
>> +// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef<FwNode>` from a
>> +// `&FwNode`.
>> +unsafe impl AlwaysRefCounted for FwNode {}
>> +
>> enum Node<'a> {
>>     Borrowed(&'a FwNode),
>>     Owned(ARef<FwNode>),
>> diff --git a/rust/kernel/drm/device.rs b/rust/kernel/drm/device.rs
>> index 3ce8f62a0056..38ce7f389ed0 100644
>> --- a/rust/kernel/drm/device.rs
>> +++ b/rust/kernel/drm/device.rs
>> @@ -11,8 +11,8 @@
>>     error::from_err_ptr,
>>     error::Result,
>>     prelude::*,
>> -    sync::aref::{ARef, AlwaysRefCounted},
>> -    types::Opaque,
>> +    sync::aref::{AlwaysRefCounted, RefCounted},
>> +    types::{ARef, Opaque},
>> };
>> use core::{alloc::Layout, mem, ops::Deref, ptr, ptr::NonNull};
>> 
>> @@ -198,7 +198,7 @@ fn deref(&self) -> &Self::Target {
>> 
>> // SAFETY: DRM device objects are always reference counted and the get/put functions
>> // satisfy the requirements.
>> -unsafe impl<T: drm::Driver> AlwaysRefCounted for Device<T> {
>> +unsafe impl<T: drm::Driver> RefCounted for Device<T> {
>>     fn inc_ref(&self) {
>>         // SAFETY: The existence of a shared reference guarantees that the refcount is non-zero.
>>         unsafe { bindings::drm_dev_get(self.as_raw()) };
>> @@ -213,6 +213,10 @@ unsafe fn dec_ref(obj: NonNull<Self>) {
>>     }
>> }
>> 
>> +// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef<Device>` from a
>> +// `&Device`.
>> +unsafe impl<T: drm::Driver> AlwaysRefCounted for Device<T> {}
>> +
>> impl<T: drm::Driver> AsRef<device::Device> for Device<T> {
>>     fn as_ref(&self) -> &device::Device {
>>         // SAFETY: `bindings::drm_device::dev` is valid as long as the DRM device itself is valid,
>> diff --git a/rust/kernel/drm/gem/mod.rs b/rust/kernel/drm/gem/mod.rs
>> index 30c853988b94..4afd36e49205 100644
>> --- a/rust/kernel/drm/gem/mod.rs
>> +++ b/rust/kernel/drm/gem/mod.rs
>> @@ -10,8 +10,8 @@
>>     drm::driver::{AllocImpl, AllocOps},
>>     error::{to_result, Result},
>>     prelude::*,
>> -    sync::aref::{ARef, AlwaysRefCounted},
>> -    types::Opaque,
>> +    sync::aref::RefCounted,
>> +    types::{ARef, AlwaysRefCounted, Opaque},
>> };
>> use core::{ops::Deref, ptr::NonNull};
>> 
>> @@ -56,7 +56,7 @@ pub trait IntoGEMObject: Sized + super::private::Sealed + AlwaysRefCounted {
>> }
>> 
>> // SAFETY: All gem objects are refcounted.
>> -unsafe impl<T: IntoGEMObject> AlwaysRefCounted for T {
>> +unsafe impl<T: IntoGEMObject> RefCounted for T {
>>     fn inc_ref(&self) {
>>         // SAFETY: The existence of a shared reference guarantees that the refcount is non-zero.
>>         unsafe { bindings::drm_gem_object_get(self.as_raw()) };
>> @@ -75,6 +75,10 @@ unsafe fn dec_ref(obj: NonNull<Self>) {
>>     }
>> }
>> 
>> +// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef<T>` from a
>> +// `&T`.
>> +unsafe impl<T: IntoGEMObject> crate::types::AlwaysRefCounted for T {}
>> +
>> extern "C" fn open_callback<T: DriverObject>(
>>     raw_obj: *mut bindings::drm_gem_object,
>>     raw_file: *mut bindings::drm_file,
>> diff --git a/rust/kernel/fs/file.rs b/rust/kernel/fs/file.rs
>> index cd6987850332..86309ca5dad0 100644
>> --- a/rust/kernel/fs/file.rs
>> +++ b/rust/kernel/fs/file.rs
>> @@ -12,8 +12,8 @@
>>     cred::Credential,
>>     error::{code::*, to_result, Error, Result},
>>     fmt,
>> -    sync::aref::{ARef, AlwaysRefCounted},
>> -    types::{NotThreadSafe, Opaque},
>> +    sync::aref::RefCounted,
>> +    types::{ARef, AlwaysRefCounted, NotThreadSafe, Opaque},
>> };
>> use core::ptr;
>> 
>> @@ -192,7 +192,7 @@ unsafe impl Sync for File {}
>> 
>> // SAFETY: The type invariants guarantee that `File` is always ref-counted. This implementation
>> // makes `ARef<File>` own a normal refcount.
>> -unsafe impl AlwaysRefCounted for File {
>> +unsafe impl RefCounted for File {
>>     #[inline]
>>     fn inc_ref(&self) {
>>         // SAFETY: The existence of a shared reference means that the refcount is nonzero.
>> @@ -207,6 +207,10 @@ unsafe fn dec_ref(obj: ptr::NonNull<File>) {
>>     }
>> }
>> 
>> +// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef<File>` from a
>> +// `&File`.
>> +unsafe impl AlwaysRefCounted for File {}
>> +
>> /// Wraps the kernel's `struct file`. Not thread safe.
>> ///
>> /// This type represents a file that is not known to be safe to transfer across thread boundaries.
>> @@ -228,7 +232,7 @@ pub struct LocalFile {
>> 
>> // SAFETY: The type invariants guarantee that `LocalFile` is always ref-counted. This implementation
>> // makes `ARef<LocalFile>` own a normal refcount.
>> -unsafe impl AlwaysRefCounted for LocalFile {
>> +unsafe impl RefCounted for LocalFile {
>>     #[inline]
>>     fn inc_ref(&self) {
>>         // SAFETY: The existence of a shared reference means that the refcount is nonzero.
>> @@ -244,6 +248,10 @@ unsafe fn dec_ref(obj: ptr::NonNull<LocalFile>) {
>>     }
>> }
>> 
>> +// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef<LocalFile>` from a
>> +// `&LocalFile`.
>> +unsafe impl AlwaysRefCounted for LocalFile {}
>> +
>> impl LocalFile {
>>     /// Constructs a new `struct file` wrapper from a file descriptor.
>>     ///
>> diff --git a/rust/kernel/mm.rs b/rust/kernel/mm.rs
>> index 4764d7b68f2a..dd9e3969e720 100644
>> --- a/rust/kernel/mm.rs
>> +++ b/rust/kernel/mm.rs
>> @@ -13,8 +13,8 @@
>> 
>> use crate::{
>>     bindings,
>> -    sync::aref::{ARef, AlwaysRefCounted},
>> -    types::{NotThreadSafe, Opaque},
>> +    sync::aref::RefCounted,
>> +    types::{ARef, AlwaysRefCounted, NotThreadSafe, Opaque},
>> };
>> use core::{ops::Deref, ptr::NonNull};
>> 
>> @@ -55,7 +55,7 @@ unsafe impl Send for Mm {}
>> unsafe impl Sync for Mm {}
>> 
>> // SAFETY: By the type invariants, this type is always refcounted.
>> -unsafe impl AlwaysRefCounted for Mm {
>> +unsafe impl RefCounted for Mm {
>>     #[inline]
>>     fn inc_ref(&self) {
>>         // SAFETY: The pointer is valid since self is a reference.
>> @@ -69,6 +69,9 @@ unsafe fn dec_ref(obj: NonNull<Self>) {
>>     }
>> }
>> 
>> +// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef<Mm>` from a `&Mm`.
>> +unsafe impl AlwaysRefCounted for Mm {}
>> +
>> /// A wrapper for the kernel's `struct mm_struct`.
>> ///
>> /// This type is like [`Mm`], but with non-zero `mm_users`. It can only be used when `mm_users` can
>> @@ -91,7 +94,7 @@ unsafe impl Send for MmWithUser {}
>> unsafe impl Sync for MmWithUser {}
>> 
>> // SAFETY: By the type invariants, this type is always refcounted.
>> -unsafe impl AlwaysRefCounted for MmWithUser {
>> +unsafe impl RefCounted for MmWithUser {
>>     #[inline]
>>     fn inc_ref(&self) {
>>         // SAFETY: The pointer is valid since self is a reference.
>> @@ -105,6 +108,10 @@ unsafe fn dec_ref(obj: NonNull<Self>) {
>>     }
>> }
>> 
>> +// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef<MmWithUser>` from a
>> +// `&MmWithUser`.
>> +unsafe impl AlwaysRefCounted for MmWithUser {}
>> +
>> // Make all `Mm` methods available on `MmWithUser`.
>> impl Deref for MmWithUser {
>>     type Target = Mm;
>> diff --git a/rust/kernel/mm/mmput_async.rs b/rust/kernel/mm/mmput_async.rs
>> index b8d2f051225c..aba4ce675c86 100644
>> --- a/rust/kernel/mm/mmput_async.rs
>> +++ b/rust/kernel/mm/mmput_async.rs
>> @@ -10,7 +10,8 @@
>> use crate::{
>>     bindings,
>>     mm::MmWithUser,
>> -    sync::aref::{ARef, AlwaysRefCounted},
>> +    sync::aref::RefCounted,
>> +    types::{ARef, AlwaysRefCounted},
>> };
>> use core::{ops::Deref, ptr::NonNull};
>> 
>> @@ -34,7 +35,7 @@ unsafe impl Send for MmWithUserAsync {}
>> unsafe impl Sync for MmWithUserAsync {}
>> 
>> // SAFETY: By the type invariants, this type is always refcounted.
>> -unsafe impl AlwaysRefCounted for MmWithUserAsync {
>> +unsafe impl RefCounted for MmWithUserAsync {
>>     #[inline]
>>     fn inc_ref(&self) {
>>         // SAFETY: The pointer is valid since self is a reference.
>> @@ -48,6 +49,10 @@ unsafe fn dec_ref(obj: NonNull<Self>) {
>>     }
>> }
>> 
>> +// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef<MmWithUserAsync>`
>> +// from a `&MmWithUserAsync`.
>> +unsafe impl AlwaysRefCounted for MmWithUserAsync {}
>> +
>> // Make all `MmWithUser` methods available on `MmWithUserAsync`.
>> impl Deref for MmWithUserAsync {
>>     type Target = MmWithUser;
>> diff --git a/rust/kernel/opp.rs b/rust/kernel/opp.rs
>> index 2c763fa9276d..77d1cc89c412 100644
>> --- a/rust/kernel/opp.rs
>> +++ b/rust/kernel/opp.rs
>> @@ -16,8 +16,8 @@
>>     ffi::c_ulong,
>>     prelude::*,
>>     str::CString,
>> -    sync::aref::{ARef, AlwaysRefCounted},
>> -    types::Opaque,
>> +    sync::aref::RefCounted,
>> +    types::{ARef, AlwaysRefCounted, Opaque},
>> };
>> 
>> #[cfg(CONFIG_CPU_FREQ)]
>> @@ -1037,7 +1037,7 @@ unsafe impl Send for OPP {}
>> unsafe impl Sync for OPP {}
>> 
>> /// SAFETY: The type invariants guarantee that [`OPP`] is always refcounted.
>> -unsafe impl AlwaysRefCounted for OPP {
>> +unsafe impl RefCounted for OPP {
>>     fn inc_ref(&self) {
>>         // SAFETY: The existence of a shared reference means that the refcount is nonzero.
>>         unsafe { bindings::dev_pm_opp_get(self.0.get()) };
>> @@ -1049,6 +1049,10 @@ unsafe fn dec_ref(obj: ptr::NonNull<Self>) {
>>     }
>> }
>> 
>> +// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef<OPP>` from an
>> +// `&OPP`.
>> +unsafe impl AlwaysRefCounted for OPP {}
>> +
>> impl OPP {
>>     /// Creates an owned reference to a [`OPP`] from a valid pointer.
>>     ///
>> diff --git a/rust/kernel/owned.rs b/rust/kernel/owned.rs
>> index a2cdd2cb8a10..a26747cbc13b 100644
>> --- a/rust/kernel/owned.rs
>> +++ b/rust/kernel/owned.rs
>> @@ -21,7 +21,7 @@
>> ///
>> /// Note: The underlying object is not required to provide internal reference counting, because it
>> /// represents a unique, owned reference. If reference counting (on the Rust side) is required,
>> -/// [`AlwaysRefCounted`](crate::types::AlwaysRefCounted) should be implemented.
>> +/// [`RefCounted`](crate::types::RefCounted) should be implemented.
>> ///
>> /// # Safety
>> ///
>> diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
>> index 7fcc5f6022c1..9ac70823fb4d 100644
>> --- a/rust/kernel/pci.rs
>> +++ b/rust/kernel/pci.rs
>> @@ -13,8 +13,8 @@
>>     io::{Io, IoRaw},
>>     irq::{self, IrqRequest},
>>     str::CStr,
>> -    sync::aref::ARef,
>> -    types::Opaque,
>> +    sync::aref::{AlwaysRefCounted, RefCounted},
>> +    types::{ARef, Opaque},
>>     ThisModule,
>> };
>> use core::{
>> @@ -601,7 +601,7 @@ pub fn set_master(&self) {
>> impl crate::dma::Device for Device<device::Core> {}
>> 
>> // SAFETY: Instances of `Device` are always reference-counted.
>> -unsafe impl crate::sync::aref::AlwaysRefCounted for Device {
>> +unsafe impl RefCounted for Device {
>>     fn inc_ref(&self) {
>>         // SAFETY: The existence of a shared reference guarantees that the refcount is non-zero.
>>         unsafe { bindings::pci_dev_get(self.as_raw()) };
>> @@ -613,6 +613,10 @@ unsafe fn dec_ref(obj: NonNull<Self>) {
>>     }
>> }
>> 
>> +// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef<Device>` from a
>> +// `&Device`.
>> +unsafe impl AlwaysRefCounted for Device {}
>> +
>> impl<Ctx: device::DeviceContext> AsRef<device::Device<Ctx>> for Device<Ctx> {
>>     fn as_ref(&self) -> &device::Device<Ctx> {
>>         // SAFETY: By the type invariant of `Self`, `self.as_raw()` is a pointer to a valid
>> diff --git a/rust/kernel/pid_namespace.rs b/rust/kernel/pid_namespace.rs
>> index 979a9718f153..4f6a94540e33 100644
>> --- a/rust/kernel/pid_namespace.rs
>> +++ b/rust/kernel/pid_namespace.rs
>> @@ -7,7 +7,11 @@
>> //! C header: [`include/linux/pid_namespace.h`](srctree/include/linux/pid_namespace.h) and
>> //! [`include/linux/pid.h`](srctree/include/linux/pid.h)
>> 
>> -use crate::{bindings, sync::aref::AlwaysRefCounted, types::Opaque};
>> +use crate::{
>> +    bindings,
>> +    sync::aref::RefCounted,
>> +    types::{AlwaysRefCounted, Opaque},
>> +};
>> use core::ptr;
>> 
>> /// Wraps the kernel's `struct pid_namespace`. Thread safe.
>> @@ -41,7 +45,7 @@ pub unsafe fn from_ptr<'a>(ptr: *const bindings::pid_namespace) -> &'a Self {
>> }
>> 
>> // SAFETY: Instances of `PidNamespace` are always reference-counted.
>> -unsafe impl AlwaysRefCounted for PidNamespace {
>> +unsafe impl RefCounted for PidNamespace {
>>     #[inline]
>>     fn inc_ref(&self) {
>>         // SAFETY: The existence of a shared reference means that the refcount is nonzero.
>> @@ -55,6 +59,10 @@ unsafe fn dec_ref(obj: ptr::NonNull<PidNamespace>) {
>>     }
>> }
>> 
>> +// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef<PidNamespace>` from
>> +// a `&PidNamespace`.
>> +unsafe impl AlwaysRefCounted for PidNamespace {}
>> +
>> // SAFETY:
>> // - `PidNamespace::dec_ref` can be called from any thread.
>> // - It is okay to send ownership of `PidNamespace` across thread boundaries.
>> diff --git a/rust/kernel/platform.rs b/rust/kernel/platform.rs
>> index 7205fe3416d3..bf2aa496b377 100644
>> --- a/rust/kernel/platform.rs
>> +++ b/rust/kernel/platform.rs
>> @@ -13,6 +13,7 @@
>>     irq::{self, IrqRequest},
>>     of,
>>     prelude::*,
>> +    sync::aref::{AlwaysRefCounted, RefCounted},
>>     types::Opaque,
>>     ThisModule,
>> };
>> @@ -468,7 +469,7 @@ pub fn optional_irq_by_name(&self, name: &CStr) -> Result<IrqRequest<'_>> {
>> impl crate::dma::Device for Device<device::Core> {}
>> 
>> // SAFETY: Instances of `Device` are always reference-counted.
>> -unsafe impl crate::sync::aref::AlwaysRefCounted for Device {
>> +unsafe impl RefCounted for Device {
>>     fn inc_ref(&self) {
>>         // SAFETY: The existence of a shared reference guarantees that the refcount is non-zero.
>>         unsafe { bindings::get_device(self.as_ref().as_raw()) };
>> @@ -480,6 +481,10 @@ unsafe fn dec_ref(obj: NonNull<Self>) {
>>     }
>> }
>> 
>> +// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef<Device>` from a
>> +// `&Device`.
>> +unsafe impl AlwaysRefCounted for Device {}
>> +
>> impl<Ctx: device::DeviceContext> AsRef<device::Device<Ctx>> for Device<Ctx> {
>>     fn as_ref(&self) -> &device::Device<Ctx> {
>>         // SAFETY: By the type invariant of `Self`, `self.as_raw()` is a pointer to a valid
>> diff --git a/rust/kernel/sync/aref.rs b/rust/kernel/sync/aref.rs
>> index e175aefe8615..4226119d5ac9 100644
>> --- a/rust/kernel/sync/aref.rs
>> +++ b/rust/kernel/sync/aref.rs
>> @@ -19,11 +19,9 @@
>> 
>> use core::{marker::PhantomData, mem::ManuallyDrop, ops::Deref, ptr::NonNull};
>> 
>> -/// Types that are _always_ reference counted.
>> +/// Types that are internally reference counted.
>> ///
>> /// It allows such types to define their own custom ref increment and decrement functions.
>> -/// Additionally, it allows users to convert from a shared reference `&T` to an owned reference
>> -/// [`ARef<T>`].
>> ///
>> /// This is usually implemented by wrappers to existing structures on the C side of the code. For
>> /// Rust code, the recommendation is to use [`Arc`](crate::sync::Arc) to create reference-counted
>> @@ -40,9 +38,8 @@
>> /// at least until matching decrements are performed.
>> ///
>> /// Implementers must also ensure that all instances are reference-counted. (Otherwise they
>> -/// won't be able to honour the requirement that [`AlwaysRefCounted::inc_ref`] keep the object
>> -/// alive.)
>> -pub unsafe trait AlwaysRefCounted {
>> +/// won't be able to honour the requirement that [`RefCounted::inc_ref`] keep the object alive.)
>> +pub unsafe trait RefCounted {
>>     /// Increments the reference count on the object.
>>     fn inc_ref(&self);
>> 
>> @@ -55,11 +52,27 @@ pub unsafe trait AlwaysRefCounted {
>>     /// Callers must ensure that there was a previous matching increment to the reference count,
>>     /// and that the object is no longer used after its reference count is decremented (as it may
>>     /// result in the object being freed), unless the caller owns another increment on the refcount
>> -    /// (e.g., it calls [`AlwaysRefCounted::inc_ref`] twice, then calls
>> -    /// [`AlwaysRefCounted::dec_ref`] once).
>> +    /// (e.g., it calls [`RefCounted::inc_ref`] twice, then calls [`RefCounted::dec_ref`] once).
>>     unsafe fn dec_ref(obj: NonNull<Self>);
>> }
>> 
>> +/// Always reference-counted type.
>> +///
>> +/// It allows to derive a counted reference [`ARef<T>`] from a `&T`.
>> +///
>> +/// This provides some convenience, but it allows "escaping" borrow checks on `&T`. As it
>> +/// complicates attempts to ensure that a reference to T is unique, it is optional to provide for
>> +/// [`RefCounted`] types. See *Safety* below.
>> +///
>> +/// # Safety
>> +///
>> +/// Implementers must ensure that no safety invariants are violated by upgrading an `&T` to an
>> +/// [`ARef<T>`]. In particular that implies [`AlwaysRefCounted`] and [`crate::types::Ownable`]
>> +/// cannot be implemented for the same type, as this would allow to violate the uniqueness guarantee
>> +/// of [`crate::types::Owned<T>`] by derefencing it into an `&T` and obtaining an [`ARef`] from
>> +/// that.
>> +pub unsafe trait AlwaysRefCounted: RefCounted {}
>> +
>> /// An owned reference to an always-reference-counted object.
>> ///
>> /// The object's reference count is automatically decremented when an instance of [`ARef`] is
>> @@ -70,7 +83,7 @@ pub unsafe trait AlwaysRefCounted {
>> ///
>> /// The pointer stored in `ptr` is non-null and valid for the lifetime of the [`ARef`] instance. In
>> /// particular, the [`ARef`] instance owns an increment on the underlying object's reference count.
>> -pub struct ARef<T: AlwaysRefCounted> {
>> +pub struct ARef<T: RefCounted> {
>>     ptr: NonNull<T>,
>>     _p: PhantomData<T>,
>> }
>> @@ -79,16 +92,16 @@ pub struct ARef<T: AlwaysRefCounted> {
>> // it effectively means sharing `&T` (which is safe because `T` is `Sync`); additionally, it needs
>> // `T` to be `Send` because any thread that has an `ARef<T>` may ultimately access `T` using a
>> // mutable reference, for example, when the reference count reaches zero and `T` is dropped.
>> -unsafe impl<T: AlwaysRefCounted + Sync + Send> Send for ARef<T> {}
>> +unsafe impl<T: RefCounted + Sync + Send> Send for ARef<T> {}
>> 
>> // SAFETY: It is safe to send `&ARef<T>` to another thread when the underlying `T` is `Sync`
>> // because it effectively means sharing `&T` (which is safe because `T` is `Sync`); additionally,
>> // it needs `T` to be `Send` because any thread that has a `&ARef<T>` may clone it and get an
>> // `ARef<T>` on that thread, so the thread may ultimately access `T` using a mutable reference, for
>> // example, when the reference count reaches zero and `T` is dropped.
>> -unsafe impl<T: AlwaysRefCounted + Sync + Send> Sync for ARef<T> {}
>> +unsafe impl<T: RefCounted + Sync + Send> Sync for ARef<T> {}
>> 
>> -impl<T: AlwaysRefCounted> ARef<T> {
>> +impl<T: RefCounted> ARef<T> {
>>     /// Creates a new instance of [`ARef`].
>>     ///
>>     /// It takes over an increment of the reference count on the underlying object.
>> @@ -117,12 +130,12 @@ pub unsafe fn from_raw(ptr: NonNull<T>) -> Self {
>>     ///
>>     /// ```
>>     /// use core::ptr::NonNull;
>> -    /// use kernel::sync::aref::{ARef, AlwaysRefCounted};
>> +    /// use kernel::sync::aref::{ARef, RefCounted};
>>     ///
>>     /// struct Empty {}
>>     ///
>>     /// # // SAFETY: TODO.
>> -    /// unsafe impl AlwaysRefCounted for Empty {
>> +    /// unsafe impl RefCounted for Empty {
>>     ///     fn inc_ref(&self) {}
>>     ///     unsafe fn dec_ref(_obj: NonNull<Self>) {}
>>     /// }
>> @@ -140,7 +153,7 @@ pub fn into_raw(me: Self) -> NonNull<T> {
>>     }
>> }
>> 
>> -impl<T: AlwaysRefCounted> Clone for ARef<T> {
>> +impl<T: RefCounted> Clone for ARef<T> {
>>     fn clone(&self) -> Self {
>>         self.inc_ref();
>>         // SAFETY: We just incremented the refcount above.
>> @@ -148,7 +161,7 @@ fn clone(&self) -> Self {
>>     }
>> }
>> 
>> -impl<T: AlwaysRefCounted> Deref for ARef<T> {
>> +impl<T: RefCounted> Deref for ARef<T> {
>>     type Target = T;
>> 
>>     fn deref(&self) -> &Self::Target {
>> @@ -165,7 +178,7 @@ fn from(b: &T) -> Self {
>>     }
>> }
>> 
>> -impl<T: AlwaysRefCounted> Drop for ARef<T> {
>> +impl<T: RefCounted> Drop for ARef<T> {
>>     fn drop(&mut self) {
>>         // SAFETY: The type invariants guarantee that the `ARef` owns the reference we're about to
>>         // decrement.
>> diff --git a/rust/kernel/task.rs b/rust/kernel/task.rs
>> index 49fad6de0674..0a6e38d98456 100644
>> --- a/rust/kernel/task.rs
>> +++ b/rust/kernel/task.rs
>> @@ -9,8 +9,8 @@
>>     ffi::{c_int, c_long, c_uint},
>>     mm::MmWithUser,
>>     pid_namespace::PidNamespace,
>> -    sync::aref::ARef,
>> -    types::{NotThreadSafe, Opaque},
>> +    sync::aref::{AlwaysRefCounted, RefCounted},
>> +    types::{ARef, NotThreadSafe, Opaque},
>> };
>> use core::{
>>     cmp::{Eq, PartialEq},
>> @@ -348,7 +348,7 @@ pub fn active_pid_ns(&self) -> Option<&PidNamespace> {
>> }
>> 
>> // SAFETY: The type invariants guarantee that `Task` is always refcounted.
>> -unsafe impl crate::sync::aref::AlwaysRefCounted for Task {
>> +unsafe impl RefCounted for Task {
>>     #[inline]
>>     fn inc_ref(&self) {
>>         // SAFETY: The existence of a shared reference means that the refcount is nonzero.
>> @@ -362,6 +362,10 @@ unsafe fn dec_ref(obj: ptr::NonNull<Self>) {
>>     }
>> }
>> 
>> +// SAFETY: We do not implement `Ownable`, thus it is okay to obtain an `ARef<Task>` from a
>> +// `&Task`.
>> +unsafe impl AlwaysRefCounted for Task {}
>> +
>> impl Kuid {
>>     /// Get the current euid.
>>     #[inline]
>> diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
>> index 7bc07c38cd6c..8ef01393352b 100644
>> --- a/rust/kernel/types.rs
>> +++ b/rust/kernel/types.rs
>> @@ -13,7 +13,7 @@
>> 
>> pub use crate::owned::{Ownable, Owned};
>> 
>> -pub use crate::sync::aref::{ARef, AlwaysRefCounted};
>> +pub use crate::sync::aref::{ARef, AlwaysRefCounted, RefCounted};
>> 
>> /// Used to transfer ownership to and from foreign (non-Rust) languages.
>> ///
>> 
>> -- 
>> 2.51.2
>> 
>> 
>> 
>
> Can you use imperative voice in the title? i.e.:
>
> rust: rename `AlwaysRefCounted` to `RefCounted
>
> …or something similar?
>
> Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>

How about this:

rust: rename `AlwaysRefCounted` to `RefCounted`.

Change `AlwaysRefCounted` to be a marker trait that indicate that obtaining
an `ARef<T>` from a `&T` is possible.

- Rename `AlwaysRefCounted` to `RefCounted`.
- Add a new unsafe trait `AlwaysRefCounted`.
- Implement the new trait `AlwaysRefCounted` for the newly renamed
  `RefCounted` implementations. This leaves functionality of existing
  implementers of `AlwaysRefCounted` intact.


Best regards,
Andreas Hindborg




^ permalink raw reply

* Re: [PATCH v13 4/4] rust: Add `OwnableRefCounted`
From: Andreas Hindborg @ 2026-02-02 10:06 UTC (permalink / raw)
  To: Daniel Almeida, Oliver Mangold
  Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Alice Ryhl, Trevor Gross, Benno Lossin,
	Danilo Krummrich, Greg Kroah-Hartman, Dave Ertman, Ira Weiny,
	Leon Romanovsky, Rafael J. Wysocki, Maarten Lankhorst,
	Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter,
	Alexander Viro, Christian Brauner, Jan Kara, Lorenzo Stoakes,
	Liam R. Howlett, Viresh Kumar, Nishanth Menon, Stephen Boyd,
	Bjorn Helgaas, Krzysztof Wilczyński, Paul Moore,
	Serge Hallyn, Asahi Lina, rust-for-linux, linux-kernel,
	linux-block, dri-devel, linux-fsdevel, linux-mm, linux-pm,
	linux-pci, linux-security-module
In-Reply-To: <A5A7C4C9-1504-439C-B4FF-C28482AF7444@collabora.com>

Daniel Almeida <daniel.almeida@collabora.com> writes:

> Hi Oliver,
>
>> On 17 Nov 2025, at 07:08, Oliver Mangold <oliver.mangold@pm.me> wrote:
>> 
>> Types implementing one of these traits can safely convert between an
>> `ARef<T>` and an `Owned<T>`.
>> 
>> This is useful for types which generally are accessed through an `ARef`
>> but have methods which can only safely be called when the reference is
>> unique, like e.g. `block::mq::Request::end_ok()`.
>> 
>> Signed-off-by: Oliver Mangold <oliver.mangold@pm.me>
>> Co-developed-by: Andreas Hindborg <a.hindborg@kernel.org>
>> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
>> Reviewed-by: Andreas Hindborg <a.hindborg@kernel.org>
>> ---
>> rust/kernel/owned.rs     | 138 ++++++++++++++++++++++++++++++++++++++++++++---
>> rust/kernel/sync/aref.rs |  11 +++-
>> rust/kernel/types.rs     |   2 +-
>> 3 files changed, 141 insertions(+), 10 deletions(-)
>> 
>> diff --git a/rust/kernel/owned.rs b/rust/kernel/owned.rs
>> index a26747cbc13b..26ab2b00ada0 100644
>> --- a/rust/kernel/owned.rs
>> +++ b/rust/kernel/owned.rs
>> @@ -5,6 +5,7 @@
>> //! These pointer types are useful for C-allocated objects which by API-contract
>> //! are owned by Rust, but need to be freed through the C API.
>> 
>> +use crate::sync::aref::{ARef, RefCounted};
>> use core::{
>>     mem::ManuallyDrop,
>>     ops::{Deref, DerefMut},
>> @@ -14,14 +15,16 @@
>> 
>> /// Type allocated and destroyed on the C side, but owned by Rust.
>> ///
>> -/// Implementing this trait allows types to be referenced via the [`Owned<Self>`] pointer type. This
>> -/// is useful when it is desirable to tie the lifetime of the reference to an owned object, rather
>> -/// than pass around a bare reference. [`Ownable`] types can define custom drop logic that is
>> -/// executed when the owned reference [`Owned<Self>`] pointing to the object is dropped.
>> +/// Implementing this trait allows types to be referenced via the [`Owned<Self>`] pointer type.
>> +///  - This is useful when it is desirable to tie the lifetime of an object reference to an owned
>> +///    object, rather than pass around a bare reference.
>> +///  - [`Ownable`] types can define custom drop logic that is executed when the owned reference
>> +///    of type [`Owned<_>`] pointing to the object is dropped.
>> ///
>> /// Note: The underlying object is not required to provide internal reference counting, because it
>> /// represents a unique, owned reference. If reference counting (on the Rust side) is required,
>> -/// [`RefCounted`](crate::types::RefCounted) should be implemented.
>> +/// [`RefCounted`] should be implemented. [`OwnableRefCounted`] should be implemented if conversion
>> +/// between unique and shared (reference counted) ownership is needed.
>> ///
>> /// # Safety
>> ///
>> @@ -143,9 +146,7 @@ impl<T: Ownable> Owned<T> {
>>     ///   mutable reference requirements. That is, the kernel will not mutate or free the underlying
>>     ///   object and is okay with it being modified by Rust code.
>>     pub unsafe fn from_raw(ptr: NonNull<T>) -> Self {
>> -        Self {
>> -            ptr,
>> -        }
>> +        Self { ptr }
>>     }
>
> Unrelated change?

Looks like a rustfmt thing. I'll split it out or remove it.

>
>> 
>>     /// Consumes the [`Owned`], returning a raw pointer.
>> @@ -193,3 +194,124 @@ fn drop(&mut self) {
>>         unsafe { T::release(self.ptr) };
>>     }
>> }
>> +
>> +/// A trait for objects that can be wrapped in either one of the reference types [`Owned`] and
>> +/// [`ARef`].
>> +///
>> +/// # Examples
>> +///
>> +/// A minimal example implementation of [`OwnableRefCounted`], [`Ownable`] and its usage with
>> +/// [`ARef`] and [`Owned`] looks like this:
>> +///
>> +/// ```
>> +/// # #![expect(clippy::disallowed_names)]
>> +/// # use core::cell::Cell;
>> +/// # use core::ptr::NonNull;
>> +/// # use kernel::alloc::{flags, kbox::KBox, AllocError};
>> +/// # use kernel::sync::aref::{ARef, RefCounted};
>> +/// # use kernel::types::{Owned, Ownable, OwnableRefCounted};
>> +///
>> +/// // Example internally refcounted struct.
>
> nit: IMHO the wording could improve ^

/// // An internally refcounted struct for demonstration purposes.

>
>> +/// //
>> +/// // # Invariants
>> +/// //
>> +/// // - `refcount` is always non-zero for a valid object.
>> +/// // - `refcount` is >1 if there are more than 1 Rust reference to it.
>> +/// //
>> +/// struct Foo {
>> +///     refcount: Cell<usize>,
>> +/// }
>> +///
>> +/// impl Foo {
>> +///     fn new() -> Result<Owned<Self>, AllocError> {
>> +///         // We are just using a `KBox` here to handle the actual allocation, as our `Foo` is
>> +///         // not actually a C-allocated object.
>> +///         let result = KBox::new(
>> +///             Foo {
>> +///                 refcount: Cell::new(1),
>> +///             },
>> +///             flags::GFP_KERNEL,
>> +///         )?;
>> +///         let result = NonNull::new(KBox::into_raw(result))
>> +///             .expect("Raw pointer to newly allocation KBox is null, this should never happen.");
>> +///         // SAFETY: We just allocated the `Self`, thus it is valid and there cannot be any other
>> +///         // Rust references. Calling `into_raw()` makes us responsible for ownership and
>> +///         // we won't use the raw pointer anymore, thus we can transfer ownership to the `Owned`.
>> +///         Ok(unsafe { Owned::from_raw(result) })
>> +///     }
>> +/// }
>> +///
>> +/// // SAFETY: We increment and decrement each time the respective function is called and only free
>> +/// // the `Foo` when the refcount reaches zero.
>> +/// unsafe impl RefCounted for Foo {
>> +///     fn inc_ref(&self) {
>> +///         self.refcount.replace(self.refcount.get() + 1);
>> +///     }
>> +///
>> +///     unsafe fn dec_ref(this: NonNull<Self>) {
>> +///         // SAFETY: By requirement on calling this function, the refcount is non-zero,
>> +///         // implying the underlying object is valid.
>> +///         let refcount = unsafe { &this.as_ref().refcount };
>> +///         let new_refcount = refcount.get() - 1;
>> +///         if new_refcount == 0 {
>> +///             // The `Foo` will be dropped when `KBox` goes out of scope.
>> +///             // SAFETY: The [`KBox<Foo>`] is still alive as the old refcount is 1. We can pass
>> +///             // ownership to the [`KBox`] as by requirement on calling this function,
>> +///             // the `Self` will no longer be used by the caller.
>> +///             unsafe { KBox::from_raw(this.as_ptr()) };
>> +///         } else {
>> +///             refcount.replace(new_refcount);
>> +///         }
>> +///     }
>> +/// }
>> +///
>> +/// impl OwnableRefCounted for Foo {
>> +///     fn try_from_shared(this: ARef<Self>) -> Result<Owned<Self>, ARef<Self>> {
>> +///         if this.refcount.get() == 1 {
>> +///             // SAFETY: The `Foo` is still alive and has no other Rust references as the refcount
>> +///             // is 1.
>> +///             Ok(unsafe { Owned::from_raw(ARef::into_raw(this)) })
>> +///         } else {
>> +///             Err(this)
>> +///         }
>> +///     }
>> +/// }
>> +///
>
> We wouldn’t need this implementation if we added a “refcount()”
> member to this trait. This lets you abstract away this logic for all
> implementors, which has the massive upside of making sure we hardcode (and thus
> enforce) the refcount == 1 check.

This exist to allow reference counting schemes that are different from
the general implementation. See [1] for an example.

[1] https://github.com/metaspace/linux/blob/3e46e95f0707fa71259b1d241f689144ad61cc62/rust/kernel/block/mq/request.rs#L478

>
>
>> +/// // SAFETY: This implementation of `release()` is safe for any valid `Self`.
>> +/// unsafe impl Ownable for Foo {
>> +///     unsafe fn release(this: NonNull<Self>) {
>> +///         // SAFETY: Using `dec_ref()` from [`RefCounted`] to release is okay, as the refcount is
>> +///         // always 1 for an [`Owned<Foo>`].
>> +///         unsafe{ Foo::dec_ref(this) };
>> +///     }
>> +/// }
>> +///
>> +/// let foo = Foo::new().expect("Failed to allocate a Foo. This shouldn't happen");
>
> All these “expects()” and custom error strings would go away if you
> place this behind a fictional function that returns Result.

I'll do that.

>
>> +/// let mut foo = ARef::from(foo);
>> +/// {
>> +///     let bar = foo.clone();
>> +///     assert!(Owned::try_from(bar).is_err());
>> +/// }
>> +/// assert!(Owned::try_from(foo).is_ok());
>> +/// ```
>> +pub trait OwnableRefCounted: RefCounted + Ownable + Sized {
>> +    /// Checks if the [`ARef`] is unique and convert it to an [`Owned`] it that is that case.
>> +    /// Otherwise it returns again an [`ARef`] to the same underlying object.
>> +    fn try_from_shared(this: ARef<Self>) -> Result<Owned<Self>, ARef<Self>>;
>
> Again, this method can go way if we add a method to expose the refcount.
>
>> +
>> +    /// Converts the [`Owned`] into an [`ARef`].
>> +    fn into_shared(this: Owned<Self>) -> ARef<Self> {
>> +        // SAFETY: Safe by the requirements on implementing the trait.
>> +        unsafe { ARef::from_raw(Owned::into_raw(this)) }
>> +    }
>> +}
>> +
>> +impl<T: OwnableRefCounted> TryFrom<ARef<T>> for Owned<T> {
>> +    type Error = ARef<T>;
>> +    /// Tries to convert the [`ARef`] to an [`Owned`] by calling
>> +    /// [`try_from_shared()`](OwnableRefCounted::try_from_shared). In case the [`ARef`] is not
>> +    /// unique, it returns again an [`ARef`] to the same underlying object.
>> +    fn try_from(b: ARef<T>) -> Result<Owned<T>, Self::Error> {
>> +        T::try_from_shared(b)
>> +    }
>> +}
>> diff --git a/rust/kernel/sync/aref.rs b/rust/kernel/sync/aref.rs
>> index 937dcf6ed5de..2dbffe2ed1b8 100644
>> --- a/rust/kernel/sync/aref.rs
>> +++ b/rust/kernel/sync/aref.rs
>> @@ -30,7 +30,10 @@
>> /// Note: Implementing this trait allows types to be wrapped in an [`ARef<Self>`]. It requires an
>> /// internal reference count and provides only shared references. If unique references are required
>> /// [`Ownable`](crate::types::Ownable) should be implemented which allows types to be wrapped in an
>> -/// [`Owned<Self>`](crate::types::Owned).
>> +/// [`Owned<Self>`](crate::types::Owned). Implementing the trait
>> +/// [`OwnableRefCounted`](crate::types::OwnableRefCounted) allows to convert between unique and
>> +/// shared references (i.e. [`Owned<Self>`](crate::types::Owned) and
>> +/// [`ARef<Self>`](crate::types::Owned)).
>> ///
>> /// # Safety
>> ///
>> @@ -180,6 +183,12 @@ fn from(b: &T) -> Self {
>>     }
>> }
>> 
>> +impl<T: crate::types::OwnableRefCounted> From<crate::types::Owned<T>> for ARef<T> {
>> +    fn from(b: crate::types::Owned<T>) -> Self {
>> +        T::into_shared(b)
>> +    }
>> +}
>> +
>
> Not sure why we’re using fully-qualified names here if we can import them, but your call.

I'll import them.


Best regards,
Andreas Hindborg



^ permalink raw reply

* Re: [PATCH v13 1/4] rust: types: Add Ownable/Owned types
From: Andreas Hindborg @ 2026-02-02  9:37 UTC (permalink / raw)
  To: Gary Guo, Oliver Mangold
  Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Björn Roy Baron,
	Alice Ryhl, Trevor Gross, Benno Lossin, Danilo Krummrich,
	Greg Kroah-Hartman, Dave Ertman, Ira Weiny, Leon Romanovsky,
	Rafael J. Wysocki, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Alexander Viro,
	Christian Brauner, Jan Kara, Lorenzo Stoakes, Liam R. Howlett,
	Viresh Kumar, Nishanth Menon, Stephen Boyd, Bjorn Helgaas,
	Krzysztof Wilczyński, Paul Moore, Serge Hallyn, Asahi Lina,
	rust-for-linux, linux-kernel, linux-block, dri-devel,
	linux-fsdevel, linux-mm, linux-pm, linux-pci,
	linux-security-module
In-Reply-To: <20251201155135.2b9c4084.gary@garyguo.net>

Gary Guo <gary@garyguo.net> writes:

> On Mon, 17 Nov 2025 10:07:40 +0000
> Oliver Mangold <oliver.mangold@pm.me> wrote:
>
>> From: Asahi Lina <lina+kernel@asahilina.net>
>> 
>> By analogy to `AlwaysRefCounted` and `ARef`, an `Ownable` type is a
>> (typically C FFI) type that *may* be owned by Rust, but need not be. Unlike
>> `AlwaysRefCounted`, this mechanism expects the reference to be unique
>> within Rust, and does not allow cloning.
>> 
>> Conceptually, this is similar to a `KBox<T>`, except that it delegates
>> resource management to the `T` instead of using a generic allocator.
>> 
>> [ om:
>>   - Split code into separate file and `pub use` it from types.rs.
>>   - Make from_raw() and into_raw() public.
>>   - Remove OwnableMut, and make DerefMut dependent on Unpin instead.
>>   - Usage example/doctest for Ownable/Owned.
>>   - Fixes to documentation and commit message.
>> ]
>> 
>> Link: https://lore.kernel.org/all/20250202-rust-page-v1-1-e3170d7fe55e@asahilina.net/
>> Signed-off-by: Asahi Lina <lina+kernel@asahilina.net>
>> Co-developed-by: Oliver Mangold <oliver.mangold@pm.me>
>> Signed-off-by: Oliver Mangold <oliver.mangold@pm.me>
>> Co-developed-by: Andreas Hindborg <a.hindborg@kernel.org>
>> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
>> Reviewed-by: Boqun Feng <boqun.feng@gmail.com>
>> ---
>>  rust/kernel/lib.rs       |   1 +
>>  rust/kernel/owned.rs     | 195 +++++++++++++++++++++++++++++++++++++++++++++++
>>  rust/kernel/sync/aref.rs |   5 ++
>>  rust/kernel/types.rs     |   2 +
>>  4 files changed, 203 insertions(+)
>> 
>> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
>> index 3dd7bebe7888..e0ee04330dd0 100644
>> --- a/rust/kernel/lib.rs
>> +++ b/rust/kernel/lib.rs
>> @@ -112,6 +112,7 @@
>>  pub mod of;
>>  #[cfg(CONFIG_PM_OPP)]
>>  pub mod opp;
>> +pub mod owned;
>>  pub mod page;
>>  #[cfg(CONFIG_PCI)]
>>  pub mod pci;
>> diff --git a/rust/kernel/owned.rs b/rust/kernel/owned.rs
>> new file mode 100644
>> index 000000000000..a2cdd2cb8a10
>> --- /dev/null
>> +++ b/rust/kernel/owned.rs
>> @@ -0,0 +1,195 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +
>> +//! Unique owned pointer types for objects with custom drop logic.
>> +//!
>> +//! These pointer types are useful for C-allocated objects which by API-contract
>> +//! are owned by Rust, but need to be freed through the C API.
>> +
>> +use core::{
>> +    mem::ManuallyDrop,
>> +    ops::{Deref, DerefMut},
>> +    pin::Pin,
>> +    ptr::NonNull,
>> +};
>> +
>> +/// Type allocated and destroyed on the C side, but owned by Rust.
>
> The example given in the documentation below shows a valid way of
> defining a type that's handled on the Rust side, so I think this
> message is somewhat inaccurate.
>
> Perhaps something like
>
> 	Types that specify their own way of performing allocation and
> 	destruction. Typically, this trait is implemented on types from
> 	the C side.

Thanks, I'll use this.

>
> ?
>
>> +///
>> +/// Implementing this trait allows types to be referenced via the [`Owned<Self>`] pointer type. This
>> +/// is useful when it is desirable to tie the lifetime of the reference to an owned object, rather
>> +/// than pass around a bare reference. [`Ownable`] types can define custom drop logic that is
>> +/// executed when the owned reference [`Owned<Self>`] pointing to the object is dropped.
>> +///
>> +/// Note: The underlying object is not required to provide internal reference counting, because it
>> +/// represents a unique, owned reference. If reference counting (on the Rust side) is required,
>> +/// [`AlwaysRefCounted`](crate::types::AlwaysRefCounted) should be implemented.
>> +///
>> +/// # Safety
>> +///
>> +/// Implementers must ensure that the [`release()`](Self::release) function frees the underlying
>> +/// object in the correct way for a valid, owned object of this type.
>> +///
>> +/// # Examples
>> +///
>> +/// A minimal example implementation of [`Ownable`] and its usage with [`Owned`] looks like this:
>> +///
>> +/// ```
>> +/// # #![expect(clippy::disallowed_names)]
>> +/// # use core::cell::Cell;
>> +/// # use core::ptr::NonNull;
>> +/// # use kernel::sync::global_lock;
>> +/// # use kernel::alloc::{flags, kbox::KBox, AllocError};
>> +/// # use kernel::types::{Owned, Ownable};
>> +///
>> +/// // Let's count the allocations to see if freeing works.
>> +/// kernel::sync::global_lock! {
>> +///     // SAFETY: we call `init()` right below, before doing anything else.
>> +///     unsafe(uninit) static FOO_ALLOC_COUNT: Mutex<usize> = 0;
>> +/// }
>> +/// // SAFETY: We call `init()` only once, here.
>> +/// unsafe { FOO_ALLOC_COUNT.init() };
>> +///
>> +/// struct Foo {
>> +/// }
>> +///
>> +/// impl Foo {
>> +///     fn new() -> Result<Owned<Self>, AllocError> {
>> +///         // We are just using a `KBox` here to handle the actual allocation, as our `Foo` is
>> +///         // not actually a C-allocated object.
>> +///         let result = KBox::new(
>> +///             Foo {},
>> +///             flags::GFP_KERNEL,
>> +///         )?;
>> +///         let result = NonNull::new(KBox::into_raw(result))
>> +///             .expect("Raw pointer to newly allocation KBox is null, this should never happen.");
>> +///         // Count new allocation
>> +///         *FOO_ALLOC_COUNT.lock() += 1;
>> +///         // SAFETY: We just allocated the `Self`, thus it is valid and there cannot be any other
>> +///         // Rust references. Calling `into_raw()` makes us responsible for ownership and we won't
>> +///         // use the raw pointer anymore. Thus we can transfer ownership to the `Owned`.
>> +///         Ok(unsafe { Owned::from_raw(result) })
>> +///     }
>> +/// }
>> +///
>> +/// // SAFETY: What out `release()` function does is safe of any valid `Self`.
>
> I can't parse this sentence. Is "out" supposed to be a different word?

I think it should be "our".

>
>> +/// unsafe impl Ownable for Foo {
>> +///     unsafe fn release(this: NonNull<Self>) {
>> +///         // The `Foo` will be dropped when `KBox` goes out of scope.
>
> I would just write `drop(unsafe { ... })` to make drop explicit instead
> of commenting about the implicit drop.

Agree, that is easier to read.

>
>> +///         // SAFETY: The [`KBox<Self>`] is still alive. We can pass ownership to the [`KBox`], as
>> +///         // by requirement on calling this function, the `Self` will no longer be used by the
>> +///         // caller.
>> +///         unsafe { KBox::from_raw(this.as_ptr()) };
>> +///         // Count released allocation
>> +///         *FOO_ALLOC_COUNT.lock() -= 1;
>> +///     }
>> +/// }
>> +///
>> +/// {
>> +///    let foo = Foo::new().expect("Failed to allocate a Foo. This shouldn't happen");
>> +///    assert!(*FOO_ALLOC_COUNT.lock() == 1);
>> +/// }
>> +/// // `foo` is out of scope now, so we expect no live allocations.
>> +/// assert!(*FOO_ALLOC_COUNT.lock() == 0);
>> +/// ```
>> +pub unsafe trait Ownable {
>> +    /// Releases the object.
>> +    ///
>> +    /// # Safety
>> +    ///
>> +    /// Callers must ensure that:
>> +    /// - `this` points to a valid `Self`.
>> +    /// - `*this` is no longer used after this call.
>> +    unsafe fn release(this: NonNull<Self>);
>> +}
>> +
>> +/// An owned reference to an owned `T`.
>> +///
>> +/// The [`Ownable`] is automatically freed or released when an instance of [`Owned`] is
>> +/// dropped.
>> +///
>> +/// # Invariants
>> +///
>> +/// - The [`Owned<T>`] has exclusive access to the instance of `T`.
>> +/// - The instance of `T` will stay alive at least as long as the [`Owned<T>`] is alive.
>> +pub struct Owned<T: Ownable> {
>> +    ptr: NonNull<T>,
>> +}
>> +
>> +// SAFETY: It is safe to send an [`Owned<T>`] to another thread when the underlying `T` is [`Send`],
>> +// because of the ownership invariant. Sending an [`Owned<T>`] is equivalent to sending the `T`.
>> +unsafe impl<T: Ownable + Send> Send for Owned<T> {}
>> +
>> +// SAFETY: It is safe to send [`&Owned<T>`] to another thread when the underlying `T` is [`Sync`],
>> +// because of the ownership invariant. Sending an [`&Owned<T>`] is equivalent to sending the `&T`.
>> +unsafe impl<T: Ownable + Sync> Sync for Owned<T> {}
>> +
>> +impl<T: Ownable> Owned<T> {
>> +    /// Creates a new instance of [`Owned`].
>> +    ///
>> +    /// It takes over ownership of the underlying object.
>> +    ///
>> +    /// # Safety
>> +    ///
>> +    /// Callers must ensure that:
>> +    /// - `ptr` points to a valid instance of `T`.
>> +    /// - Ownership of the underlying `T` can be transferred to the `Self<T>` (i.e. operations
>> +    ///   which require ownership will be safe).
>> +    /// - No other Rust references to the underlying object exist. This implies that the underlying
>> +    ///   object is not accessed through `ptr` anymore after the function call (at least until the
>> +    ///   the `Self<T>` is dropped.
>
> Is this correct? If `Self<T>` is dropped then `T::release` is called so
> the pointer should also not be accessed further?

I can't follow you point here. Are you saying that the requirement is
wrong because `T::release` will access the object by reference? If so,
that is part of `Owned<_>::drop`, which is explicitly mentioned in the
comment (until .. dropped).

>
>> +    /// - The C code follows the usual shared reference requirements. That is, the kernel will never
>> +    ///   mutate or free the underlying object (excluding interior mutability that follows the usual
>> +    ///   rules) while Rust owns it.
>
> The concept "interior mutability" doesn't really exist on the C side.
> Also, use of interior mutability (by UnsafeCell) would be incorrect if
> the type is implemented in the rust side (as this requires a
> UnsafePinned).
>
> Interior mutability means things can be mutated behind a shared
> reference -- however in this case, we have a mutable reference (either
> `Pin<&mut Self>` or `&mut Self`)!
>
> Perhaps together with the next line, they could be just phrased like
> this?
>
> - The underlying object must not be accessed (read or mutated) through
>   any pointer other than the created `Owned<T>`.
>   Opt-out is still possbile similar to a mutable reference (e.g. by
>   using p`Opaque`]). 
>
> I think we should just tell the user "this is just a unique reference
> similar to &mut". They should be able to deduce that all the `!Unpin`
> that opts out from uniqueness of mutable reference applies here too.

I agree. I would suggest updating the struct documentation:

    @@ -108,7 +108,7 @@ pub unsafe trait Ownable {
        unsafe fn release(this: NonNull<Self>);
    }

    -/// An owned reference to an owned `T`.
    +/// An mutable reference to an owned `T`.
    ///
    /// The [`Ownable`] is automatically freed or released when an instance of [`Owned`] is
    /// dropped.

And then the safety requirement as

 An `Owned<T>` is a mutable reference to the underlying object. As such,
 the object must not be accessed (read or mutated) through any pointer
 other than the created `Owned<T>`. Opt-out is still possbile similar to
 a mutable reference (e.g. by using [`Opaque`]).


>> +    /// - In case `T` implements [`Unpin`] the previous requirement is extended from shared to
>> +    ///   mutable reference requirements. That is, the kernel will not mutate or free the underlying
>> +    ///   object and is okay with it being modified by Rust code.
>
> - If `T` implements [`Unpin`], the structure must not be mutated for
>   the entire lifetime of `Owned<T>`.

Would it be OK to just write "If `T: Unpin`, the ..."?

Again, opt out is possible, right?

>
>> +    pub unsafe fn from_raw(ptr: NonNull<T>) -> Self {
>
> This needs a (rather trivial) INVARIANT comment.

OK.

>
>> +        Self {
>> +            ptr,
>> +        }
>> +    }
>> +
>> +    /// Consumes the [`Owned`], returning a raw pointer.
>> +    ///
>> +    /// This function does not actually relinquish ownership of the object. After calling this
>
> Perhaps "relinquish" isn't the best word here? In my mental model
> this function is pretty much relinquishing ownership as `Owned<T>` no
> longer exists. It just doesn't release the object.

How about this:


    /// Consumes the [`Owned`], returning a raw pointer.
    ///
    /// This function does not drop the underlying `T`. When this function returns, ownership of the
    /// underlying `T` is with the caller.


Thanks for the comments!


Best regards,
Andreas Hindborg




^ permalink raw reply

* Re: [PATCH v13 2/4] rust: `AlwaysRefCounted` is renamed to `RefCounted`.
From: Andreas Hindborg @ 2026-02-02  9:48 UTC (permalink / raw)
  To: Gary Guo, Oliver Mangold
  Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Björn Roy Baron,
	Alice Ryhl, Trevor Gross, Benno Lossin, Danilo Krummrich,
	Greg Kroah-Hartman, Dave Ertman, Ira Weiny, Leon Romanovsky,
	Rafael J. Wysocki, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, David Airlie, Simona Vetter, Alexander Viro,
	Christian Brauner, Jan Kara, Lorenzo Stoakes, Liam R. Howlett,
	Viresh Kumar, Nishanth Menon, Stephen Boyd, Bjorn Helgaas,
	Krzysztof Wilczyński, Paul Moore, Serge Hallyn, Asahi Lina,
	rust-for-linux, linux-kernel, linux-block, dri-devel,
	linux-fsdevel, linux-mm, linux-pm, linux-pci,
	linux-security-module
In-Reply-To: <20251201160030.6956a834.gary@garyguo.net>

Gary Guo <gary@garyguo.net> writes:

> On Mon, 17 Nov 2025 10:07:57 +0000
> Oliver Mangold <oliver.mangold@pm.me> wrote:
>
>> `AlwaysRefCounted` will become a marker trait to indicate that it is
>> allowed to obtain an `ARef<T>` from a `&T`, which cannot be allowed for
>> types which are also Ownable.
>
> The message needs a rationale for making the change rather than relying
> on the reader to deduce so.
>
> For example:
>
> 	There are types where it may both be referenced counted in some
> 	cases and owned in other. In such cases, obtaining `ARef<T>`
> 	from `&T` would be unsound as it allows creation of `ARef<T>`
> 	copy from `&Owned<T>`.
>
> 	Therefore, we split `AlwaysRefCounted` into `RefCounted` (which
> 	`ARef<T>` would require) and a marker trait to indicate that
> 	the type is always reference counted (and not `Ownable`) so the
> 	`&T` -> `ARef<T>` conversion is possible.

Thanks, I'll mix this in with the one I sent to Daniel.


Best regards,
Andreas Hindborg




^ permalink raw reply

* Re: [PATCH v4 00/17] module: Introduce hash-based integrity checking
From: David Howells @ 2026-02-02  9:21 UTC (permalink / raw)
  To: Eric Biggers
  Cc: dhowells, =?UTF-8?q?Mihai-Drosi=20C=C3=A2ju?=, linux, arnd,
	arnout, atomlin, bigeasy, chleroy, christian, corbet, coxu,
	da.gomez, da.gomez, dmitry.kasatkin, eric.snowberg,
	f.gruenbichler, jmorris, kpcyrd, linux-arch, linux-doc,
	linux-integrity, linux-kbuild, linux-kernel, linux-modules,
	linux-security-module, linuxppc-dev, lkp, maddy, mattia, mcgrof,
	mpe, nathan, naveen, nicolas.bouchinet, nicolas.schier, npiggin,
	nsc, paul, petr.pavlu, roberto.sassu, samitolvanen, serge,
	xiujianfeng, zohar
In-Reply-To: <20260201201218.GA15755@quark>

Eric Biggers <ebiggers@kernel.org> wrote:

> With that being the case, why is there still effort being put into
> adding more features to module signing?  I would think efforts should be
> focused on hash-based module authentication, i.e. this patchset.

Because it's not just signing of modules and it's not just modules built with
the kernel.  Also a hash table just of module hashes built into the core
kernel image will increase the size of the kernel by around a third of a meg
(on Fedora 43 and assuming SHA512) with uncompressible data.

David


^ permalink raw reply

* Re: [PATCH] xfrm: kill xfrm_dev_{state,policy}_flush_secctx_check()
From: Paul Moore @ 2026-02-02  4:07 UTC (permalink / raw)
  To: Tetsuo Handa
  Cc: SELinux, linux-security-module, Steffen Klassert, Herbert Xu,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, Network Development
In-Reply-To: <93d291db-4175-48c4-830c-e83bab373ae2@I-love.SAKURA.ne.jp>

On Sat, Jan 31, 2026 at 1:01 AM Tetsuo Handa
<penguin-kernel@i-love.sakura.ne.jp> wrote:
> On 2026/01/31 6:56, Paul Moore wrote:
> > On Wed, Jan 28, 2026 at 5:28 AM Tetsuo Handa
> > <penguin-kernel@i-love.sakura.ne.jp> wrote:
> >> On 2026/01/28 6:59, Paul Moore wrote:
> >>> It sounds like we either need to confirm that
> >>> security_xfrm_{policy,state}_delete() is already present in all code
> >>> paths that result in SPD/SAD deletions (in a place that can safely
> >>> fail and return an error),
> >>
> >> Yes.
> >
> > To clarify, do you mean "yes, I agree", or "yes, I've already checked
> > this and can confirm that the LSM hooks are already being called"?
>
> I mean "yes, I agree".
>
> >
> >>>                            or we need to place
> >>> xfrm_dev_{policy,state}_flush_secctx_check() in a location that can
> >>> safely fail.
> >>
> >> Did you mean xfrm_{policy,state}_flush_secctx_check() ?
> >
> > They both call into the security_xfrm_policy_delete() LSM hook which
> > is what I care about as that hook is what authorizes the operation.
>
> I can't understand what your authorization is.

I'm asking you to verify that we have the LSM xfrm hooks in all of the
necessary locations to ensure that we are safely and comprehensively
gating all of the operations that result in removal of SPD and SAD
entries.

> No authorization can be placed during must-not-fail operation.

Of course, but that means that we simply need to make sure we have the
authorization hooks placed elsewhere to ensure that users can not
remove SPD and SAD entries if they are not allowed.  I'm not arguing
about if returning an error in a place that can not handle an error
condition is correct or not, I'm arguing that you should audit the SPD
and SAD removal code paths to ensure that they all have the proper LSM
xfrm hook authorizations.

-- 
paul-moore.com

^ permalink raw reply

* Re: [PATCH v2 5/6] selftests/landlock: Repurpose scoped_abstract_unix_test.c for pathname sockets too.
From: Tingmao Wang @ 2026-02-02  0:06 UTC (permalink / raw)
  To: Mickaël Salaün
  Cc: Günther Noack, Demi Marie Obenour, Alyssa Ross, Jann Horn,
	Tahera Fahimi, Justin Suess, linux-security-module
In-Reply-To: <20260129.ePhaehieJoh4@digikod.net>

On 1/29/26 21:28, Mickaël Salaün wrote:
> [...]
> On Tue, Dec 30, 2025 at 05:20:23PM +0000, Tingmao Wang wrote:
>> [...]
>> @@ -308,6 +413,12 @@ TEST_F(scoped_audit, connect_to_child)
>>  	char buf;
>>  	int dgram_client;
>>  	struct audit_records records;
>> +	struct service_fixture *const dgram_address =
>> +		variant->abstract_socket ? &self->dgram_address_abstract :
>> +					   &self->dgram_address_pathname;
>> +	size_t log_match_remaining = 500;
> 
> const
> 
> Why this number?  Could you please follow the same logic as in
> matches_log_fs_extra()?

It's not a const since we decrement it below as we fill log_match with
stpncpy().

matches_log_fs_extra() uses 2*PATH_MAX + length of various fixed strings.
I guess since the path is a resolved absolute path which can vary based on
where this test is ran, it is safest to use a value based on PATH_MAX.  I
will update.

> 
>> +	char log_match[log_match_remaining];
>> +	char *log_match_cursor = log_match;
>>  
>>  	/* Makes sure there is no superfluous logged records. */
>>  	EXPECT_EQ(0, audit_count_records(self->audit_fd, &records));
>> @@ -330,8 +441,8 @@ TEST_F(scoped_audit, connect_to_child)
>>  
>>  		dgram_server = socket(AF_UNIX, SOCK_DGRAM, 0);
>>  		ASSERT_LE(0, dgram_server);
>> -		ASSERT_EQ(0, bind(dgram_server, &self->dgram_address.unix_addr,
>> -				  self->dgram_address.unix_addr_len));
>> +		ASSERT_EQ(0, bind(dgram_server, &dgram_address->unix_addr,
>> +				  dgram_address->unix_addr_len));
>>  
>>  		/* Signals to the parent that child is listening. */
>>  		ASSERT_EQ(1, write(pipe_child[1], ".", 1));
>> @@ -345,7 +456,9 @@ TEST_F(scoped_audit, connect_to_child)
>>  	EXPECT_EQ(0, close(pipe_child[1]));
>>  	EXPECT_EQ(0, close(pipe_parent[0]));
>>  
>> -	create_scoped_domain(_metadata, LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET);
>> +	create_scoped_domain(_metadata,
>> +			     LANDLOCK_SCOPE_ABSTRACT_UNIX_SOCKET |
>> +				     LANDLOCK_SCOPE_PATHNAME_UNIX_SOCKET);
>>  
>>  	/* Signals that the parent is in a domain, if any. */
>>  	ASSERT_EQ(1, write(pipe_parent[1], ".", 1));
>> @@ -355,19 +468,62 @@ TEST_F(scoped_audit, connect_to_child)
>>  
>>  	/* Waits for the child to listen */
>>  	ASSERT_EQ(1, read(pipe_child[0], &buf, 1));
>> -	err_dgram = connect(dgram_client, &self->dgram_address.unix_addr,
>> -			    self->dgram_address.unix_addr_len);
>> +	err_dgram = connect(dgram_client, &dgram_address->unix_addr,
>> +			    dgram_address->unix_addr_len);
>>  	EXPECT_EQ(-1, err_dgram);
>>  	EXPECT_EQ(EPERM, errno);
>>  
>> -	EXPECT_EQ(
>> -		0,
>> -		audit_match_record(
>> -			self->audit_fd, AUDIT_LANDLOCK_ACCESS,
>> +	if (variant->abstract_socket) {
>> +		log_match_cursor = stpncpy(
>> +			log_match,
>>  			REGEX_LANDLOCK_PREFIX
>>  			" blockers=scope\\.abstract_unix_socket path=" ABSTRACT_SOCKET_PATH_PREFIX
>>  			"[0-9A-F]\\+$",
>> -			NULL));
>> +			log_match_remaining);
>> +		log_match_remaining =
>> +			sizeof(log_match) - (log_match_cursor - log_match);
>> +		ASSERT_NE(0, log_match_remaining);
>> +	} else {
>> +		/*
>> +		 * It is assumed that absolute_path does not contain control
>> +		 * characters nor spaces, see audit_string_contains_control().
>> +		 */
>> +		char *absolute_path =
> 
> const char *absolute_path

Can't use const char * here since we free() it later:

scoped_unix_test.c:513:22: warning: passing argument 1 of ‘free’ discards ‘const’ qualifier from pointer target type [-Wdiscarded-qualifiers]
  513 |                 free(absolute_path);
      |                      ^~~~~~~~~~~~~

But I guess we can char *const.  Will update to:

		char *const absolute_path =
			realpath(dgram_address->unix_addr.sun_path, NULL);

> 
>> +			realpath(dgram_address->unix_addr.sun_path, NULL);
>> +
>> +		EXPECT_NE(NULL, absolute_path)
>> +		{
>> +			TH_LOG("realpath() failed: %s", strerror(errno));
>> +			return;
>> +		}
>> +
>> +		log_match_cursor =
>> +			stpncpy(log_match,
>> +				REGEX_LANDLOCK_PREFIX
>> +				" blockers=scope\\.pathname_unix_socket path=\"",
>> +				log_match_remaining);
>> +		log_match_remaining =
>> +			sizeof(log_match) - (log_match_cursor - log_match);
>> +		ASSERT_NE(0, log_match_remaining);
>> +		log_match_cursor = regex_escape(absolute_path, log_match_cursor,
>> +						log_match_remaining);
>> +		free(absolute_path);
>> +		if (log_match_cursor < 0) {
>> +			TH_LOG("regex_escape() failed (buffer too small)");
>> +			return;
>> +		}
>> +		log_match_remaining =
>> +			sizeof(log_match) - (log_match_cursor - log_match);
>> +		ASSERT_NE(0, log_match_remaining);
>> +		log_match_cursor =
>> +			stpncpy(log_match_cursor, "\"$", log_match_remaining);
>> +		log_match_remaining =
>> +			sizeof(log_match) - (log_match_cursor - log_match);
>> +		ASSERT_NE(0, log_match_remaining);
>> +	}
>> +
>> +	EXPECT_EQ(0, audit_match_record(self->audit_fd, AUDIT_LANDLOCK_ACCESS,
>> +					log_match, NULL));
>>  
>>  	ASSERT_EQ(1, write(pipe_parent[1], ".", 1));
>>  	EXPECT_EQ(0, close(dgram_client));
>> -- 
>> 2.52.0
>>
>>

^ permalink raw reply

* Re: [PATCH v6 6/6] docs: trusted-encryped: add PKWM as a new trust source
From: Jarkko Sakkinen @ 2026-02-01 22:29 UTC (permalink / raw)
  To: Srish Srinivasan
  Cc: linux-integrity, keyrings, linuxppc-dev, maddy, mpe, npiggin,
	christophe.leroy, James.Bottomley, zohar, nayna, rnsastry,
	linux-kernel, linux-security-module
In-Reply-To: <20260201135930.898721-7-ssrish@linux.ibm.com>

On Sun, Feb 01, 2026 at 07:29:30PM +0530, Srish Srinivasan wrote:
> From: Nayna Jain <nayna@linux.ibm.com>
> 
> Update Documentation/security/keys/trusted-encrypted.rst and Documentation/
> admin-guide/kernel-parameters.txt with PowerVM Key Wrapping Module (PKWM)
> as a new trust source
> 
> Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
> Signed-off-by: Srish Srinivasan <ssrish@linux.ibm.com>
> Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>

Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>

And you are free to take 5/6 and 6/6 to a pull request if you prefer
that route.

> ---
>  .../admin-guide/kernel-parameters.txt         |  1 +
>  .../security/keys/trusted-encrypted.rst       | 50 +++++++++++++++++++
>  2 files changed, 51 insertions(+)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 1058f2a6d6a8..aac15079b33d 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -7790,6 +7790,7 @@ Kernel parameters
>  			- "tee"
>  			- "caam"
>  			- "dcp"
> +			- "pkwm"
>  			If not specified then it defaults to iterating through
>  			the trust source list starting with TPM and assigns the
>  			first trust source as a backend which is initialized
> diff --git a/Documentation/security/keys/trusted-encrypted.rst b/Documentation/security/keys/trusted-encrypted.rst
> index eae6a36b1c9a..ddff7c7c2582 100644
> --- a/Documentation/security/keys/trusted-encrypted.rst
> +++ b/Documentation/security/keys/trusted-encrypted.rst
> @@ -81,6 +81,14 @@ safe.
>           and the UNIQUE key. Default is to use the UNIQUE key, but selecting
>           the OTP key can be done via a module parameter (dcp_use_otp_key).
>  
> +     (5) PKWM (PowerVM Key Wrapping Module: IBM PowerVM + Platform KeyStore)
> +
> +         Rooted to a unique, per-LPAR key, which is derived from a system-wide,
> +         randomly generated LPAR root key. Both the per-LPAR keys and the LPAR
> +         root key are stored in hypervisor-owned secure memory at runtime,
> +         and the LPAR root key is additionally persisted in secure locations
> +         such as the processor SEEPROMs and encrypted NVRAM.
> +
>    *  Execution isolation
>  
>       (1) TPM
> @@ -102,6 +110,14 @@ safe.
>           environment. Only basic blob key encryption is executed there.
>           The actual key sealing/unsealing is done on main processor/kernel space.
>  
> +     (5) PKWM (PowerVM Key Wrapping Module: IBM PowerVM + Platform KeyStore)
> +
> +         Fixed set of cryptographic operations done on on-chip hardware
> +         cryptographic acceleration unit NX. Keys for wrapping and unwrapping
> +         are managed by PowerVM Platform KeyStore, which stores keys in an
> +         isolated in-memory copy in secure hypervisor memory, as well as in a
> +         persistent copy in hypervisor-encrypted NVRAM.
> +
>    * Optional binding to platform integrity state
>  
>       (1) TPM
> @@ -129,6 +145,11 @@ safe.
>           Relies on Secure/Trusted boot process (called HAB by vendor) for
>           platform integrity.
>  
> +     (5) PKWM (PowerVM Key Wrapping Module: IBM PowerVM + Platform KeyStore)
> +
> +         Relies on secure and trusted boot process of IBM Power systems for
> +         platform integrity.
> +
>    *  Interfaces and APIs
>  
>       (1) TPM
> @@ -149,6 +170,11 @@ safe.
>           Vendor-specific API that is implemented as part of the DCP crypto driver in
>           ``drivers/crypto/mxs-dcp.c``.
>  
> +     (5) PKWM (PowerVM Key Wrapping Module: IBM PowerVM + Platform KeyStore)
> +
> +         Platform Keystore has well documented interfaces in PAPR document.
> +         Refer to ``Documentation/arch/powerpc/papr_hcalls.rst``
> +
>    *  Threat model
>  
>       The strength and appropriateness of a particular trust source for a given
> @@ -191,6 +217,10 @@ selected trust source:
>       a dedicated hardware RNG that is independent from DCP which can be enabled
>       to back the kernel RNG.
>  
> +   * PKWM (PowerVM Key Wrapping Module: IBM PowerVM + Platform KeyStore)
> +
> +     The normal kernel random number generator is used to generate keys.
> +
>  Users may override this by specifying ``trusted.rng=kernel`` on the kernel
>  command-line to override the used RNG with the kernel's random number pool.
>  
> @@ -321,6 +351,26 @@ Usage::
>  specific to this DCP key-blob implementation.  The key length for new keys is
>  always in bytes. Trusted Keys can be 32 - 128 bytes (256 - 1024 bits).
>  
> +Trusted Keys usage: PKWM
> +------------------------
> +
> +Usage::
> +
> +    keyctl add trusted name "new keylen [options]" ring
> +    keyctl add trusted name "load hex_blob" ring
> +    keyctl print keyid
> +
> +    options:
> +       wrap_flags=   ascii hex value of security policy requirement
> +                       0x00: no secure boot requirement (default)
> +                       0x01: require secure boot to be in either audit or
> +                             enforced mode
> +                       0x02: require secure boot to be in enforced mode
> +
> +"keyctl print" returns an ASCII hex copy of the sealed key, which is in format
> +specific to PKWM key-blob implementation.  The key length for new keys is
> +always in bytes. Trusted Keys can be 32 - 128 bytes (256 - 1024 bits).
> +
>  Encrypted Keys usage
>  --------------------
>  
> -- 
> 2.47.3
> 

BR, Jarkko

^ permalink raw reply

* Re: [PATCH v9 01/11] KEYS: trusted: Use get_random-fallback for TPM
From: Jarkko Sakkinen @ 2026-02-01 22:25 UTC (permalink / raw)
  To: Roberto Sassu
  Cc: linux-integrity, Eric Biggers, James Bottomley, Mimi Zohar,
	David Howells, Paul Moore, James Morris, Serge E. Hallyn,
	open list:KEYS-TRUSTED, open list:SECURITY SUBSYSTEM, open list
In-Reply-To: <facea3621fc240ebb05dedb0127d8a514970d40d.camel@huaweicloud.com>

On Thu, Jan 29, 2026 at 05:18:55PM +0100, Roberto Sassu wrote:
> On Sun, 2026-01-25 at 21:25 +0200, Jarkko Sakkinen wrote:
> > 1. tpm2_get_random() is costly when TCG_TPM2_HMAC is enabled and thus its
> >    use should be pooled rather than directly used. This both reduces
> >    latency and improves its predictability.
> > 
> > 2. Linux is better off overall if every subsystem uses the same source for
> >    generating the random numbers required.
> > 
> > Thus, unset '.get_random', which causes fallback to kernel_get_random().
> > 
> > One might argue that TPM RNG should be used for the generated trusted keys,
> > so that they have matching entropy with the TPM internally generated
> > objects.
> > 
> > This argument does have some weight into it but as far cryptography goes,
> > FIPS certification sets the exact bar, not which exact FIPS certified RNG
> > will be used. Thus, the rational choice is obviously to pick the lowest
> > latency path, which is kernel RNG.
> > 
> > Finally, there is an actual defence in depth benefit when using kernel RNG
> > as it helps to mitigate TPM firmware bugs concerning RNG implementation,
> > given the obfuscation by the other entropy sources.
> > 
> > Reviewed-by: Eric Biggers <ebiggers@kernel.org>
> > Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> > ---
> > v7:
> > - A new patch. Simplifies follow up patches.
> > ---
> >  security/keys/trusted-keys/trusted_tpm1.c | 16 ++++++++++------
> >  1 file changed, 10 insertions(+), 6 deletions(-)
> > 
> > diff --git a/security/keys/trusted-keys/trusted_tpm1.c b/security/keys/trusted-keys/trusted_tpm1.c
> > index 636acb66a4f6..7ce7e31bcdfb 100644
> > --- a/security/keys/trusted-keys/trusted_tpm1.c
> > +++ b/security/keys/trusted-keys/trusted_tpm1.c
> > @@ -6,6 +6,16 @@
> >   * See Documentation/security/keys/trusted-encrypted.rst
> >   */
> >  
> > +/**
> > + * DOC: Random Number Generation
> > + *
> > + * tpm_get_random() was previously used here as the RNG in order to have equal
> > + * entropy with the objects fully inside the TPM. However, as far as goes,
> > + * kernel RNG is equally fine, as long as long as it is FIPS certified. Also,
> > + * using kernel RNG has the benefit of mitigating bugs in the TPM firmware
> > + * associated with the RNG.
> > + */
> 
> If we switch to the kernel RNG that is better, and the TPM one is
> flawed, I guess we are going to have big problems anyway, since the TPM
> random number generator is used by the TPM itself internally.
> 
> I think it makes sense to leave as it is.

There's neither really formal case for not doing this unless the random
number provided by TPM would be opaque to kernel because as soon as CPU
touches it, the "risk" matches kernel RNG generated random number.

These change do have a measurable benefit as they  objectively decrease
TPM traffic.

And as we probably know, security certifications do not really apply
simply by using TPM RNG.

BR, Jarkko

^ permalink raw reply

* Re: [PATCH v4 00/17] module: Introduce hash-based integrity checking
From: Eric Biggers @ 2026-02-01 20:12 UTC (permalink / raw)
  To: David Howells
  Cc: =?UTF-8?q?Mihai-Drosi=20C=C3=A2ju?=, linux, arnd, arnout, atomlin,
	bigeasy, chleroy, christian, corbet, coxu, da.gomez, da.gomez,
	dmitry.kasatkin, eric.snowberg, f.gruenbichler, jmorris, kpcyrd,
	linux-arch, linux-doc, linux-integrity, linux-kbuild,
	linux-kernel, linux-modules, linux-security-module, linuxppc-dev,
	lkp, maddy, mattia, mcgrof, mpe, nathan, naveen,
	nicolas.bouchinet, nicolas.schier, npiggin, nsc, paul, petr.pavlu,
	roberto.sassu, samitolvanen, serge, xiujianfeng, zohar
In-Reply-To: <2316630.1769965788@warthog.procyon.org.uk>

On Sun, Feb 01, 2026 at 05:09:48PM +0000, David Howells wrote:
> Mihai-Drosi Câju <mcaju95@gmail.com> wrote:
> 
> > > The current signature-based module integrity checking has some drawbacks
> > in combination with reproducible builds. Either the module signing key
> > is generated at build time, which makes the build unreproducible, or a
> > static signing key is used, which precludes rebuilds by third parties
> > and makes the whole build and packaging process much more complicated.
> 
> There is another issue too: If you have a static private key that you use to
> sign modules (and probably other things), someone will likely give you a GPL
> request to get it.
> 
> One advantage of using a transient key every build and deleting it after is
> that no one has the key.
> 
> One other thing to remember: security is *meant* to get in the way.  That's
> the whole point of it.
> 
> However, IANAL.
> 
> David

It sounds like hash-based module authentication is just better, then.
If the full set of authentic modules is known at kernel build time, then
signatures are unnecessary to verify their authenticity: a list of
hashes built into the kernel image is perfectly sufficient.

(This patchset actually gets a little fancy and makes it a Merkle tree
root.  But it could be simplified to just a list of hashes.)

With that being the case, why is there still effort being put into
adding more features to module signing?  I would think efforts should be
focused on hash-based module authentication, i.e. this patchset.

- Eric

^ permalink raw reply

* Re: [PATCH v4 00/17] module: Introduce hash-based integrity checking
From: David Howells @ 2026-02-01 17:09 UTC (permalink / raw)
  To: =?UTF-8?q?Mihai-Drosi=20C=C3=A2ju?=
  Cc: dhowells, linux, arnd, arnout, atomlin, bigeasy, chleroy,
	christian, corbet, coxu, da.gomez, da.gomez, dmitry.kasatkin,
	eric.snowberg, f.gruenbichler, jmorris, kpcyrd, linux-arch,
	linux-doc, linux-integrity, linux-kbuild, linux-kernel,
	linux-modules, linux-security-module, linuxppc-dev, lkp, maddy,
	mattia, mcgrof, mpe, nathan, naveen, nicolas.bouchinet,
	nicolas.schier, npiggin, nsc, paul, petr.pavlu, roberto.sassu,
	samitolvanen, serge, xiujianfeng, zohar
In-Reply-To: <20260131073636.65494-1-mcaju95@gmail.com>

Mihai-Drosi Câju <mcaju95@gmail.com> wrote:

> > The current signature-based module integrity checking has some drawbacks
> in combination with reproducible builds. Either the module signing key
> is generated at build time, which makes the build unreproducible, or a
> static signing key is used, which precludes rebuilds by third parties
> and makes the whole build and packaging process much more complicated.

There is another issue too: If you have a static private key that you use to
sign modules (and probably other things), someone will likely give you a GPL
request to get it.

One advantage of using a transient key every build and deleting it after is
that no one has the key.

One other thing to remember: security is *meant* to get in the way.  That's
the whole point of it.

However, IANAL.

David


^ permalink raw reply

* Re: [PATCH v4 00/17] module: Introduce hash-based integrity checking
From: Thomas Weißschuh @ 2026-02-01 16:22 UTC (permalink / raw)
  To: Mihai-Drosi Câju
  Cc: arnd, arnout, atomlin, bigeasy, chleroy, christian, corbet, coxu,
	da.gomez, da.gomez, dmitry.kasatkin, eric.snowberg,
	f.gruenbichler, jmorris, kpcyrd, linux-arch, linux-doc,
	linux-integrity, linux-kbuild, linux-kernel, linux-modules,
	linux-security-module, linuxppc-dev, lkp, maddy, mattia, mcgrof,
	mpe, nathan, naveen, nicolas.bouchinet, nicolas.schier, npiggin,
	nsc, paul, petr.pavlu, roberto.sassu, samitolvanen, serge,
	xiujianfeng, zohar
In-Reply-To: <20260131073636.65494-1-mcaju95@gmail.com>

Hi Mihai-Drosi,

thanks for taking an interest into these patches!

On 2026-01-31 09:36:36+0200, Mihai-Drosi Câju wrote:
> > The current signature-based module integrity checking has some drawbacks
> > in combination with reproducible builds. Either the module signing key
> > is generated at build time, which makes the build unreproducible, or a
> > static signing key is used, which precludes rebuilds by third parties
> > and makes the whole build and packaging process much more complicated.
> 
> I think there is a middle ground where the module signing key is generated
> using a key derivation function that has as an input a deterministic value
> on the build host, such as /etc/machine-id . The problem with this approach
> is that only hosts knowing the value will be able to reproduce the build.

The goal is to make the distro kernel packages rebuildable by the
general public. Any involvement of secret values will break this goal.

> Maybe this is a solution to NixOS secret management? Introduce minimal
> impurity as a cryptographic seed and derive the rest of the secrets using
> something like Argon2(seed, key_uuid).

I am not familiar with NixOS and its secret management.
This patchset serves a wider audience.

> There might be another approach to code integrity rather than step-by-step
> reproducibility. One may exploit the very cryptographic primitives that make
> reproducibility hard to ensure that reproducibility is most  likely valid.
> 
> For example, the module signing issue, the build host publishes four artifacts:
> * The source-code
> * The compiled and signed binary
> * The build environment
> * Its public key
> 
> Now, we don't need to sign with the private key to know that building the source
> code using the specific build environment and signing the result with the private
> key will result in the claimed binary. We can just compile and verify with the
> public key.

This could work if the goal is only to verify the reproducibility of a
single, signed-en-bloc artifact. But we also need to handle vmlinux which
contains the corresponding public key. It would need different handling.
We can add some special logic to strip that public key before
comparision. But then vmlinux might be compressed or wrapped in some
other format. Another whole collection of special logic.

(...)


Thomas

^ permalink raw reply

* Re: [RFC PATCH v3 6/8] selftests/landlock: Add tests for UDP sendmsg
From: Tingmao Wang @ 2026-02-01 16:19 UTC (permalink / raw)
  To: Matthieu Buffet
  Cc: Mickaël Salaün, Günther Noack,
	linux-security-module, Mikhail Ivanov, konstantin.meskhidze,
	netdev
In-Reply-To: <20251212163704.142301-7-matthieu@buffet.re>

Hi Matthieu,

I noticed in passing that

On 12/12/25 16:37, Matthieu Buffet wrote:
> [...]
> +	EXPECT_EQ(-EACCES, sendto_variant(sock_fd, &self->srv0, "A", 1, 0));
> +	EXPECT_EQ(0, matches_log_prot(self->audit_fd, "net.sendto_udp", "daddr",
> +				      variant->addr, "dest"));

net.sendto_udp should probably be net\.sendto_udp

> +
> +	EXPECT_EQ(0, audit_count_records(self->audit_fd, &records));
> +	EXPECT_EQ(0, records.access);
> +	EXPECT_EQ(1, records.domain);
> +
> +	EXPECT_EQ(-EACCES, sendto_variant(sock_fd, &self->unspec_srv0, "B", 1, 0));
> +	EXPECT_EQ(0, matches_log_prot(self->audit_fd, "net.sendto_udp", "daddr",
> +				      NULL, "dest"));

and same here

> +
> +	EXPECT_EQ(0, audit_count_records(self->audit_fd, &records));
> +	EXPECT_EQ(0, records.access);
> +	EXPECT_EQ(0, records.domain);
> +
> +	EXPECT_EQ(0, close(sock_fd));
> +}
> +
>  TEST_HARNESS_MAIN

^ permalink raw reply

* Re: [PATCH v6 0/6] Extend "trusted" keys to support a new trust source named the PowerVM Key Wrapping Module (PKWM)
From: Srish Srinivasan @ 2026-02-01 15:19 UTC (permalink / raw)
  To: linux-integrity, keyrings, linuxppc-dev
  Cc: maddy, mpe, npiggin, christophe.leroy, James.Bottomley, jarkko,
	zohar, nayna, rnsastry, linux-kernel, linux-security-module
In-Reply-To: <20260201135930.898721-1-ssrish@linux.ibm.com>

Please ignore this series (v6).

thanks,
Srish.

On 2/1/26 7:29 PM, Srish Srinivasan wrote:
> Power11 has introduced a feature called the PowerVM Key Wrapping Module
> (PKWM), where PowerVM in combination with Power LPAR Platform KeyStore
> (PLPKS) [1] supports a new feature called "Key Wrapping" [2] to protect
> user secrets by wrapping them using a hypervisor generated wrapping key.
> This wrapping key is an AES-GCM-256 symmetric key that is stored as an
> object in the PLPKS. It has policy based protections that prevents it from
> being read out or exposed to the user. This wrapping key can then be used
> by the OS to wrap or unwrap secrets via hypervisor calls.
>
> This patchset intends to add the PKWM, which is a combination of IBM
> PowerVM and PLPKS, as a new trust source for trusted keys. The wrapping key
> does not exist by default and its generation is requested by the kernel at
> the time of PKWM initialization. This key is then persisted by the PKWM and
> is used for wrapping any kernel provided key, and is never exposed to the
> user. The kernel is aware of only the label to this wrapping key.
>
> Along with the PKWM implementation, this patchset includes two preparatory
> patches: one fixing the kernel-doc inconsistencies in the PLPKS code and
> another reorganizing PLPKS config variables in the sysfs.
>
> Changelog:
>
> v6:
>
> * Patch 1 to Patch 3:
>    - Add Nayna's Tested-by tag
> * Patch 4
>    - Fix build error reported by kernel test robot <lkp@intel.com>
>    - Add Nayna's Tested-by tag
> * Patch 5
>    - Add Nayna's Tested-by tag
>
> v5:
>
> * Patch 1 to Patch 3:
>    - Add Nayna's Reviewed-by tag
> * Patch 4:
>    - Fix build error identified by chleroy@kernel.org
>    - Add Nayna's Reviewed-by tag
> * Patch 5:
>    - Add Reviewed-by tags from Nayna and Jarkko
>
> v4:
>
> * Patch 5:
>    - Add a per-backend private data pointer in trusted_key_options
>      to store a pointer to the backend-specific options structure
>    - Minor clean-up
>
> v3:
>
> * Patch 2:
>    - Add Mimi's Reviewed-by tag
> * Patch 4:
>    - Minor tweaks to some print statements
>    - Fix typos
> * Patch 5:
>    - Fix typos
>    - Add Mimi's Reviewed-by tag
> * Patch 6:
>    - Add Mimi's Reviewed-by tag
>
> v2:
>
> * Patch 2:
>    - Fix build warning detected by the kernel test bot
> * Patch 5:
>    - Use pr_debug inside dump_options
>    - Replace policyhande with wrap_flags inside dump_options
>    - Provide meaningful error messages with error codes
>
> Nayna Jain (1):
>    docs: trusted-encryped: add PKWM as a new trust source
>
> Srish Srinivasan (5):
>    pseries/plpks: fix kernel-doc comment inconsistencies
>    powerpc/pseries: move the PLPKS config inside its own sysfs directory
>    pseries/plpks: expose PowerVM wrapping features via the sysfs
>    pseries/plpks: add HCALLs for PowerVM Key Wrapping Module
>    keys/trusted_keys: establish PKWM as a trusted source
>
>   .../ABI/testing/sysfs-firmware-plpks          |  58 ++
>   Documentation/ABI/testing/sysfs-secvar        |  65 --
>   .../admin-guide/kernel-parameters.txt         |   1 +
>   Documentation/arch/powerpc/papr_hcalls.rst    |  43 ++
>   .../security/keys/trusted-encrypted.rst       |  50 ++
>   MAINTAINERS                                   |   9 +
>   arch/powerpc/include/asm/hvcall.h             |   4 +-
>   arch/powerpc/include/asm/plpks.h              |  95 +--
>   arch/powerpc/include/asm/secvar.h             |   1 -
>   arch/powerpc/kernel/secvar-sysfs.c            |  21 +-
>   arch/powerpc/platforms/pseries/Makefile       |   2 +-
>   arch/powerpc/platforms/pseries/plpks-secvar.c |  29 -
>   arch/powerpc/platforms/pseries/plpks-sysfs.c  |  96 +++
>   arch/powerpc/platforms/pseries/plpks.c        | 688 +++++++++++++++++-
>   include/keys/trusted-type.h                   |   7 +-
>   include/keys/trusted_pkwm.h                   |  33 +
>   security/keys/trusted-keys/Kconfig            |   8 +
>   security/keys/trusted-keys/Makefile           |   2 +
>   security/keys/trusted-keys/trusted_core.c     |   6 +-
>   security/keys/trusted-keys/trusted_pkwm.c     | 190 +++++
>   20 files changed, 1207 insertions(+), 201 deletions(-)
>   create mode 100644 Documentation/ABI/testing/sysfs-firmware-plpks
>   create mode 100644 arch/powerpc/platforms/pseries/plpks-sysfs.c
>   create mode 100644 include/keys/trusted_pkwm.h
>   create mode 100644 security/keys/trusted-keys/trusted_pkwm.c
>

^ permalink raw reply

* [PATCH v6 4/6] pseries/plpks: add HCALLs for PowerVM Key Wrapping Module
From: Srish Srinivasan @ 2026-02-01 13:59 UTC (permalink / raw)
  To: linux-integrity, keyrings, linuxppc-dev
  Cc: maddy, mpe, npiggin, christophe.leroy, James.Bottomley, jarkko,
	zohar, nayna, rnsastry, linux-kernel, linux-security-module,
	ssrish
In-Reply-To: <20260201135930.898721-1-ssrish@linux.ibm.com>

The hypervisor generated wrapping key is an AES-GCM-256 symmetric key which
is stored in a non-volatile, secure, and encrypted storage called the Power
LPAR Platform KeyStore. It has policy based protections that prevent it
from being read out or exposed to the user.

Implement H_PKS_GEN_KEY, H_PKS_WRAP_OBJECT, and H_PKS_UNWRAP_OBJECT HCALLs
to enable using the PowerVM Key Wrapping Module (PKWM) as a new trust
source for trusted keys. Disallow H_PKS_READ_OBJECT, H_PKS_SIGNED_UPDATE,
and H_PKS_WRITE_OBJECT for objects with the 'wrapping key' policy set.
Capture the availability status for the H_PKS_WRAP_OBJECT interface.

Signed-off-by: Srish Srinivasan <ssrish@linux.ibm.com>
Reviewed-by: Nayna Jain <nayna@linux.ibm.com>
Tested-by: Nayna Jain <nayna@linux.ibm.com>
---
 Documentation/arch/powerpc/papr_hcalls.rst |  43 +++
 arch/powerpc/include/asm/plpks.h           |  10 +
 arch/powerpc/platforms/pseries/plpks.c     | 344 ++++++++++++++++++++-
 3 files changed, 395 insertions(+), 2 deletions(-)

diff --git a/Documentation/arch/powerpc/papr_hcalls.rst b/Documentation/arch/powerpc/papr_hcalls.rst
index 805e1cb9bab9..14e39f095a1c 100644
--- a/Documentation/arch/powerpc/papr_hcalls.rst
+++ b/Documentation/arch/powerpc/papr_hcalls.rst
@@ -300,6 +300,49 @@ H_HTM supports setup, configuration, control and dumping of Hardware Trace
 Macro (HTM) function and its data. HTM buffer stores tracing data for functions
 like core instruction, core LLAT and nest.
 
+**H_PKS_GEN_KEY**
+
+| Input: authorization, objectlabel, objectlabellen, policy, out, outlen
+| Out: *Hypervisor Generated Key, or None when the wrapping key policy is set*
+| Return Value: *H_SUCCESS, H_Function, H_State, H_R_State, H_Parameter, H_P2,
+                H_P3, H_P4, H_P5, H_P6, H_Authority, H_Nomem, H_Busy, H_Resource,
+                H_Aborted*
+
+H_PKS_GEN_KEY is used to have the hypervisor generate a new random key.
+This key is stored as an object in the Power LPAR Platform KeyStore with
+the provided object label. With the wrapping key policy set the key is only
+visible to the hypervisor, while the key's label would still be visible to
+the user. Generation of wrapping keys is supported only for a key size of
+32 bytes.
+
+**H_PKS_WRAP_OBJECT**
+
+| Input: authorization, wrapkeylabel, wrapkeylabellen, objectwrapflags, in,
+|        inlen, out, outlen, continue-token
+| Out: *continue-token, byte size of wrapped object, wrapped object*
+| Return Value: *H_SUCCESS, H_Function, H_State, H_R_State, H_Parameter, H_P2,
+                H_P3, H_P4, H_P5, H_P6, H_P7, H_P8, H_P9, H_Authority, H_Invalid_Key,
+                H_NOT_FOUND, H_Busy, H_LongBusy, H_Aborted*
+
+H_PKS_WRAP_OBJECT is used to wrap an object using a wrapping key stored in the
+Power LPAR Platform KeyStore and return the wrapped object to the caller. The
+caller provides a label to a wrapping key with the 'wrapping key' policy set,
+which must have been previously created with H_PKS_GEN_KEY. The provided object
+is then encrypted with the wrapping key and additional metadata and the result
+is returned to the caller.
+
+
+**H_PKS_UNWRAP_OBJECT**
+
+| Input: authorization, objectwrapflags, in, inlen, out, outlen, continue-token
+| Out: *continue-token, byte size of unwrapped object, unwrapped object*
+| Return Value: *H_SUCCESS, H_Function, H_State, H_R_State, H_Parameter, H_P2,
+                H_P3, H_P4, H_P5, H_P6, H_P7, H_Authority, H_Unsupported, H_Bad_Data,
+                H_NOT_FOUND, H_Invalid_Key, H_Busy, H_LongBusy, H_Aborted*
+
+H_PKS_UNWRAP_OBJECT is used to unwrap an object that was previously warapped with
+H_PKS_WRAP_OBJECT.
+
 References
 ==========
 .. [1] "Power Architecture Platform Reference"
diff --git a/arch/powerpc/include/asm/plpks.h b/arch/powerpc/include/asm/plpks.h
index 8f034588fdf7..e87f90e40d4e 100644
--- a/arch/powerpc/include/asm/plpks.h
+++ b/arch/powerpc/include/asm/plpks.h
@@ -113,6 +113,16 @@ void plpks_early_init_devtree(void);
 int plpks_populate_fdt(void *fdt);
 
 int plpks_config_create_softlink(struct kobject *from);
+
+bool plpks_wrapping_is_supported(void);
+
+int plpks_gen_wrapping_key(void);
+
+int plpks_wrap_object(u8 **input_buf, u32 input_len, u16 wrap_flags,
+		      u8 **output_buf, u32 *output_len);
+
+int plpks_unwrap_object(u8 **input_buf, u32 input_len,
+			u8 **output_buf, u32 *output_len);
 #else // CONFIG_PSERIES_PLPKS
 static inline bool plpks_is_available(void) { return false; }
 static inline u16 plpks_get_passwordlen(void) { BUILD_BUG(); }
diff --git a/arch/powerpc/platforms/pseries/plpks.c b/arch/powerpc/platforms/pseries/plpks.c
index 4a08f51537c8..23e4e2a922fc 100644
--- a/arch/powerpc/platforms/pseries/plpks.c
+++ b/arch/powerpc/platforms/pseries/plpks.c
@@ -9,6 +9,32 @@
 
 #define pr_fmt(fmt) "plpks: " fmt
 
+#define PLPKS_WRAPKEY_COMPONENT	"PLPKSWR"
+#define PLPKS_WRAPKEY_NAME	"default-wrapping-key"
+
+/*
+ * To 4K align the {input, output} buffers to the {UN}WRAP H_CALLs
+ */
+#define PLPKS_WRAPPING_BUF_ALIGN	4096
+
+/*
+ * To ensure the output buffer's length is at least 1024 bytes greater
+ * than the input buffer's length during the WRAP H_CALL
+ */
+#define PLPKS_WRAPPING_BUF_DIFF	1024
+
+#define PLPKS_WRAP_INTERFACE_BIT	3
+#define PLPKS_WRAPPING_KEY_LENGTH	32
+
+#define WRAPFLAG_BE_BIT_SET(be_bit) \
+	BIT_ULL(63 - (be_bit))
+
+#define WRAPFLAG_BE_GENMASK(be_bit_hi, be_bit_lo) \
+	GENMASK_ULL(63 - (be_bit_hi), 63 - (be_bit_lo))
+
+#define WRAPFLAG_BE_FIELD_PREP(be_bit_hi, be_bit_lo, val) \
+	FIELD_PREP(WRAPFLAG_BE_GENMASK(be_bit_hi, be_bit_lo), (val))
+
 #include <linux/delay.h>
 #include <linux/errno.h>
 #include <linux/io.h>
@@ -19,6 +45,7 @@
 #include <linux/of_fdt.h>
 #include <linux/libfdt.h>
 #include <linux/memblock.h>
+#include <linux/bitfield.h>
 #include <asm/hvcall.h>
 #include <asm/machdep.h>
 #include <asm/plpks.h>
@@ -39,6 +66,7 @@ static u32 supportedpolicies;
 static u32 maxlargeobjectsize;
 static u64 signedupdatealgorithms;
 static u64 wrappingfeatures;
+static bool wrapsupport;
 
 struct plpks_auth {
 	u8 version;
@@ -283,6 +311,7 @@ static int _plpks_get_config(void)
 	maxlargeobjectsize = be32_to_cpu(config->maxlargeobjectsize);
 	signedupdatealgorithms = be64_to_cpu(config->signedupdatealgorithms);
 	wrappingfeatures = be64_to_cpu(config->wrappingfeatures);
+	wrapsupport = config->flags & PPC_BIT8(PLPKS_WRAP_INTERFACE_BIT);
 
 	// Validate that the numbers we get back match the requirements of the spec
 	if (maxpwsize < 32) {
@@ -614,6 +643,9 @@ int plpks_signed_update_var(struct plpks_var *var, u64 flags)
 	if (!(var->policy & PLPKS_SIGNEDUPDATE))
 		return -EINVAL;
 
+	if (var->policy & PLPKS_WRAPPINGKEY)
+		return -EINVAL;
+
 	// Signed updates need the component to be NULL.
 	if (var->component)
 		return -EINVAL;
@@ -696,6 +728,9 @@ int plpks_write_var(struct plpks_var var)
 	if (var.policy & PLPKS_SIGNEDUPDATE)
 		return -EINVAL;
 
+	if (var.policy & PLPKS_WRAPPINGKEY)
+		return -EINVAL;
+
 	auth = construct_auth(PLPKS_OS_OWNER);
 	if (IS_ERR(auth))
 		return PTR_ERR(auth);
@@ -790,6 +825,9 @@ static int plpks_read_var(u8 consumer, struct plpks_var *var)
 	if (var->namelen > PLPKS_MAX_NAME_SIZE)
 		return -EINVAL;
 
+	if (var->policy & PLPKS_WRAPPINGKEY)
+		return -EINVAL;
+
 	auth = construct_auth(consumer);
 	if (IS_ERR(auth))
 		return PTR_ERR(auth);
@@ -845,8 +883,310 @@ static int plpks_read_var(u8 consumer, struct plpks_var *var)
 }
 
 /**
- * plpks_read_os_var() - Fetch the data for the specified variable that is
- * owned by the OS consumer.
+ * plpks_wrapping_is_supported() - Get the H_PKS_WRAP_OBJECT interface
+ * availability status for the LPAR.
+ *
+ * Successful execution of the H_PKS_GET_CONFIG HCALL during initialization
+ * sets bit 3 of the flags variable in the PLPKS config structure if the
+ * H_PKS_WRAP_OBJECT interface is supported.
+ *
+ * Returns: true if the H_PKS_WRAP_OBJECT interface is supported, false if not.
+ */
+bool plpks_wrapping_is_supported(void)
+{
+	return wrapsupport;
+}
+EXPORT_SYMBOL_GPL(plpks_wrapping_is_supported);
+
+/**
+ * plpks_gen_wrapping_key() - Generate a new random key with the 'wrapping key'
+ * policy set.
+ *
+ * The H_PKS_GEN_KEY HCALL makes the hypervisor generate a new random key and
+ * store the key in a PLPKS object with the provided object label. With the
+ * 'wrapping key' policy set, only the label to the newly generated random key
+ * would be visible to the user.
+ *
+ * Possible reasons for the returned errno values:
+ *
+ * -ENXIO	if PLPKS is not supported
+ * -EIO		if PLPKS access is blocked due to the LPAR's state
+ *		if PLPKS modification is blocked due to the LPAR's state
+ *		if an error occurred while processing the request
+ * -EINVAL	if invalid authorization parameter
+ *		if invalid object label parameter
+ *		if invalid object label len parameter
+ *		if invalid or unsupported policy declaration
+ *		if invalid output buffer parameter
+ *		if invalid output buffer length parameter
+ * -EPERM	if access is denied
+ * -ENOMEM	if there is inadequate memory to perform this operation
+ * -EBUSY	if unable to handle the request
+ * -EEXIST	if the object label already exists
+ *
+ * Returns: On success 0 is returned, a negative errno if not.
+ */
+int plpks_gen_wrapping_key(void)
+{
+	unsigned long retbuf[PLPAR_HCALL_BUFSIZE] = { 0 };
+	struct plpks_auth *auth;
+	struct label *label;
+	int rc = 0, pseries_status = 0;
+	struct plpks_var var = {
+		.name = PLPKS_WRAPKEY_NAME,
+		.namelen = strlen(var.name),
+		.policy = PLPKS_WRAPPINGKEY,
+		.os = PLPKS_VAR_LINUX,
+		.component = PLPKS_WRAPKEY_COMPONENT
+	};
+
+	auth = construct_auth(PLPKS_OS_OWNER);
+	if (IS_ERR(auth))
+		return PTR_ERR(auth);
+
+	label = construct_label(var.component, var.os, var.name, var.namelen);
+	if (IS_ERR(label)) {
+		rc = PTR_ERR(label);
+		goto out;
+	}
+
+	rc = plpar_hcall(H_PKS_GEN_KEY, retbuf,
+			 virt_to_phys(auth), virt_to_phys(label),
+			 label->size, var.policy,
+			 NULL, PLPKS_WRAPPING_KEY_LENGTH);
+
+	if (!rc)
+		rc = plpks_confirm_object_flushed(label, auth);
+
+	pseries_status = rc;
+	rc = pseries_status_to_err(rc);
+
+	if (rc && rc != -EEXIST) {
+		pr_err("H_PKS_GEN_KEY failed. pseries_status=%d, rc=%d",
+		       pseries_status, rc);
+	} else {
+		rc = 0;
+	}
+
+	kfree(label);
+out:
+	kfree(auth);
+	return rc;
+}
+EXPORT_SYMBOL_GPL(plpks_gen_wrapping_key);
+
+/**
+ * plpks_wrap_object() - Wrap an object using the default wrapping key stored in
+ * the PLPKS.
+ * @input_buf: buffer containing the data to be wrapped
+ * @input_len: length of the input buffer
+ * @wrap_flags: object wrapping flags
+ * @output_buf: buffer to store the wrapped data
+ * @output_len: length of the output buffer
+ *
+ * The H_PKS_WRAP_OBJECT HCALL wraps an object using a wrapping key stored in
+ * the PLPKS and returns the wrapped object to the caller. The caller provides a
+ * label to the wrapping key with the 'wrapping key' policy set that must have
+ * been previously created with the H_PKS_GEN_KEY HCALL. The provided object is
+ * then encrypted with the wrapping key and additional metadata and the result
+ * is returned to the user. The metadata includes the wrapping algorithm and the
+ * wrapping key name so those parameters are not required during unwrap.
+ *
+ * Possible reasons for the returned errno values:
+ *
+ * -ENXIO	if PLPKS is not supported
+ * -EIO		if PLPKS access is blocked due to the LPAR's state
+ *		if PLPKS modification is blocked due to the LPAR's state
+ *		if an error occurred while processing the request
+ * -EINVAL	if invalid authorization parameter
+ *		if invalid wrapping key label parameter
+ *		if invalid wrapping key label length parameter
+ *		if invalid or unsupported object wrapping flags
+ *		if invalid input buffer parameter
+ *		if invalid input buffer length parameter
+ *		if invalid output buffer parameter
+ *		if invalid output buffer length parameter
+ *		if invalid continue token parameter
+ *		if the wrapping key is not compatible with the wrapping
+ *		algorithm
+ * -EPERM	if access is denied
+ * -ENOENT	if the requested wrapping key was not found
+ * -EBUSY	if unable to handle the request or long running operation
+ *		initiated, retry later.
+ *
+ * Returns: On success 0 is returned, a negative errno if not.
+ */
+int plpks_wrap_object(u8 **input_buf, u32 input_len, u16 wrap_flags,
+		      u8 **output_buf, u32 *output_len)
+{
+	unsigned long retbuf[PLPAR_HCALL9_BUFSIZE] = { 0 };
+	struct plpks_auth *auth;
+	struct label *label;
+	u64 continuetoken = 0;
+	u64 objwrapflags = 0;
+	int rc = 0, pseries_status = 0;
+	bool sb_audit_or_enforce_bit = wrap_flags & BIT(0);
+	bool sb_enforce_bit = wrap_flags & BIT(1);
+	struct plpks_var var = {
+		.name = PLPKS_WRAPKEY_NAME,
+		.namelen = strlen(var.name),
+		.os = PLPKS_VAR_LINUX,
+		.component = PLPKS_WRAPKEY_COMPONENT
+	};
+
+	auth = construct_auth(PLPKS_OS_OWNER);
+	if (IS_ERR(auth))
+		return PTR_ERR(auth);
+
+	label = construct_label(var.component, var.os, var.name, var.namelen);
+	if (IS_ERR(label)) {
+		rc = PTR_ERR(label);
+		goto out;
+	}
+
+	/* Set the consumer password requirement bit. A must have. */
+	objwrapflags |= WRAPFLAG_BE_BIT_SET(3);
+
+	/* Set the wrapping algorithm bit. Just one algorithm option for now */
+	objwrapflags |= WRAPFLAG_BE_FIELD_PREP(60, 63, 0x1);
+
+	if (sb_audit_or_enforce_bit & sb_enforce_bit) {
+		pr_err("Cannot set both audit/enforce and enforce bits.");
+		rc = -EINVAL;
+		goto out_free_label;
+	} else if (sb_audit_or_enforce_bit) {
+		objwrapflags |= WRAPFLAG_BE_BIT_SET(1);
+	} else if (sb_enforce_bit) {
+		objwrapflags |= WRAPFLAG_BE_BIT_SET(2);
+	}
+
+	*output_len = input_len + PLPKS_WRAPPING_BUF_DIFF;
+
+	*output_buf = kzalloc(ALIGN(*output_len, PLPKS_WRAPPING_BUF_ALIGN),
+			      GFP_KERNEL);
+	if (!(*output_buf)) {
+		pr_err("Output buffer allocation failed. Returning -ENOMEM.");
+		rc = -ENOMEM;
+		goto out_free_label;
+	}
+
+	do {
+		rc = plpar_hcall9(H_PKS_WRAP_OBJECT, retbuf,
+				  virt_to_phys(auth), virt_to_phys(label),
+				  label->size, objwrapflags,
+				  virt_to_phys(*input_buf), input_len,
+				  virt_to_phys(*output_buf), *output_len,
+				  continuetoken);
+
+		continuetoken = retbuf[0];
+		pseries_status = rc;
+		rc = pseries_status_to_err(rc);
+	} while (rc == -EBUSY);
+
+	if (rc) {
+		pr_err("H_PKS_WRAP_OBJECT failed. pseries_status=%d, rc=%d",
+		       pseries_status, rc);
+		kfree(*output_buf);
+		*output_buf = NULL;
+	} else {
+		*output_len = retbuf[1];
+	}
+
+out_free_label:
+	kfree(label);
+out:
+	kfree(auth);
+	return rc;
+}
+EXPORT_SYMBOL_GPL(plpks_wrap_object);
+
+/**
+ * plpks_unwrap_object() - Unwrap an object using the default wrapping key
+ * stored in the PLPKS.
+ * @input_buf: buffer containing the data to be unwrapped
+ * @input_len: length of the input buffer
+ * @output_buf: buffer to store the unwrapped data
+ * @output_len: length of the output buffer
+ *
+ * The H_PKS_UNWRAP_OBJECT HCALL unwraps an object that was previously wrapped
+ * using the H_PKS_WRAP_OBJECT HCALL.
+ *
+ * Possible reasons for the returned errno values:
+ *
+ * -ENXIO	if PLPKS is not supported
+ * -EIO		if PLPKS access is blocked due to the LPAR's state
+ *		if PLPKS modification is blocked due to the LPAR's state
+ *		if an error occurred while processing the request
+ * -EINVAL	if invalid authorization parameter
+ *		if invalid or unsupported object unwrapping flags
+ *		if invalid input buffer parameter
+ *		if invalid input buffer length parameter
+ *		if invalid output buffer parameter
+ *		if invalid output buffer length parameter
+ *		if invalid continue token parameter
+ *		if the wrapping key is not compatible with the wrapping
+ *		algorithm
+ *		if the wrapped object's format is not supported
+ *		if the wrapped object is invalid
+ * -EPERM	if access is denied
+ * -ENOENT	if the wrapping key for the provided object was not found
+ * -EBUSY	if unable to handle the request or long running operation
+ *		initiated, retry later.
+ *
+ * Returns: On success 0 is returned, a negative errno if not.
+ */
+int plpks_unwrap_object(u8 **input_buf, u32 input_len, u8 **output_buf,
+			u32 *output_len)
+{
+	unsigned long retbuf[PLPAR_HCALL9_BUFSIZE] = { 0 };
+	struct plpks_auth *auth;
+	u64 continuetoken = 0;
+	u64 objwrapflags = 0;
+	int rc = 0, pseries_status = 0;
+
+	auth = construct_auth(PLPKS_OS_OWNER);
+	if (IS_ERR(auth))
+		return PTR_ERR(auth);
+
+	*output_len = input_len - PLPKS_WRAPPING_BUF_DIFF;
+	*output_buf = kzalloc(ALIGN(*output_len, PLPKS_WRAPPING_BUF_ALIGN),
+			      GFP_KERNEL);
+	if (!(*output_buf)) {
+		pr_err("Output buffer allocation failed. Returning -ENOMEM.");
+		rc = -ENOMEM;
+		goto out;
+	}
+
+	do {
+		rc = plpar_hcall9(H_PKS_UNWRAP_OBJECT, retbuf,
+				  virt_to_phys(auth), objwrapflags,
+				  virt_to_phys(*input_buf), input_len,
+				  virt_to_phys(*output_buf), *output_len,
+				  continuetoken);
+
+		continuetoken = retbuf[0];
+		pseries_status = rc;
+		rc = pseries_status_to_err(rc);
+	} while (rc == -EBUSY);
+
+	if (rc) {
+		pr_err("H_PKS_UNWRAP_OBJECT failed. pseries_status=%d, rc=%d",
+		       pseries_status, rc);
+		kfree(*output_buf);
+		*output_buf = NULL;
+	} else {
+		*output_len = retbuf[1];
+	}
+
+out:
+	kfree(auth);
+	return rc;
+}
+EXPORT_SYMBOL_GPL(plpks_unwrap_object);
+
+/**
+ * plpks_read_os_var() - Fetch the data for the specified variable that is owned
+ * by the OS consumer.
  * @var: variable to be read from the PLPKS
  *
  * The consumer or the owner of the object is the os kernel. The
-- 
2.47.3


^ permalink raw reply related

* [PATCH v6 6/6] docs: trusted-encryped: add PKWM as a new trust source
From: Srish Srinivasan @ 2026-02-01 13:59 UTC (permalink / raw)
  To: linux-integrity, keyrings, linuxppc-dev
  Cc: maddy, mpe, npiggin, christophe.leroy, James.Bottomley, jarkko,
	zohar, nayna, rnsastry, linux-kernel, linux-security-module,
	ssrish
In-Reply-To: <20260201135930.898721-1-ssrish@linux.ibm.com>

From: Nayna Jain <nayna@linux.ibm.com>

Update Documentation/security/keys/trusted-encrypted.rst and Documentation/
admin-guide/kernel-parameters.txt with PowerVM Key Wrapping Module (PKWM)
as a new trust source

Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
Signed-off-by: Srish Srinivasan <ssrish@linux.ibm.com>
Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
---
 .../admin-guide/kernel-parameters.txt         |  1 +
 .../security/keys/trusted-encrypted.rst       | 50 +++++++++++++++++++
 2 files changed, 51 insertions(+)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 1058f2a6d6a8..aac15079b33d 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -7790,6 +7790,7 @@ Kernel parameters
 			- "tee"
 			- "caam"
 			- "dcp"
+			- "pkwm"
 			If not specified then it defaults to iterating through
 			the trust source list starting with TPM and assigns the
 			first trust source as a backend which is initialized
diff --git a/Documentation/security/keys/trusted-encrypted.rst b/Documentation/security/keys/trusted-encrypted.rst
index eae6a36b1c9a..ddff7c7c2582 100644
--- a/Documentation/security/keys/trusted-encrypted.rst
+++ b/Documentation/security/keys/trusted-encrypted.rst
@@ -81,6 +81,14 @@ safe.
          and the UNIQUE key. Default is to use the UNIQUE key, but selecting
          the OTP key can be done via a module parameter (dcp_use_otp_key).
 
+     (5) PKWM (PowerVM Key Wrapping Module: IBM PowerVM + Platform KeyStore)
+
+         Rooted to a unique, per-LPAR key, which is derived from a system-wide,
+         randomly generated LPAR root key. Both the per-LPAR keys and the LPAR
+         root key are stored in hypervisor-owned secure memory at runtime,
+         and the LPAR root key is additionally persisted in secure locations
+         such as the processor SEEPROMs and encrypted NVRAM.
+
   *  Execution isolation
 
      (1) TPM
@@ -102,6 +110,14 @@ safe.
          environment. Only basic blob key encryption is executed there.
          The actual key sealing/unsealing is done on main processor/kernel space.
 
+     (5) PKWM (PowerVM Key Wrapping Module: IBM PowerVM + Platform KeyStore)
+
+         Fixed set of cryptographic operations done on on-chip hardware
+         cryptographic acceleration unit NX. Keys for wrapping and unwrapping
+         are managed by PowerVM Platform KeyStore, which stores keys in an
+         isolated in-memory copy in secure hypervisor memory, as well as in a
+         persistent copy in hypervisor-encrypted NVRAM.
+
   * Optional binding to platform integrity state
 
      (1) TPM
@@ -129,6 +145,11 @@ safe.
          Relies on Secure/Trusted boot process (called HAB by vendor) for
          platform integrity.
 
+     (5) PKWM (PowerVM Key Wrapping Module: IBM PowerVM + Platform KeyStore)
+
+         Relies on secure and trusted boot process of IBM Power systems for
+         platform integrity.
+
   *  Interfaces and APIs
 
      (1) TPM
@@ -149,6 +170,11 @@ safe.
          Vendor-specific API that is implemented as part of the DCP crypto driver in
          ``drivers/crypto/mxs-dcp.c``.
 
+     (5) PKWM (PowerVM Key Wrapping Module: IBM PowerVM + Platform KeyStore)
+
+         Platform Keystore has well documented interfaces in PAPR document.
+         Refer to ``Documentation/arch/powerpc/papr_hcalls.rst``
+
   *  Threat model
 
      The strength and appropriateness of a particular trust source for a given
@@ -191,6 +217,10 @@ selected trust source:
      a dedicated hardware RNG that is independent from DCP which can be enabled
      to back the kernel RNG.
 
+   * PKWM (PowerVM Key Wrapping Module: IBM PowerVM + Platform KeyStore)
+
+     The normal kernel random number generator is used to generate keys.
+
 Users may override this by specifying ``trusted.rng=kernel`` on the kernel
 command-line to override the used RNG with the kernel's random number pool.
 
@@ -321,6 +351,26 @@ Usage::
 specific to this DCP key-blob implementation.  The key length for new keys is
 always in bytes. Trusted Keys can be 32 - 128 bytes (256 - 1024 bits).
 
+Trusted Keys usage: PKWM
+------------------------
+
+Usage::
+
+    keyctl add trusted name "new keylen [options]" ring
+    keyctl add trusted name "load hex_blob" ring
+    keyctl print keyid
+
+    options:
+       wrap_flags=   ascii hex value of security policy requirement
+                       0x00: no secure boot requirement (default)
+                       0x01: require secure boot to be in either audit or
+                             enforced mode
+                       0x02: require secure boot to be in enforced mode
+
+"keyctl print" returns an ASCII hex copy of the sealed key, which is in format
+specific to PKWM key-blob implementation.  The key length for new keys is
+always in bytes. Trusted Keys can be 32 - 128 bytes (256 - 1024 bits).
+
 Encrypted Keys usage
 --------------------
 
-- 
2.47.3


^ permalink raw reply related

* [PATCH v6 2/6] powerpc/pseries: move the PLPKS config inside its own sysfs directory
From: Srish Srinivasan @ 2026-02-01 13:59 UTC (permalink / raw)
  To: linux-integrity, keyrings, linuxppc-dev
  Cc: maddy, mpe, npiggin, christophe.leroy, James.Bottomley, jarkko,
	zohar, nayna, rnsastry, linux-kernel, linux-security-module,
	ssrish
In-Reply-To: <20260201135930.898721-1-ssrish@linux.ibm.com>

The /sys/firmware/secvar/config directory represents Power LPAR Platform
KeyStore (PLPKS) configuration properties such as max_object_size, signed_
update_algorithms, supported_policies, total_size, used_space, and version.
These attributes describe the PLPKS, and not the secure boot variables
(secvars).

Create /sys/firmware/plpks directory and move the PLPKS config inside this
directory. For backwards compatibility, create a soft link from the secvar
sysfs directory to this config and emit a warning stating that the older
sysfs path has been deprecated. Separate out the plpks specific
documentation from secvar.

Signed-off-by: Srish Srinivasan <ssrish@linux.ibm.com>
Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
Reviewed-by: Nayna Jain <nayna@linux.ibm.com>
Tested-by: Nayna Jain <nayna@linux.ibm.com>
---
 .../ABI/testing/sysfs-firmware-plpks          | 50 ++++++++++
 Documentation/ABI/testing/sysfs-secvar        | 65 -------------
 arch/powerpc/include/asm/plpks.h              |  5 +
 arch/powerpc/include/asm/secvar.h             |  1 -
 arch/powerpc/kernel/secvar-sysfs.c            | 21 ++---
 arch/powerpc/platforms/pseries/Makefile       |  2 +-
 arch/powerpc/platforms/pseries/plpks-secvar.c | 29 ------
 arch/powerpc/platforms/pseries/plpks-sysfs.c  | 94 +++++++++++++++++++
 8 files changed, 156 insertions(+), 111 deletions(-)
 create mode 100644 Documentation/ABI/testing/sysfs-firmware-plpks
 create mode 100644 arch/powerpc/platforms/pseries/plpks-sysfs.c

diff --git a/Documentation/ABI/testing/sysfs-firmware-plpks b/Documentation/ABI/testing/sysfs-firmware-plpks
new file mode 100644
index 000000000000..af0353f34115
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-firmware-plpks
@@ -0,0 +1,50 @@
+What:		/sys/firmware/plpks/config
+Date:		February 2023
+Contact:	Nayna Jain <nayna@linux.ibm.com>
+Description:	This optional directory contains read-only config attributes as
+		defined by the PLPKS implementation. All data is in ASCII
+		format.
+
+What:		/sys/firmware/plpks/config/version
+Date:		February 2023
+Contact:	Nayna Jain <nayna@linux.ibm.com>
+Description:	Config version as reported by the hypervisor in ASCII decimal
+		format.
+
+What:		/sys/firmware/plpks/config/max_object_size
+Date:		February 2023
+Contact:	Nayna Jain <nayna@linux.ibm.com>
+Description:	Maximum allowed size of	objects in the keystore in bytes,
+		represented in ASCII decimal format.
+
+		This is not necessarily the same as the max size that can be
+		written to an update file as writes can contain more than
+		object data, you should use the size of the update file for
+		that purpose.
+
+What:		/sys/firmware/plpks/config/total_size
+Date:		February 2023
+Contact:	Nayna Jain <nayna@linux.ibm.com>
+Description:	Total size of the PLPKS in bytes, represented in ASCII decimal
+		format.
+
+What:		/sys/firmware/plpks/config/used_space
+Date:		February 2023
+Contact:	Nayna Jain <nayna@linux.ibm.com>
+Description:	Current space consumed by the key store, in bytes, represented
+		in ASCII decimal format.
+
+What:		/sys/firmware/plpks/config/supported_policies
+Date:		February 2023
+Contact:	Nayna Jain <nayna@linux.ibm.com>
+Description:	Bitmask of supported policy flags by the hypervisor, represented
+		as an 8 byte hexadecimal ASCII string. Consult the hypervisor
+		documentation for what these flags are.
+
+What:		/sys/firmware/plpks/config/signed_update_algorithms
+Date:		February 2023
+Contact:	Nayna Jain <nayna@linux.ibm.com>
+Description:	Bitmask of flags indicating which algorithms the hypervisor
+		supports for signed update of objects, represented as a 16 byte
+		hexadecimal ASCII string. Consult the hypervisor documentation
+		for what these flags mean.
diff --git a/Documentation/ABI/testing/sysfs-secvar b/Documentation/ABI/testing/sysfs-secvar
index 1016967a730f..c52a5fd15709 100644
--- a/Documentation/ABI/testing/sysfs-secvar
+++ b/Documentation/ABI/testing/sysfs-secvar
@@ -63,68 +63,3 @@ Contact:	Nayna Jain <nayna@linux.ibm.com>
 Description:	A write-only file that is used to submit the new value for the
 		variable. The size of the file represents the maximum size of
 		the variable data that can be written.
-
-What:		/sys/firmware/secvar/config
-Date:		February 2023
-Contact:	Nayna Jain <nayna@linux.ibm.com>
-Description:	This optional directory contains read-only config attributes as
-		defined by the secure variable implementation.  All data is in
-		ASCII format. The directory is only created if the backing
-		implementation provides variables to populate it, which at
-		present is only PLPKS on the pseries platform.
-
-What:		/sys/firmware/secvar/config/version
-Date:		February 2023
-Contact:	Nayna Jain <nayna@linux.ibm.com>
-Description:	Config version as reported by the hypervisor in ASCII decimal
-		format.
-
-		Currently only provided by PLPKS on the pseries platform.
-
-What:		/sys/firmware/secvar/config/max_object_size
-Date:		February 2023
-Contact:	Nayna Jain <nayna@linux.ibm.com>
-Description:	Maximum allowed size of	objects in the keystore in bytes,
-		represented in ASCII decimal format.
-
-		This is not necessarily the same as the max size that can be
-		written to an update file as writes can contain more than
-		object data, you should use the size of the update file for
-		that purpose.
-
-		Currently only provided by PLPKS on the pseries platform.
-
-What:		/sys/firmware/secvar/config/total_size
-Date:		February 2023
-Contact:	Nayna Jain <nayna@linux.ibm.com>
-Description:	Total size of the PLPKS in bytes, represented in ASCII decimal
-		format.
-
-		Currently only provided by PLPKS on the pseries platform.
-
-What:		/sys/firmware/secvar/config/used_space
-Date:		February 2023
-Contact:	Nayna Jain <nayna@linux.ibm.com>
-Description:	Current space consumed by the key store, in bytes, represented
-		in ASCII decimal format.
-
-		Currently only provided by PLPKS on the pseries platform.
-
-What:		/sys/firmware/secvar/config/supported_policies
-Date:		February 2023
-Contact:	Nayna Jain <nayna@linux.ibm.com>
-Description:	Bitmask of supported policy flags by the hypervisor,
-		represented as an 8 byte hexadecimal ASCII string. Consult the
-		hypervisor documentation for what these flags are.
-
-		Currently only provided by PLPKS on the pseries platform.
-
-What:		/sys/firmware/secvar/config/signed_update_algorithms
-Date:		February 2023
-Contact:	Nayna Jain <nayna@linux.ibm.com>
-Description:	Bitmask of flags indicating which algorithms the hypervisor
-		supports for signed update of objects, represented as a 16 byte
-		hexadecimal ASCII string. Consult the hypervisor documentation
-		for what these flags mean.
-
-		Currently only provided by PLPKS on the pseries platform.
diff --git a/arch/powerpc/include/asm/plpks.h b/arch/powerpc/include/asm/plpks.h
index f303922bf622..8829a13bfda0 100644
--- a/arch/powerpc/include/asm/plpks.h
+++ b/arch/powerpc/include/asm/plpks.h
@@ -13,6 +13,7 @@
 
 #include <linux/types.h>
 #include <linux/list.h>
+#include <linux/kobject.h>
 
 // Object policy flags from supported_policies
 #define PLPKS_OSSECBOOTAUDIT	PPC_BIT32(1) // OS secure boot must be audit/enforce
@@ -107,11 +108,15 @@ u16 plpks_get_passwordlen(void);
 void plpks_early_init_devtree(void);
 
 int plpks_populate_fdt(void *fdt);
+
+int plpks_config_create_softlink(struct kobject *from);
 #else // CONFIG_PSERIES_PLPKS
 static inline bool plpks_is_available(void) { return false; }
 static inline u16 plpks_get_passwordlen(void) { BUILD_BUG(); }
 static inline void plpks_early_init_devtree(void) { }
 static inline int plpks_populate_fdt(void *fdt) { BUILD_BUG(); }
+static inline int plpks_config_create_softlink(struct kobject *from)
+						{ return 0; }
 #endif // CONFIG_PSERIES_PLPKS
 
 #endif // _ASM_POWERPC_PLPKS_H
diff --git a/arch/powerpc/include/asm/secvar.h b/arch/powerpc/include/asm/secvar.h
index 4828e0ab7e3c..fd5006307f2a 100644
--- a/arch/powerpc/include/asm/secvar.h
+++ b/arch/powerpc/include/asm/secvar.h
@@ -20,7 +20,6 @@ struct secvar_operations {
 	int (*set)(const char *key, u64 key_len, u8 *data, u64 data_size);
 	ssize_t (*format)(char *buf, size_t bufsize);
 	int (*max_size)(u64 *max_size);
-	const struct attribute **config_attrs;
 
 	// NULL-terminated array of fixed variable names
 	// Only used if get_next() isn't provided
diff --git a/arch/powerpc/kernel/secvar-sysfs.c b/arch/powerpc/kernel/secvar-sysfs.c
index ec900bce0257..4111b21962eb 100644
--- a/arch/powerpc/kernel/secvar-sysfs.c
+++ b/arch/powerpc/kernel/secvar-sysfs.c
@@ -12,6 +12,7 @@
 #include <linux/string.h>
 #include <linux/of.h>
 #include <asm/secvar.h>
+#include <asm/plpks.h>
 
 #define NAME_MAX_SIZE	   1024
 
@@ -145,19 +146,6 @@ static __init int update_kobj_size(void)
 	return 0;
 }
 
-static __init int secvar_sysfs_config(struct kobject *kobj)
-{
-	struct attribute_group config_group = {
-		.name = "config",
-		.attrs = (struct attribute **)secvar_ops->config_attrs,
-	};
-
-	if (secvar_ops->config_attrs)
-		return sysfs_create_group(kobj, &config_group);
-
-	return 0;
-}
-
 static __init int add_var(const char *name)
 {
 	struct kobject *kobj;
@@ -260,12 +248,15 @@ static __init int secvar_sysfs_init(void)
 		goto err;
 	}
 
-	rc = secvar_sysfs_config(secvar_kobj);
+	rc = plpks_config_create_softlink(secvar_kobj);
 	if (rc) {
-		pr_err("Failed to create config directory\n");
+		pr_err("Failed to create softlink to PLPKS config directory");
 		goto err;
 	}
 
+	pr_info("/sys/firmware/secvar/config is now deprecated.\n");
+	pr_info("Will be removed in future versions.\n");
+
 	if (secvar_ops->get_next)
 		rc = secvar_sysfs_load();
 	else
diff --git a/arch/powerpc/platforms/pseries/Makefile b/arch/powerpc/platforms/pseries/Makefile
index 931ebaa474c8..3ced289a675b 100644
--- a/arch/powerpc/platforms/pseries/Makefile
+++ b/arch/powerpc/platforms/pseries/Makefile
@@ -30,7 +30,7 @@ obj-$(CONFIG_PAPR_SCM)		+= papr_scm.o
 obj-$(CONFIG_PPC_SPLPAR)	+= vphn.o
 obj-$(CONFIG_PPC_SVM)		+= svm.o
 obj-$(CONFIG_FA_DUMP)		+= rtas-fadump.o
-obj-$(CONFIG_PSERIES_PLPKS)	+= plpks.o
+obj-$(CONFIG_PSERIES_PLPKS)	+= plpks.o plpks-sysfs.o
 obj-$(CONFIG_PPC_SECURE_BOOT)	+= plpks-secvar.o
 obj-$(CONFIG_PSERIES_PLPKS_SED)	+= plpks_sed_ops.o
 obj-$(CONFIG_SUSPEND)		+= suspend.o
diff --git a/arch/powerpc/platforms/pseries/plpks-secvar.c b/arch/powerpc/platforms/pseries/plpks-secvar.c
index f9e9cc40c9d0..a50ff6943d80 100644
--- a/arch/powerpc/platforms/pseries/plpks-secvar.c
+++ b/arch/powerpc/platforms/pseries/plpks-secvar.c
@@ -20,33 +20,6 @@
 #include <asm/secvar.h>
 #include <asm/plpks.h>
 
-// Config attributes for sysfs
-#define PLPKS_CONFIG_ATTR(name, fmt, func)			\
-	static ssize_t name##_show(struct kobject *kobj,	\
-				   struct kobj_attribute *attr,	\
-				   char *buf)			\
-	{							\
-		return sysfs_emit(buf, fmt, func());		\
-	}							\
-	static struct kobj_attribute attr_##name = __ATTR_RO(name)
-
-PLPKS_CONFIG_ATTR(version, "%u\n", plpks_get_version);
-PLPKS_CONFIG_ATTR(max_object_size, "%u\n", plpks_get_maxobjectsize);
-PLPKS_CONFIG_ATTR(total_size, "%u\n", plpks_get_totalsize);
-PLPKS_CONFIG_ATTR(used_space, "%u\n", plpks_get_usedspace);
-PLPKS_CONFIG_ATTR(supported_policies, "%08x\n", plpks_get_supportedpolicies);
-PLPKS_CONFIG_ATTR(signed_update_algorithms, "%016llx\n", plpks_get_signedupdatealgorithms);
-
-static const struct attribute *config_attrs[] = {
-	&attr_version.attr,
-	&attr_max_object_size.attr,
-	&attr_total_size.attr,
-	&attr_used_space.attr,
-	&attr_supported_policies.attr,
-	&attr_signed_update_algorithms.attr,
-	NULL,
-};
-
 static u32 get_policy(const char *name)
 {
 	if ((strcmp(name, "db") == 0) ||
@@ -225,7 +198,6 @@ static const struct secvar_operations plpks_secvar_ops_static = {
 	.set = plpks_set_variable,
 	.format = plpks_secvar_format,
 	.max_size = plpks_max_size,
-	.config_attrs = config_attrs,
 	.var_names = plpks_var_names_static,
 };
 
@@ -234,7 +206,6 @@ static const struct secvar_operations plpks_secvar_ops_dynamic = {
 	.set = plpks_set_variable,
 	.format = plpks_secvar_format,
 	.max_size = plpks_max_size,
-	.config_attrs = config_attrs,
 	.var_names = plpks_var_names_dynamic,
 };
 
diff --git a/arch/powerpc/platforms/pseries/plpks-sysfs.c b/arch/powerpc/platforms/pseries/plpks-sysfs.c
new file mode 100644
index 000000000000..01d526185783
--- /dev/null
+++ b/arch/powerpc/platforms/pseries/plpks-sysfs.c
@@ -0,0 +1,94 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2025 IBM Corporation, Srish Srinivasan <ssrish@linux.ibm.com>
+ *
+ * This code exposes PLPKS config to user via sysfs
+ */
+
+#define pr_fmt(fmt) "plpks-sysfs: "fmt
+
+#include <linux/init.h>
+#include <linux/printk.h>
+#include <linux/types.h>
+#include <asm/machdep.h>
+#include <asm/plpks.h>
+
+/* config attributes for sysfs */
+#define PLPKS_CONFIG_ATTR(name, fmt, func)			\
+	static ssize_t name##_show(struct kobject *kobj,	\
+				   struct kobj_attribute *attr,	\
+				   char *buf)			\
+	{							\
+		return sysfs_emit(buf, fmt, func());		\
+	}							\
+	static struct kobj_attribute attr_##name = __ATTR_RO(name)
+
+PLPKS_CONFIG_ATTR(version, "%u\n", plpks_get_version);
+PLPKS_CONFIG_ATTR(max_object_size, "%u\n", plpks_get_maxobjectsize);
+PLPKS_CONFIG_ATTR(total_size, "%u\n", plpks_get_totalsize);
+PLPKS_CONFIG_ATTR(used_space, "%u\n", plpks_get_usedspace);
+PLPKS_CONFIG_ATTR(supported_policies, "%08x\n", plpks_get_supportedpolicies);
+PLPKS_CONFIG_ATTR(signed_update_algorithms, "%016llx\n",
+		  plpks_get_signedupdatealgorithms);
+
+static const struct attribute *config_attrs[] = {
+	&attr_version.attr,
+	&attr_max_object_size.attr,
+	&attr_total_size.attr,
+	&attr_used_space.attr,
+	&attr_supported_policies.attr,
+	&attr_signed_update_algorithms.attr,
+	NULL,
+};
+
+static struct kobject *plpks_kobj, *plpks_config_kobj;
+
+int plpks_config_create_softlink(struct kobject *from)
+{
+	if (!plpks_config_kobj)
+		return -EINVAL;
+	return sysfs_create_link(from, plpks_config_kobj, "config");
+}
+
+static __init int plpks_sysfs_config(struct kobject *kobj)
+{
+	struct attribute_group config_group = {
+		.name = NULL,
+		.attrs = (struct attribute **)config_attrs,
+	};
+
+	return sysfs_create_group(kobj, &config_group);
+}
+
+static __init int plpks_sysfs_init(void)
+{
+	int rc;
+
+	if (!plpks_is_available())
+		return -ENODEV;
+
+	plpks_kobj = kobject_create_and_add("plpks", firmware_kobj);
+	if (!plpks_kobj) {
+		pr_err("Failed to create plpks kobj\n");
+		return -ENOMEM;
+	}
+
+	plpks_config_kobj = kobject_create_and_add("config", plpks_kobj);
+	if (!plpks_config_kobj) {
+		pr_err("Failed to create plpks config kobj\n");
+		kobject_put(plpks_kobj);
+		return -ENOMEM;
+	}
+
+	rc = plpks_sysfs_config(plpks_config_kobj);
+	if (rc) {
+		pr_err("Failed to create attribute group for plpks config\n");
+		kobject_put(plpks_config_kobj);
+		kobject_put(plpks_kobj);
+		return rc;
+	}
+
+	return 0;
+}
+
+machine_subsys_initcall(pseries, plpks_sysfs_init);
-- 
2.47.3


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox