* [PATCH v15 6/9] rust: page: update formatting of `use` statements
From: Andreas Hindborg @ 2026-02-20 9:51 UTC (permalink / raw)
To: Miguel Ojeda, Gary Guo, Björn Roy Baron, Benno Lossin,
Alice Ryhl, Trevor Gross, Danilo Krummrich, Greg Kroah-Hartman,
Dave Ertman, Ira Weiny, Leon Romanovsky, Paul Moore, Serge Hallyn,
Rafael J. Wysocki, David Airlie, Simona Vetter, Alexander Viro,
Christian Brauner, Jan Kara, Igor Korotin, Daniel Almeida,
Lorenzo Stoakes, Liam R. Howlett, Viresh Kumar, Nishanth Menon,
Stephen Boyd, Bjorn Helgaas, Krzysztof Wilczyński,
Boqun Feng, Boqun Feng
Cc: linux-kernel, rust-for-linux, linux-block, linux-security-module,
dri-devel, linux-fsdevel, linux-mm, linux-pm, linux-pci,
Andreas Hindborg
In-Reply-To: <20260220-unique-ref-v15-0-893ed86b06cc@kernel.org>
Update formatting in preparation for next patch
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
rust/kernel/page.rs | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/rust/kernel/page.rs b/rust/kernel/page.rs
index 432fc0297d4a8..bf3bed7e2d3fe 100644
--- a/rust/kernel/page.rs
+++ b/rust/kernel/page.rs
@@ -3,17 +3,23 @@
//! Kernel page allocation and management.
use crate::{
- alloc::{AllocError, Flags},
+ alloc::{
+ AllocError,
+ Flags, //
+ },
bindings,
error::code::*,
error::Result,
- uaccess::UserSliceReader,
+ uaccess::UserSliceReader, //
};
use core::{
marker::PhantomData,
mem::ManuallyDrop,
ops::Deref,
- ptr::{self, NonNull},
+ ptr::{
+ self,
+ NonNull, //
+ }, //
};
/// A bitwise shift for the page size.
--
2.51.2
^ permalink raw reply related
* [PATCH v15 3/9] rust: Add missing SAFETY documentation for `ARef` example
From: Andreas Hindborg @ 2026-02-20 9:51 UTC (permalink / raw)
To: Miguel Ojeda, Gary Guo, Björn Roy Baron, Benno Lossin,
Alice Ryhl, Trevor Gross, Danilo Krummrich, Greg Kroah-Hartman,
Dave Ertman, Ira Weiny, Leon Romanovsky, Paul Moore, Serge Hallyn,
Rafael J. Wysocki, David Airlie, Simona Vetter, Alexander Viro,
Christian Brauner, Jan Kara, Igor Korotin, Daniel Almeida,
Lorenzo Stoakes, Liam R. Howlett, Viresh Kumar, Nishanth Menon,
Stephen Boyd, Bjorn Helgaas, Krzysztof Wilczyński,
Boqun Feng, Boqun Feng
Cc: linux-kernel, rust-for-linux, linux-block, linux-security-module,
dri-devel, linux-fsdevel, linux-mm, linux-pm, linux-pci,
Andreas Hindborg, Oliver Mangold
In-Reply-To: <20260220-unique-ref-v15-0-893ed86b06cc@kernel.org>
From: Oliver Mangold <oliver.mangold@pm.me>
SAFETY comment in rustdoc example was just 'TODO'. Fixed.
Signed-off-by: Oliver Mangold <oliver.mangold@pm.me>
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
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 61caddfd89619..efe16a7fdfa5d 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 owning an increment on the refcount for
+ /// // the object 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
^ permalink raw reply related
* [PATCH v15 5/9] rust: Add `OwnableRefCounted`
From: Andreas Hindborg @ 2026-02-20 9:51 UTC (permalink / raw)
To: Miguel Ojeda, Gary Guo, Björn Roy Baron, Benno Lossin,
Alice Ryhl, Trevor Gross, Danilo Krummrich, Greg Kroah-Hartman,
Dave Ertman, Ira Weiny, Leon Romanovsky, Paul Moore, Serge Hallyn,
Rafael J. Wysocki, David Airlie, Simona Vetter, Alexander Viro,
Christian Brauner, Jan Kara, Igor Korotin, Daniel Almeida,
Lorenzo Stoakes, Liam R. Howlett, Viresh Kumar, Nishanth Menon,
Stephen Boyd, Bjorn Helgaas, Krzysztof Wilczyński,
Boqun Feng, Boqun Feng
Cc: linux-kernel, rust-for-linux, linux-block, linux-security-module,
dri-devel, linux-fsdevel, linux-mm, linux-pm, linux-pci,
Andreas Hindborg, Oliver Mangold
In-Reply-To: <20260220-unique-ref-v15-0-893ed86b06cc@kernel.org>
From: Oliver Mangold <oliver.mangold@pm.me>
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>
[ Andreas: Fix formatting, update documentation, fix error handling in
examples. ]
Co-developed-by: Andreas Hindborg <a.hindborg@kernel.org>
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
rust/kernel/owned.rs | 143 ++++++++++++++++++++++++++++++++++++++++++++---
rust/kernel/sync/aref.rs | 15 ++++-
rust/kernel/types.rs | 1 +
3 files changed, 150 insertions(+), 9 deletions(-)
diff --git a/rust/kernel/owned.rs b/rust/kernel/owned.rs
index b8d3b9c725cf6..a9bc871e07ce1 100644
--- a/rust/kernel/owned.rs
+++ b/rust/kernel/owned.rs
@@ -14,18 +14,24 @@
pin::Pin,
ptr::NonNull, //
};
+use kernel::{
+ sync::aref::ARef,
+ types::RefCounted, //
+};
/// Types that specify their own way of performing allocation and destruction. Typically, this trait
/// is implemented on types from the C side.
///
-/// 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
///
@@ -63,8 +69,7 @@
/// 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.");
+/// let result = NonNull::new(KBox::into_raw(result)).ok_or(ENOMEM)?;
/// // Count new allocation
/// *FOO_ALLOC_COUNT.lock() += 1;
/// // SAFETY: We just allocated the `Self`, thus it is valid and there cannot be any other
@@ -88,11 +93,12 @@
/// }
///
/// {
-/// let foo = Foo::new().expect("Failed to allocate a Foo. This shouldn't happen");
+/// let foo = Foo::new()?;
/// assert!(*FOO_ALLOC_COUNT.lock() == 1);
/// }
/// // `foo` is out of scope now, so we expect no live allocations.
/// assert!(*FOO_ALLOC_COUNT.lock() == 0);
+/// # Ok::<(), Error>(())
/// ```
pub unsafe trait Ownable {
/// Releases the object.
@@ -194,3 +200,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};
+///
+/// // An internally refcounted struct for demonstration purposes.
+/// //
+/// // # Invariants
+/// //
+/// // - `refcount` is always non-zero for a valid object.
+/// // - `refcount` is >1 if there is more than one Rust reference to it.
+/// //
+/// struct Foo {
+/// refcount: Cell<usize>,
+/// }
+///
+/// impl Foo {
+/// fn new() -> Result<Owned<Self>> {
+/// // 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)).ok_or(ENOMEM)?;
+/// // 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)
+/// }
+/// }
+/// }
+///
+/// // 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()?;
+/// let mut foo = ARef::from(foo);
+/// {
+/// let bar = foo.clone();
+/// assert!(Owned::try_from(bar).is_err());
+/// }
+/// assert!(Owned::try_from(foo).is_ok());
+/// # Ok::<(), Error>(())
+/// ```
+pub trait OwnableRefCounted: RefCounted + Ownable + Sized {
+ /// Checks if the [`ARef`] is unique and converts it to an [`Owned`] if that is the case.
+ /// Otherwise it returns again an [`ARef`] to the same underlying object.
+ fn try_from_shared(this: ARef<Self>) -> Result<Owned<Self>, ARef<Self>>;
+
+ /// 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 3c63c9a5fb9be..77f6c8dc411eb 100644
--- a/rust/kernel/sync/aref.rs
+++ b/rust/kernel/sync/aref.rs
@@ -23,6 +23,10 @@
ops::Deref,
ptr::NonNull, //
};
+use kernel::types::{
+ OwnableRefCounted,
+ Owned, //
+};
/// Types that are internally reference counted.
///
@@ -35,7 +39,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`] allows to convert between unique and
+/// shared references (i.e. [`Owned<Self>`](crate::types::Owned) and
+/// [`ARef<Self>`](crate::types::Owned)).
///
/// # Safety
///
@@ -185,6 +192,12 @@ fn from(b: &T) -> Self {
}
}
+impl<T: OwnableRefCounted> From<Owned<T>> for ARef<T> {
+ fn from(b: Owned<T>) -> Self {
+ T::into_shared(b)
+ }
+}
+
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
diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
index 9b96aa2ebdb7e..f43c091eeb8b7 100644
--- a/rust/kernel/types.rs
+++ b/rust/kernel/types.rs
@@ -14,6 +14,7 @@
pub use crate::{
owned::{
Ownable,
+ OwnableRefCounted,
Owned, //
},
sync::aref::{
--
2.51.2
^ permalink raw reply related
* [PATCH v15 7/9] rust: page: convert to `Ownable`
From: Andreas Hindborg @ 2026-02-20 9:51 UTC (permalink / raw)
To: Miguel Ojeda, Gary Guo, Björn Roy Baron, Benno Lossin,
Alice Ryhl, Trevor Gross, Danilo Krummrich, Greg Kroah-Hartman,
Dave Ertman, Ira Weiny, Leon Romanovsky, Paul Moore, Serge Hallyn,
Rafael J. Wysocki, David Airlie, Simona Vetter, Alexander Viro,
Christian Brauner, Jan Kara, Igor Korotin, Daniel Almeida,
Lorenzo Stoakes, Liam R. Howlett, Viresh Kumar, Nishanth Menon,
Stephen Boyd, Bjorn Helgaas, Krzysztof Wilczyński,
Boqun Feng, Boqun Feng
Cc: linux-kernel, rust-for-linux, linux-block, linux-security-module,
dri-devel, linux-fsdevel, linux-mm, linux-pm, linux-pci,
Andreas Hindborg, Asahi Lina, Asahi Lina
In-Reply-To: <20260220-unique-ref-v15-0-893ed86b06cc@kernel.org>
From: Asahi Lina <lina+kernel@asahilina.net>
This allows Page references to be returned as borrowed references,
without necessarily owning the struct page.
Signed-off-by: Asahi Lina <lina@asahilina.net>
[ Andreas: Fix formatting and add a safety comment. ]
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
rust/kernel/page.rs | 34 +++++++++++++++++++++++-----------
1 file changed, 23 insertions(+), 11 deletions(-)
diff --git a/rust/kernel/page.rs b/rust/kernel/page.rs
index bf3bed7e2d3fe..4591b7b01c3d2 100644
--- a/rust/kernel/page.rs
+++ b/rust/kernel/page.rs
@@ -10,6 +10,11 @@
bindings,
error::code::*,
error::Result,
+ types::{
+ Opaque,
+ Ownable,
+ Owned, //
+ },
uaccess::UserSliceReader, //
};
use core::{
@@ -83,7 +88,7 @@ pub const fn page_align(addr: usize) -> usize {
///
/// [`VBox`]: kernel::alloc::VBox
/// [`Vmalloc`]: kernel::alloc::allocator::Vmalloc
-pub struct BorrowedPage<'a>(ManuallyDrop<Page>, PhantomData<&'a Page>);
+pub struct BorrowedPage<'a>(ManuallyDrop<Owned<Page>>, PhantomData<&'a Owned<Page>>);
impl<'a> BorrowedPage<'a> {
/// Constructs a [`BorrowedPage`] from a raw pointer to a `struct page`.
@@ -93,7 +98,9 @@ impl<'a> BorrowedPage<'a> {
/// - `ptr` must point to a valid `bindings::page`.
/// - `ptr` must remain valid for the entire lifetime `'a`.
pub unsafe fn from_raw(ptr: NonNull<bindings::page>) -> Self {
- let page = Page { page: ptr };
+ let page: Owned<Page> =
+ // SAFETY: By function safety requirements `ptr` is non null and valid for 'a.
+ unsafe { Owned::from_raw(NonNull::new_unchecked(ptr.as_ptr().cast())) };
// INVARIANT: The safety requirements guarantee that `ptr` is valid for the entire lifetime
// `'a`.
@@ -126,8 +133,9 @@ pub trait AsPageIter {
/// # Invariants
///
/// The pointer is valid, and has ownership over the page.
+#[repr(transparent)]
pub struct Page {
- page: NonNull<bindings::page>,
+ page: Opaque<bindings::page>,
}
// SAFETY: Pages have no logic that relies on them staying on a given thread, so moving them across
@@ -161,19 +169,20 @@ impl Page {
/// # Ok::<(), kernel::alloc::AllocError>(())
/// ```
#[inline]
- pub fn alloc_page(flags: Flags) -> Result<Self, AllocError> {
+ pub fn alloc_page(flags: Flags) -> Result<Owned<Self>, AllocError> {
// SAFETY: Depending on the value of `gfp_flags`, this call may sleep. Other than that, it
// is always safe to call this method.
let page = unsafe { bindings::alloc_pages(flags.as_raw(), 0) };
let page = NonNull::new(page).ok_or(AllocError)?;
- // INVARIANT: We just successfully allocated a page, so we now have ownership of the newly
- // allocated page. We transfer that ownership to the new `Page` object.
- Ok(Self { page })
+ // SAFETY: We just successfully allocated a page, so we now have ownership of the newly
+ // allocated page. We transfer that ownership to the new `Owned<Page>` object.
+ // Since `Page` is transparent, we can cast the pointer directly.
+ Ok(unsafe { Owned::from_raw(page.cast()) })
}
/// Returns a raw pointer to the page.
pub fn as_ptr(&self) -> *mut bindings::page {
- self.page.as_ptr()
+ Opaque::cast_into(&self.page)
}
/// Get the node id containing this page.
@@ -348,10 +357,13 @@ pub unsafe fn copy_from_user_slice_raw(
}
}
-impl Drop for Page {
+// SAFETY: `Owned<Page>` objects returned by Page::alloc_page() follow the requirements of
+// the Ownable abstraction.
+unsafe impl Ownable for Page {
#[inline]
- fn drop(&mut self) {
+ unsafe fn release(this: NonNull<Self>) {
// SAFETY: By the type invariants, we have ownership of the page and can free it.
- unsafe { bindings::__free_pages(self.page.as_ptr(), 0) };
+ // Since Page is transparent, we can cast the raw pointer directly.
+ unsafe { bindings::__free_pages(this.cast().as_ptr(), 0) };
}
}
--
2.51.2
^ permalink raw reply related
* [PATCH v15 1/9] rust: types: Add Ownable/Owned types
From: Andreas Hindborg @ 2026-02-20 9:51 UTC (permalink / raw)
To: Miguel Ojeda, Gary Guo, Björn Roy Baron, Benno Lossin,
Alice Ryhl, Trevor Gross, Danilo Krummrich, Greg Kroah-Hartman,
Dave Ertman, Ira Weiny, Leon Romanovsky, Paul Moore, Serge Hallyn,
Rafael J. Wysocki, David Airlie, Simona Vetter, Alexander Viro,
Christian Brauner, Jan Kara, Igor Korotin, Daniel Almeida,
Lorenzo Stoakes, Liam R. Howlett, Viresh Kumar, Nishanth Menon,
Stephen Boyd, Bjorn Helgaas, Krzysztof Wilczyński,
Boqun Feng, Boqun Feng
Cc: linux-kernel, rust-for-linux, linux-block, linux-security-module,
dri-devel, linux-fsdevel, linux-mm, linux-pm, linux-pci,
Andreas Hindborg, Asahi Lina, Oliver Mangold
In-Reply-To: <20260220-unique-ref-v15-0-893ed86b06cc@kernel.org>
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>
Reviewed-by: Boqun Feng <boqun.feng@gmail.com>
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
[ Andreas: Updated documentation, examples, and formatting ]
Reviewed-by: Gary Guo <gary@garyguo.net>
Co-developed-by: Andreas Hindborg <a.hindborg@kernel.org>
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
rust/kernel/lib.rs | 1 +
rust/kernel/owned.rs | 196 +++++++++++++++++++++++++++++++++++++++++++++++
rust/kernel/sync/aref.rs | 5 ++
rust/kernel/types.rs | 11 ++-
4 files changed, 212 insertions(+), 1 deletion(-)
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 696f62f85eb5f..a2bec807f03f1 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -121,6 +121,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 0000000000000..d566ad0aa1c99
--- /dev/null
+++ b/rust/kernel/owned.rs
@@ -0,0 +1,196 @@
+// 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, //
+};
+
+/// Types that specify their own way of performing allocation and destruction. Typically, this trait
+/// is implemented on types from the C side.
+///
+/// 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>> {
+/// // 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: The implementation of `release` in this trait implementation correctly frees the
+/// // owned `Foo`.
+/// unsafe impl Ownable for Foo {
+/// unsafe fn release(this: NonNull<Self>) {
+/// // 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.
+/// drop(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>);
+}
+
+/// A mutable 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>,
+}
+
+impl<T: Ownable> Owned<T> {
+ /// Creates a new instance of [`Owned`].
+ ///
+ /// This function 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).
+ /// - 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 possible similar to
+ /// a mutable reference (e.g. by using [`Opaque`]).
+ ///
+ /// [`Opaque`]: kernel::types::Opaque
+ pub unsafe fn from_raw(ptr: NonNull<T>) -> Self {
+ // INVARIANT: By function safety requirement:
+ // - The resulting object has exclusive access to the `T` pointed to by `ptr`.
+ // - The `T` object pointed to by `ptr` is alive at least as long as the returned `Self`.
+ Self { ptr }
+ }
+
+ /// 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.
+ 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 as_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) }
+ }
+}
+
+// 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> 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 0d24a0432015d..e175aefe86151 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 9c5e7dbf16323..4aec7b699269a 100644
--- a/rust/kernel/types.rs
+++ b/rust/kernel/types.rs
@@ -11,7 +11,16 @@
};
use pin_init::{PinInit, Wrapper, Zeroable};
-pub use crate::sync::aref::{ARef, AlwaysRefCounted};
+pub use crate::{
+ owned::{
+ Ownable,
+ Owned, //
+ },
+ sync::aref::{
+ ARef,
+ AlwaysRefCounted, //
+ }, //
+};
/// Used to transfer ownership to and from foreign (non-Rust) languages.
///
--
2.51.2
^ permalink raw reply related
* [PATCH v15 8/9] rust: implement `ForeignOwnable` for `Owned`
From: Andreas Hindborg @ 2026-02-20 9:51 UTC (permalink / raw)
To: Miguel Ojeda, Gary Guo, Björn Roy Baron, Benno Lossin,
Alice Ryhl, Trevor Gross, Danilo Krummrich, Greg Kroah-Hartman,
Dave Ertman, Ira Weiny, Leon Romanovsky, Paul Moore, Serge Hallyn,
Rafael J. Wysocki, David Airlie, Simona Vetter, Alexander Viro,
Christian Brauner, Jan Kara, Igor Korotin, Daniel Almeida,
Lorenzo Stoakes, Liam R. Howlett, Viresh Kumar, Nishanth Menon,
Stephen Boyd, Bjorn Helgaas, Krzysztof Wilczyński,
Boqun Feng, Boqun Feng
Cc: linux-kernel, rust-for-linux, linux-block, linux-security-module,
dri-devel, linux-fsdevel, linux-mm, linux-pm, linux-pci,
Andreas Hindborg
In-Reply-To: <20260220-unique-ref-v15-0-893ed86b06cc@kernel.org>
Implement `ForeignOwnable` for `Owned<T>`. This allows use of `Owned<T>` in
places such as the `XArray`.
Note that `T` does not need to implement `ForeignOwnable` for `Owned<T>` to
implement `ForeignOwnable`.
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
rust/kernel/owned.rs | 45 ++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 44 insertions(+), 1 deletion(-)
diff --git a/rust/kernel/owned.rs b/rust/kernel/owned.rs
index a9bc871e07ce1..b115b4f3db6d0 100644
--- a/rust/kernel/owned.rs
+++ b/rust/kernel/owned.rs
@@ -16,7 +16,10 @@
};
use kernel::{
sync::aref::ARef,
- types::RefCounted, //
+ types::{
+ ForeignOwnable, //
+ RefCounted,
+ }, //
};
/// Types that specify their own way of performing allocation and destruction. Typically, this trait
@@ -120,6 +123,7 @@ pub unsafe trait Ownable {
///
/// - 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.
+#[repr(transparent)]
pub struct Owned<T: Ownable> {
ptr: NonNull<T>,
}
@@ -201,6 +205,45 @@ fn drop(&mut self) {
}
}
+// SAFETY: We derive the pointer to `T` from a valid `T`, so the returned
+// pointer satisfy alignment requirements of `T`.
+unsafe impl<T: Ownable + 'static> ForeignOwnable for Owned<T> {
+ const FOREIGN_ALIGN: usize = core::mem::align_of::<Owned<T>>();
+
+ type Borrowed<'a> = &'a T;
+ type BorrowedMut<'a> = Pin<&'a mut T>;
+
+ fn into_foreign(self) -> *mut kernel::ffi::c_void {
+ let ptr = self.ptr.as_ptr().cast();
+ core::mem::forget(self);
+ ptr
+ }
+
+ unsafe fn from_foreign(ptr: *mut kernel::ffi::c_void) -> Self {
+ Self {
+ // SAFETY: By function safety contract, `ptr` came from
+ // `into_foreign` and cannot be null.
+ ptr: unsafe { NonNull::new_unchecked(ptr.cast()) },
+ }
+ }
+
+ unsafe fn borrow<'a>(ptr: *mut kernel::ffi::c_void) -> Self::Borrowed<'a> {
+ // SAFETY: By function safety requirements, `ptr` is valid for use as a
+ // reference for `'a`.
+ unsafe { &*ptr.cast() }
+ }
+
+ unsafe fn borrow_mut<'a>(ptr: *mut kernel::ffi::c_void) -> Self::BorrowedMut<'a> {
+ // SAFETY: By function safety requirements, `ptr` is valid for use as a
+ // unique reference for `'a`.
+ let inner = unsafe { &mut *ptr.cast() };
+
+ // SAFETY: We never move out of inner, and we do not hand out mutable
+ // references when `T: !Unpin`.
+ unsafe { Pin::new_unchecked(inner) }
+ }
+}
+
/// A trait for objects that can be wrapped in either one of the reference types [`Owned`] and
/// [`ARef`].
///
--
2.51.2
^ permalink raw reply related
* [PATCH v15 4/9] rust: aref: update formatting of use statements
From: Andreas Hindborg @ 2026-02-20 9:51 UTC (permalink / raw)
To: Miguel Ojeda, Gary Guo, Björn Roy Baron, Benno Lossin,
Alice Ryhl, Trevor Gross, Danilo Krummrich, Greg Kroah-Hartman,
Dave Ertman, Ira Weiny, Leon Romanovsky, Paul Moore, Serge Hallyn,
Rafael J. Wysocki, David Airlie, Simona Vetter, Alexander Viro,
Christian Brauner, Jan Kara, Igor Korotin, Daniel Almeida,
Lorenzo Stoakes, Liam R. Howlett, Viresh Kumar, Nishanth Menon,
Stephen Boyd, Bjorn Helgaas, Krzysztof Wilczyński,
Boqun Feng, Boqun Feng
Cc: linux-kernel, rust-for-linux, linux-block, linux-security-module,
dri-devel, linux-fsdevel, linux-mm, linux-pm, linux-pci,
Andreas Hindborg
In-Reply-To: <20260220-unique-ref-v15-0-893ed86b06cc@kernel.org>
Update formatting if use statements in preparation for next commit.
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
rust/kernel/sync/aref.rs | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/rust/kernel/sync/aref.rs b/rust/kernel/sync/aref.rs
index efe16a7fdfa5d..3c63c9a5fb9be 100644
--- a/rust/kernel/sync/aref.rs
+++ b/rust/kernel/sync/aref.rs
@@ -17,7 +17,12 @@
//! [`Arc`]: crate::sync::Arc
//! [`Arc<T>`]: crate::sync::Arc
-use core::{marker::PhantomData, mem::ManuallyDrop, ops::Deref, ptr::NonNull};
+use core::{
+ marker::PhantomData,
+ mem::ManuallyDrop,
+ ops::Deref,
+ ptr::NonNull, //
+};
/// Types that are internally reference counted.
///
--
2.51.2
^ permalink raw reply related
* [PATCH] cred: clarify usage of get_cred_rcu()
From: Alice Ryhl @ 2026-02-20 9:19 UTC (permalink / raw)
To: Paul Moore, Serge Hallyn; +Cc: linux-security-module, linux-kernel, Alice Ryhl
After being confused by looking at get_cred() and get_cred_rcu(), I
figured out what's going on. Thus, add some comments to clarify how
get_cred_rcu() works for the benefit of others looking in the future.
Note that in principle we could add an assertion that non_rcu is zero in
the failure path of atomic_long_inc_not_zero().
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
include/linux/cred.h | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/include/linux/cred.h b/include/linux/cred.h
index ed1609d78cd7..95dcf5e967c7 100644
--- a/include/linux/cred.h
+++ b/include/linux/cred.h
@@ -213,32 +213,50 @@ * get_cred_many - Get references on a set of credentials
static inline const struct cred *get_cred_many(const struct cred *cred, int nr)
{
struct cred *nonconst_cred = (struct cred *) cred;
if (!cred)
return cred;
nonconst_cred->non_rcu = 0;
atomic_long_add(nr, &nonconst_cred->usage);
return cred;
}
/*
* get_cred - Get a reference on a set of credentials
* @cred: The credentials to reference
*
* Get a reference on the specified set of credentials. The caller must
* release the reference. If %NULL is passed, it is returned with no action.
*
* This is used to deal with a committed set of credentials.
*/
static inline const struct cred *get_cred(const struct cred *cred)
{
return get_cred_many(cred, 1);
}
+/*
+ * get_cred_rcu - Get a reference on a set of credentials under rcu
+ * @cred: The credentials to reference
+ *
+ * Get a reference on the specified set of credentials, or %NULL if the last
+ * refcount has already been put.
+ *
+ * This is used to obtain a reference under an rcu read lock.
+ */
static inline const struct cred *get_cred_rcu(const struct cred *cred)
{
struct cred *nonconst_cred = (struct cred *) cred;
if (!cred)
return NULL;
if (!atomic_long_inc_not_zero(&nonconst_cred->usage))
return NULL;
+ /*
+ * If non_rcu is not already zero, then this call to get_cred_rcu() is
+ * probably wrong because if 'usage' goes to zero prior to this call,
+ * then get_cred_rcu() assumes it is freed with rcu.
+ *
+ * However, an exception to this is using get_cred_rcu() in cases where
+ * get_cred() would have been okay. To support that case, we do not
+ * check non_rcu and set it to zero regardless.
+ */
nonconst_cred->non_rcu = 0;
return cred;
}
--
2.53.0.345.g96ddfc5eaa-goog
^ permalink raw reply related
* Re: [PATCH v2 v2] evm: check return values of crypto_shash functions
From: Roberto Sassu @ 2026-02-20 9:06 UTC (permalink / raw)
To: Daniel Hodges
Cc: Daniel Hodges, zohar, roberto.sassu, dmitry.kasatkin,
eric.snowberg, paul, jmorris, serge, linux-integrity,
linux-security-module, linux-kernel
In-Reply-To: <lj2loy57pavtihqeuywpc2aev7zy3k3poop346dtmelmocp75q@lbejeufrrxwr>
On Thu, 2026-02-19 at 10:01 -0500, Daniel Hodges wrote:
> On Thu, Feb 19, 2026 at 01:36:39PM +0100, Roberto Sassu wrote:
> > On Thu, 2026-02-19 at 10:26 +0100, Roberto Sassu wrote:
> > > On Thu, 2026-02-05 at 21:42 -0500, Daniel Hodges wrote:
> > > > The crypto_shash_update() and crypto_shash_final() functions can fail
> > > > and return error codes, but their return values were not being checked
> > > > in several places in security/integrity/evm/evm_crypto.c:
> > > >
> > > > - hmac_add_misc() ignored returns from crypto_shash_update() and
> > > > crypto_shash_final()
> > > > - evm_calc_hmac_or_hash() ignored returns from crypto_shash_update()
> > > > - evm_init_hmac() ignored returns from crypto_shash_update()
> > > >
> > > > If these hash operations fail silently, the resulting HMAC could be
> > > > invalid or incomplete, which could weaken the integrity verification
> > > > security that EVM provides.
> > > >
> > > > This patch converts hmac_add_misc() from void to int return type and
> > > > adds proper error checking and propagation for all crypto_shash_*
> > > > function calls. All callers are updated to handle the new return values.
> > > > Additionally, error messages are logged when cryptographic operations
> > > > fail to provide visibility into the failure rather than silently
> > > > returning error codes.
> > > >
> > > > Fixes: 66dbc325afce ("evm: re-release")
> > > > Signed-off-by: Daniel Hodges <git@danielhodges.dev>
> > >
> > > After fixing the minor issue below:
> >
> > Already did it. The patch is here (after fixing a conflict with
> > 0496fc9cdc38 "evm: Use ordered xattrs list to calculate HMAC in
> > evm_init_hmac()"):
> >
> > https://github.com/robertosassu/linux/commit/d5aba42198b602c6de002ef02a4e6cc1d75652d7
> >
> > Roberto
>
> Nice, thanks for handling that!
Welcome!
Roberto
^ permalink raw reply
* Re: [PATCH 2/5] security: export binder symbols
From: Paul Moore @ 2026-02-20 0:00 UTC (permalink / raw)
To: Alice Ryhl
Cc: Greg Kroah-Hartman, Carlos Llamas, Alexander Viro,
Christian Brauner, Jan Kara, James Morris, Serge E. Hallyn,
Andrew Morton, Dave Chinner, Qi Zheng, Roman Gushchin,
Muchun Song, David Hildenbrand, Lorenzo Stoakes, Liam R. Howlett,
Vlastimil Babka, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, Danilo Krummrich,
kernel-team, linux-fsdevel, linux-kernel, linux-security-module,
linux-mm, rust-for-linux
In-Reply-To: <20260205-binder-tristate-v1-2-dfc947c35d35@google.com>
On Thu, Feb 5, 2026 at 5:51 AM Alice Ryhl <aliceryhl@google.com> wrote:
>
> To enable building Binder as a module, export these symbols.
>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
> security/security.c | 4 ++++
> 1 file changed, 4 insertions(+)
Acked-by: Paul Moore <paul@paul-moore.com>
--
paul-moore.com
^ permalink raw reply
* Re: [PATCH v6] lsm: Add LSM hook security_unix_find
From: Günther Noack @ 2026-02-19 20:26 UTC (permalink / raw)
To: Justin Suess
Cc: brauner, demiobenour, fahimitahera, hi, horms, ivanov.mikhail1,
jannh, jmorris, john.johansen, konstantin.meskhidze,
linux-security-module, m, matthieu, mic, netdev, paul,
samasth.norway.ananda, serge, viro
In-Reply-To: <20260219200459.1474232-1-utilityemal77@gmail.com>
On Thu, Feb 19, 2026 at 03:04:59PM -0500, Justin Suess wrote:
> Add a LSM hook security_unix_find.
>
> This hook is called to check the path of a named unix socket before a
> connection is initiated. The peer socket may be inspected as well.
>
> Why existing hooks are unsuitable:
>
> Existing socket hooks, security_unix_stream_connect(),
> security_unix_may_send(), and security_socket_connect() don't provide
> TOCTOU-free / namespace independent access to the paths of sockets.
>
> (1) We cannot resolve the path from the struct sockaddr in existing hooks.
> This requires another path lookup. A change in the path between the
> two lookups will cause a TOCTOU bug.
>
> (2) We cannot use the struct path from the listening socket, because it
> may be bound to a path in a different namespace than the caller,
> resulting in a path that cannot be referenced at policy creation time.
>
> Cc: Günther Noack <gnoack3000@gmail.com>
> Cc: Tingmao Wang <m@maowtm.org>
> Signed-off-by: Justin Suess <utilityemal77@gmail.com>
> ---
> include/linux/lsm_hook_defs.h | 5 +++++
> include/linux/security.h | 11 +++++++++++
> net/unix/af_unix.c | 13 ++++++++++---
> security/security.c | 20 ++++++++++++++++++++
> 4 files changed, 46 insertions(+), 3 deletions(-)
>
> diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
> index 8c42b4bde09c..7a0fd3dbfa29 100644
> --- a/include/linux/lsm_hook_defs.h
> +++ b/include/linux/lsm_hook_defs.h
> @@ -317,6 +317,11 @@ LSM_HOOK(int, 0, post_notification, const struct cred *w_cred,
> LSM_HOOK(int, 0, watch_key, struct key *key)
> #endif /* CONFIG_SECURITY && CONFIG_KEY_NOTIFICATIONS */
>
> +#if defined(CONFIG_SECURITY_NETWORK) && defined(CONFIG_SECURITY_PATH)
> +LSM_HOOK(int, 0, unix_find, const struct path *path, struct sock *other,
> + int flags)
> +#endif /* CONFIG_SECURITY_NETWORK && CONFIG_SECURITY_PATH */
> +
> #ifdef CONFIG_SECURITY_NETWORK
> LSM_HOOK(int, 0, unix_stream_connect, struct sock *sock, struct sock *other,
> struct sock *newsk)
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 83a646d72f6f..99a33d8eb28d 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -1931,6 +1931,17 @@ static inline int security_mptcp_add_subflow(struct sock *sk, struct sock *ssk)
> }
> #endif /* CONFIG_SECURITY_NETWORK */
>
> +#if defined(CONFIG_SECURITY_NETWORK) && defined(CONFIG_SECURITY_PATH)
> +
> +int security_unix_find(const struct path *path, struct sock *other, int flags);
> +
> +#else /* CONFIG_SECURITY_NETWORK && CONFIG_SECURITY_PATH */
> +static inline int security_unix_find(const struct path *path, struct sock *other, int flags)
> +{
> + return 0;
> +}
> +#endif /* CONFIG_SECURITY_NETWORK && CONFIG_SECURITY_PATH */
> +
> #ifdef CONFIG_SECURITY_INFINIBAND
> int security_ib_pkey_access(void *sec, u64 subnet_prefix, u16 pkey);
> int security_ib_endport_manage_subnet(void *sec, const char *name, u8 port_num);
> diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
> index f6d56e70c7a2..41698460194b 100644
> --- a/net/unix/af_unix.c
> +++ b/net/unix/af_unix.c
> @@ -1231,10 +1231,17 @@ static struct sock *unix_find_bsd(struct sockaddr_un *sunaddr, int addr_len,
> goto path_put;
>
> err = -EPROTOTYPE;
> - if (sk->sk_type == type)
> - touch_atime(&path);
> - else
> + if (sk->sk_type != type)
> + goto sock_put;
> +
> + /*
> + * We call the hook because we know that the inode is a socket and we
> + * hold a valid reference to it via the path.
> + */
> + err = security_unix_find(&path, sk, flags);
> + if (err)
> goto sock_put;
> + touch_atime(&path);
>
> path_put(&path);
>
> diff --git a/security/security.c b/security/security.c
> index 67af9228c4e9..c73196b8db4b 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -4731,6 +4731,26 @@ int security_mptcp_add_subflow(struct sock *sk, struct sock *ssk)
>
> #endif /* CONFIG_SECURITY_NETWORK */
>
> +#if defined(CONFIG_SECURITY_NETWORK) && defined(CONFIG_SECURITY_PATH)
> +/**
> + * security_unix_find() - Check if a named AF_UNIX socket can connect
> + * @path: path of the socket being connected to
> + * @other: peer sock
> + * @flags: flags associated with the socket
> + *
> + * This hook is called to check permissions before connecting to a named
> + * AF_UNIX socket.
> + *
> + * Return: Returns 0 if permission is granted.
> + */
> +int security_unix_find(const struct path *path, struct sock *other, int flags)
> +{
> + return call_int_hook(unix_find, path, other, flags);
> +}
> +EXPORT_SYMBOL(security_unix_find);
> +
> +#endif /* CONFIG_SECURITY_NETWORK && CONFIG_SECURITY_PATH */
> +
> #ifdef CONFIG_SECURITY_INFINIBAND
> /**
> * security_ib_pkey_access() - Check if access to an IB pkey is allowed
> --
> 2.52.0
>
Reviewed-by: Günther Noack <gnoack3000@gmail.com>
Thank you, this looks good. I'll include it in the next version of the
Unix connect patch set again.
–Günther
^ permalink raw reply
* [PATCH v6] lsm: Add LSM hook security_unix_find
From: Justin Suess @ 2026-02-19 20:04 UTC (permalink / raw)
To: utilityemal77
Cc: brauner, demiobenour, fahimitahera, gnoack3000, hi, horms,
ivanov.mikhail1, jannh, jmorris, john.johansen,
konstantin.meskhidze, linux-security-module, m, matthieu, mic,
netdev, paul, samasth.norway.ananda, serge, viro
In-Reply-To: <aZcPd3OKqxwCZV-5@suesslenovo>
Add a LSM hook security_unix_find.
This hook is called to check the path of a named unix socket before a
connection is initiated. The peer socket may be inspected as well.
Why existing hooks are unsuitable:
Existing socket hooks, security_unix_stream_connect(),
security_unix_may_send(), and security_socket_connect() don't provide
TOCTOU-free / namespace independent access to the paths of sockets.
(1) We cannot resolve the path from the struct sockaddr in existing hooks.
This requires another path lookup. A change in the path between the
two lookups will cause a TOCTOU bug.
(2) We cannot use the struct path from the listening socket, because it
may be bound to a path in a different namespace than the caller,
resulting in a path that cannot be referenced at policy creation time.
Cc: Günther Noack <gnoack3000@gmail.com>
Cc: Tingmao Wang <m@maowtm.org>
Signed-off-by: Justin Suess <utilityemal77@gmail.com>
---
include/linux/lsm_hook_defs.h | 5 +++++
include/linux/security.h | 11 +++++++++++
net/unix/af_unix.c | 13 ++++++++++---
security/security.c | 20 ++++++++++++++++++++
4 files changed, 46 insertions(+), 3 deletions(-)
diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
index 8c42b4bde09c..7a0fd3dbfa29 100644
--- a/include/linux/lsm_hook_defs.h
+++ b/include/linux/lsm_hook_defs.h
@@ -317,6 +317,11 @@ LSM_HOOK(int, 0, post_notification, const struct cred *w_cred,
LSM_HOOK(int, 0, watch_key, struct key *key)
#endif /* CONFIG_SECURITY && CONFIG_KEY_NOTIFICATIONS */
+#if defined(CONFIG_SECURITY_NETWORK) && defined(CONFIG_SECURITY_PATH)
+LSM_HOOK(int, 0, unix_find, const struct path *path, struct sock *other,
+ int flags)
+#endif /* CONFIG_SECURITY_NETWORK && CONFIG_SECURITY_PATH */
+
#ifdef CONFIG_SECURITY_NETWORK
LSM_HOOK(int, 0, unix_stream_connect, struct sock *sock, struct sock *other,
struct sock *newsk)
diff --git a/include/linux/security.h b/include/linux/security.h
index 83a646d72f6f..99a33d8eb28d 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -1931,6 +1931,17 @@ static inline int security_mptcp_add_subflow(struct sock *sk, struct sock *ssk)
}
#endif /* CONFIG_SECURITY_NETWORK */
+#if defined(CONFIG_SECURITY_NETWORK) && defined(CONFIG_SECURITY_PATH)
+
+int security_unix_find(const struct path *path, struct sock *other, int flags);
+
+#else /* CONFIG_SECURITY_NETWORK && CONFIG_SECURITY_PATH */
+static inline int security_unix_find(const struct path *path, struct sock *other, int flags)
+{
+ return 0;
+}
+#endif /* CONFIG_SECURITY_NETWORK && CONFIG_SECURITY_PATH */
+
#ifdef CONFIG_SECURITY_INFINIBAND
int security_ib_pkey_access(void *sec, u64 subnet_prefix, u16 pkey);
int security_ib_endport_manage_subnet(void *sec, const char *name, u8 port_num);
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index f6d56e70c7a2..41698460194b 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -1231,10 +1231,17 @@ static struct sock *unix_find_bsd(struct sockaddr_un *sunaddr, int addr_len,
goto path_put;
err = -EPROTOTYPE;
- if (sk->sk_type == type)
- touch_atime(&path);
- else
+ if (sk->sk_type != type)
+ goto sock_put;
+
+ /*
+ * We call the hook because we know that the inode is a socket and we
+ * hold a valid reference to it via the path.
+ */
+ err = security_unix_find(&path, sk, flags);
+ if (err)
goto sock_put;
+ touch_atime(&path);
path_put(&path);
diff --git a/security/security.c b/security/security.c
index 67af9228c4e9..c73196b8db4b 100644
--- a/security/security.c
+++ b/security/security.c
@@ -4731,6 +4731,26 @@ int security_mptcp_add_subflow(struct sock *sk, struct sock *ssk)
#endif /* CONFIG_SECURITY_NETWORK */
+#if defined(CONFIG_SECURITY_NETWORK) && defined(CONFIG_SECURITY_PATH)
+/**
+ * security_unix_find() - Check if a named AF_UNIX socket can connect
+ * @path: path of the socket being connected to
+ * @other: peer sock
+ * @flags: flags associated with the socket
+ *
+ * This hook is called to check permissions before connecting to a named
+ * AF_UNIX socket.
+ *
+ * Return: Returns 0 if permission is granted.
+ */
+int security_unix_find(const struct path *path, struct sock *other, int flags)
+{
+ return call_int_hook(unix_find, path, other, flags);
+}
+EXPORT_SYMBOL(security_unix_find);
+
+#endif /* CONFIG_SECURITY_NETWORK && CONFIG_SECURITY_PATH */
+
#ifdef CONFIG_SECURITY_INFINIBAND
/**
* security_ib_pkey_access() - Check if access to an IB pkey is allowed
--
2.52.0
^ permalink raw reply related
* Re: [PATCH v4 15/17] module: Introduce hash-based integrity checking
From: Nicolas Schier @ 2026-02-19 14:27 UTC (permalink / raw)
To: Thomas Weißschuh
Cc: Petr Pavlu, Nathan Chancellor, Arnd Bergmann, Luis Chamberlain,
Sami Tolvanen, Daniel Gomez, Paul Moore, James Morris,
Serge E. Hallyn, Jonathan Corbet, Madhavan Srinivasan,
Michael Ellerman, Nicholas Piggin, Naveen N Rao, Mimi Zohar,
Roberto Sassu, Dmitry Kasatkin, Eric Snowberg, Daniel Gomez,
Aaron Tomlin, Christophe Leroy (CS GROUP), Nicolas Bouchinet,
Xiu Jianfeng, Fabian Grünbichler, Arnout Engelen,
Mattia Rizzolo, kpcyrd, Christian Heusel, Câju Mihai-Drosi,
Sebastian Andrzej Siewior, linux-kbuild, linux-kernel, linux-arch,
linux-modules, linux-security-module, linux-doc, linuxppc-dev,
linux-integrity
In-Reply-To: <28cf8d51-7530-41d5-a47b-cad5ecabd269@t-8ch.de>
On Tue, Feb 03, 2026 at 01:55:05PM +0100, Thomas Weißschuh wrote:
> On 2026-01-30 18:06:20+0100, Petr Pavlu wrote:
> > On 1/13/26 1:28 PM, Thomas Weißschuh wrote:
> > > Normally the .ko module files depend on a fully built vmlinux to be
> > > available for modpost validation and BTF generation. With
> > > CONFIG_MODULE_HASHES, vmlinux now depends on the modules
> > > to build a merkle tree. This introduces a dependency cycle which is
> > > impossible to satisfy. Work around this by building the modules during
> > > link-vmlinux.sh, after vmlinux is complete enough for modpost and BTF
> > > but before the final module hashes are
> >
> > I wonder if this dependency cycle could be resolved by utilizing the
> > split into vmlinux.unstripped and vmlinux that occurred last year.
> >
> > The idea is to create the following ordering: vmlinux.unstripped ->
> > modules -> vmlinux, and to patch in .module_hashes only when building
> > the final vmlinux.
> >
> > This would require the following:
> > * Split scripts/Makefile.vmlinux into two Makefiles, one that builds the
> > current vmlinux.unstripped and the second one that builds the final
> > vmlinux from it.
> > * Modify the top Makefile to recognize vmlinux.unstripped and update the
> > BTF generation rule 'modules: vmlinux' to
> > 'modules: vmlinux.unstripped'.
> > * Add the 'vmlinux: modules' ordering in the top Makefile for
> > CONFIG_MODULE_HASHES=y.
> > * Remove the patching of vmlinux.unstripped in scripts/link-vmlinux.sh
> > and instead move it into scripts/Makefile.vmlinux when running objcopy
> > to produce the final vmlinux.
> >
> > I think this approach has two main advantages:
> > * CONFIG_MODULE_HASHES can be made orthogonal to
> > CONFIG_DEBUG_INFO_BTF_MODULES.
> > * All dependencies are expressed at the Makefile level instead of having
> > scripts/link-vmlinux.sh invoke 'make -f Makefile modules'.
> >
> > Below is a rough prototype that applies on top of this series. It is a
> > bit verbose due to the splitting of part of scripts/Makefile.vmlinux
> > into scripts/Makefile.vmlinux_unstripped.
>
> That looks like a feasible alternative. Before adopting it, I'd like to
> hear the preference of the kbuild folks.
>
> > diff --git a/Makefile b/Makefile
> > index 841772a5a260..19a3beb82fa7 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -1259,7 +1259,7 @@ vmlinux_o: vmlinux.a $(KBUILD_VMLINUX_LIBS)
> > vmlinux.o modules.builtin.modinfo modules.builtin: vmlinux_o
> > @:
> >
> > -PHONY += vmlinux
> > +PHONY += vmlinux.unstripped vmlinux
> > # LDFLAGS_vmlinux in the top Makefile defines linker flags for the top vmlinux,
> > # not for decompressors. LDFLAGS_vmlinux in arch/*/boot/compressed/Makefile is
> > # unrelated; the decompressors just happen to have the same base name,
> > @@ -1270,9 +1270,11 @@ PHONY += vmlinux
> > # https://savannah.gnu.org/bugs/?61463
> > # For Make > 4.4, the following simple code will work:
> > # vmlinux: private export LDFLAGS_vmlinux := $(LDFLAGS_vmlinux)
> > -vmlinux: private _LDFLAGS_vmlinux := $(LDFLAGS_vmlinux)
> > -vmlinux: export LDFLAGS_vmlinux = $(_LDFLAGS_vmlinux)
> > -vmlinux: vmlinux.o $(KBUILD_LDS) modpost
> > +vmlinux.unstripped: private _LDFLAGS_vmlinux := $(LDFLAGS_vmlinux)
> > +vmlinux.unstripped: export LDFLAGS_vmlinux = $(_LDFLAGS_vmlinux)
> > +vmlinux.unstripped: vmlinux.o $(KBUILD_LDS) modpost
> > + $(Q)$(MAKE) -f $(srctree)/scripts/Makefile.vmlinux_unstripped
> > +vmlinux: vmlinux.unstripped
> > $(Q)$(MAKE) -f $(srctree)/scripts/Makefile.vmlinux
>
> Maybe we could keep them together in a single Makefile,
> and instead have different targets in it.
>
yes, I think so, too. I like the Petr's alternative.
Kind regards,
Nicolas
^ permalink raw reply
* Re: [PATCH v2 v2] evm: check return values of crypto_shash functions
From: Daniel Hodges @ 2026-02-19 15:01 UTC (permalink / raw)
To: Roberto Sassu
Cc: Daniel Hodges, zohar, roberto.sassu, dmitry.kasatkin,
eric.snowberg, paul, jmorris, serge, linux-integrity,
linux-security-module, linux-kernel
In-Reply-To: <cfbd9e3c63e03500279198ec7a80ba009dfccc63.camel@huaweicloud.com>
On Thu, Feb 19, 2026 at 01:36:39PM +0100, Roberto Sassu wrote:
> On Thu, 2026-02-19 at 10:26 +0100, Roberto Sassu wrote:
> > On Thu, 2026-02-05 at 21:42 -0500, Daniel Hodges wrote:
> > > The crypto_shash_update() and crypto_shash_final() functions can fail
> > > and return error codes, but their return values were not being checked
> > > in several places in security/integrity/evm/evm_crypto.c:
> > >
> > > - hmac_add_misc() ignored returns from crypto_shash_update() and
> > > crypto_shash_final()
> > > - evm_calc_hmac_or_hash() ignored returns from crypto_shash_update()
> > > - evm_init_hmac() ignored returns from crypto_shash_update()
> > >
> > > If these hash operations fail silently, the resulting HMAC could be
> > > invalid or incomplete, which could weaken the integrity verification
> > > security that EVM provides.
> > >
> > > This patch converts hmac_add_misc() from void to int return type and
> > > adds proper error checking and propagation for all crypto_shash_*
> > > function calls. All callers are updated to handle the new return values.
> > > Additionally, error messages are logged when cryptographic operations
> > > fail to provide visibility into the failure rather than silently
> > > returning error codes.
> > >
> > > Fixes: 66dbc325afce ("evm: re-release")
> > > Signed-off-by: Daniel Hodges <git@danielhodges.dev>
> >
> > After fixing the minor issue below:
>
> Already did it. The patch is here (after fixing a conflict with
> 0496fc9cdc38 "evm: Use ordered xattrs list to calculate HMAC in
> evm_init_hmac()"):
>
> https://github.com/robertosassu/linux/commit/d5aba42198b602c6de002ef02a4e6cc1d75652d7
>
> Roberto
Nice, thanks for handling that!
-Daniel
^ permalink raw reply
* Re: [PATCH] task: delete task_euid()
From: Gary Guo @ 2026-02-19 14:43 UTC (permalink / raw)
To: Alice Ryhl
Cc: Paul Moore, Serge Hallyn, Jonathan Corbet, Greg Kroah-Hartman,
Shuah Khan, Alex Shi, Yanteng Si, Dongliang Mu, Miguel Ojeda,
Boqun Feng, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Trevor Gross, Danilo Krummrich, linux-security-module, linux-doc,
linux-kernel, rust-for-linux, Jann Horn
In-Reply-To: <20260219-remove-task-euid-v1-1-904060826e07@google.com>
On 2026-02-19 12:14, Alice Ryhl wrote:
> task_euid() is a very weird operation. You can see how weird it is by
> grepping for task_euid() - binder is its only user. task_euid() obtains
> the objective effective UID - it looks at the credentials of the task
> for purposes of acting on it as an object, but then accesses the
> effective UID (which the credentials.7 man page describes as "[...]
> used
> by the kernel to determine the permissions that the process will have
> when accessing shared resources [...]").
>
> Since usage in Binder has now been removed, get rid of the resulting
> dead code.
>
> Changes to the zh_CN translation was carried out with the help of
> Gemini and Google Translate.
>
> Suggested-by: Jann Horn <jannh@google.com>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
Reviewed-by: Gary Guo <gary@garyguo.net>
With Alex's translation suggestion applied.
Thanks,
Gary
> ---
> Depends on these two changes:
> https://lore.kernel.org/all/20260212-rust-uid-v1-1-deff4214c766@google.com/
> https://lore.kernel.org/all/20260213-binder-uid-v1-0-7b795ae05523@google.com/
> ---
> Documentation/security/credentials.rst | 6 ++----
> Documentation/translations/zh_CN/security/credentials.rst | 6 ++----
> include/linux/cred.h | 1 -
> rust/helpers/task.c | 5 -----
> rust/kernel/task.rs | 10
> ----------
> 5 files changed, 4 insertions(+), 24 deletions(-)
^ permalink raw reply
* Re: [PATCH] task: delete task_euid()
From: Alice Ryhl @ 2026-02-19 14:35 UTC (permalink / raw)
To: Alex Shi
Cc: Paul Moore, Serge Hallyn, Jonathan Corbet, Greg Kroah-Hartman,
Shuah Khan, Alex Shi, Yanteng Si, Dongliang Mu, Miguel Ojeda,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Trevor Gross, Danilo Krummrich,
linux-security-module, linux-doc, linux-kernel, rust-for-linux,
Jann Horn
In-Reply-To: <c2b65a8f-b0cc-4bba-8ef9-7a079314d52d@gmail.com>
On Thu, Feb 19, 2026 at 10:27:01PM +0800, Alex Shi wrote:
>
>
> On 2026/2/19 20:14, Alice Ryhl wrote:
> > task_euid() is a very weird operation. You can see how weird it is by
> > grepping for task_euid() - binder is its only user. task_euid() obtains
> > the objective effective UID - it looks at the credentials of the task
> > for purposes of acting on it as an object, but then accesses the
> > effective UID (which the credentials.7 man page describes as "[...] used
> > by the kernel to determine the permissions that the process will have
> > when accessing shared resources [...]").
> >
> > Since usage in Binder has now been removed, get rid of the resulting
> > dead code.
> >
> > Changes to the zh_CN translation was carried out with the help of
> > Gemini and Google Translate.
> > diff --git a/Documentation/translations/zh_CN/security/credentials.rst b/Documentation/translations/zh_CN/security/credentials.rst
> > index 88fcd9152ffe91d79fc10bfc7b2a37d301b4938a..f0b2efec342438b81be415dc513622c961bb7e59 100644
> > --- a/Documentation/translations/zh_CN/security/credentials.rst
> > +++ b/Documentation/translations/zh_CN/security/credentials.rst
> > @@ -337,15 +337,13 @@ const指针上操作，因此不需要进行类型转换，但需要临时放弃
> > ``__task_cred()`` 的结果不应直接传递给 ``get_cred()`` ，
> > 因为这可能与 ``commit_cred()`` 发生竞争条件。
> > -还有一些方便的函数可以访问另一个任务凭据的特定部分，将RCU操作对调用方隐藏起来::
> > +有一个方便的函数可用于访问另一个任务凭据的特定部分，从而对调用方隐藏RCU机制::
>
> LGTM.
>
> > uid_t task_uid(task) Task's real UID
> > - uid_t task_euid(task) Task's effective UID
> > -如果调用方在此时已经持有RCU读锁，则应使用::
> > +如果调用方在此时已经持有RCU读锁，则应改为使用::
>
> Please keep the old version. the new one is a bit ambiguous.
Thanks for checking!
Alice
^ permalink raw reply
* Re: [PATCH] task: delete task_euid()
From: Alex Shi @ 2026-02-19 14:27 UTC (permalink / raw)
To: Alice Ryhl, Paul Moore, Serge Hallyn, Jonathan Corbet,
Greg Kroah-Hartman, Shuah Khan, Alex Shi, Yanteng Si,
Dongliang Mu
Cc: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, Danilo Krummrich,
linux-security-module, linux-doc, linux-kernel, rust-for-linux,
Jann Horn
In-Reply-To: <20260219-remove-task-euid-v1-1-904060826e07@google.com>
On 2026/2/19 20:14, Alice Ryhl wrote:
> task_euid() is a very weird operation. You can see how weird it is by
> grepping for task_euid() - binder is its only user. task_euid() obtains
> the objective effective UID - it looks at the credentials of the task
> for purposes of acting on it as an object, but then accesses the
> effective UID (which the credentials.7 man page describes as "[...] used
> by the kernel to determine the permissions that the process will have
> when accessing shared resources [...]").
>
> Since usage in Binder has now been removed, get rid of the resulting
> dead code.
>
> Changes to the zh_CN translation was carried out with the help of
> Gemini and Google Translate.
>
> Suggested-by: Jann Horn<jannh@google.com>
> Signed-off-by: Alice Ryhl<aliceryhl@google.com>
> ---
> Depends on these two changes:
> https://lore.kernel.org/all/20260212-rust-uid-v1-1-deff4214c766@google.com/
> https://lore.kernel.org/all/20260213-binder-uid-
> v1-0-7b795ae05523@google.com/
> ---
> Documentation/security/credentials.rst | 6 ++----
> Documentation/translations/zh_CN/security/credentials.rst | 6 ++----
> include/linux/cred.h | 1 -
> rust/helpers/task.c | 5 -----
> rust/kernel/task.rs | 10 ----------
> 5 files changed, 4 insertions(+), 24 deletions(-)
>
> diff --git a/Documentation/security/credentials.rst b/Documentation/security/credentials.rst
> index d0191c8b8060edb7b272402c019cff941ec22743..81d3b5737d85bde9b77bff94dfb93ed8037b2302 100644
> --- a/Documentation/security/credentials.rst
> +++ b/Documentation/security/credentials.rst
> @@ -393,16 +393,14 @@ the credentials so obtained when they're finished with.
> The result of ``__task_cred()`` should not be passed directly to
> ``get_cred()`` as this may race with ``commit_cred()``.
>
> -There are a couple of convenience functions to access bits of another task's
> -credentials, hiding the RCU magic from the caller::
> +There is a convenience function to access bits of another task's credentials,
> +hiding the RCU magic from the caller::
>
> uid_t task_uid(task) Task's real UID
> - uid_t task_euid(task) Task's effective UID
>
> If the caller is holding the RCU read lock at the time anyway, then::
>
> __task_cred(task)->uid
> - __task_cred(task)->euid
>
> should be used instead. Similarly, if multiple aspects of a task's credentials
> need to be accessed, RCU read lock should be used, ``__task_cred()`` called,
> diff --git a/Documentation/translations/zh_CN/security/credentials.rst b/Documentation/translations/zh_CN/security/credentials.rst
> index 88fcd9152ffe91d79fc10bfc7b2a37d301b4938a..f0b2efec342438b81be415dc513622c961bb7e59 100644
> --- a/Documentation/translations/zh_CN/security/credentials.rst
> +++ b/Documentation/translations/zh_CN/security/credentials.rst
> @@ -337,15 +337,13 @@ const指针上操作，因此不需要进行类型转换，但需要临时放弃
> ``__task_cred()`` 的结果不应直接传递给 ``get_cred()`` ，
> 因为这可能与 ``commit_cred()`` 发生竞争条件。
>
> -还有一些方便的函数可以访问另一个任务凭据的特定部分，将RCU操作对调用方隐藏起来::
> +有一个方便的函数可用于访问另一个任务凭据的特定部分，从而对调用方隐藏RCU机制::
LGTM.
>
> uid_t task_uid(task) Task's real UID
> - uid_t task_euid(task) Task's effective UID
>
> -如果调用方在此时已经持有RCU读锁，则应使用::
> +如果调用方在此时已经持有RCU读锁，则应改为使用::
Please keep the old version. the new one is a bit ambiguous.
Thanks
Alex
^ permalink raw reply
* Re: [PATCH v5 2/9] landlock: Control pathname UNIX domain socket resolution by path
From: Günther Noack @ 2026-02-19 13:59 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Günther Noack, John Johansen, Tingmao Wang, Justin Suess,
Jann Horn, linux-security-module, Samasth Norway Ananda,
Matthieu Buffet, Mikhail Ivanov, konstantin.meskhidze,
Demi Marie Obenour, Alyssa Ross, Tahera Fahimi
In-Reply-To: <20260219.IF4zee3Quo3j@digikod.net>
On Thu, Feb 19, 2026 at 10:45:44AM +0100, Mickaël Salaün wrote:
> On Wed, Feb 18, 2026 at 10:37:16AM +0100, Mickaël Salaün wrote:
> > On Sun, Feb 15, 2026 at 11:51:50AM +0100, Günther Noack wrote:
> > > * Add a new access right LANDLOCK_ACCESS_FS_RESOLVE_UNIX, which
> > > controls the look up operations for named UNIX domain sockets. The
> > > resolution happens during connect() and sendmsg() (depending on
> > > socket type).
> > > * Hook into the path lookup in unix_find_bsd() in af_unix.c, using a
> > > LSM hook. Make policy decisions based on the new access rights
> > > * Increment the Landlock ABI version.
> > > * Minor test adaptions to keep the tests working.
> > >
> > > With this access right, access is granted if either of the following
> > > conditions is met:
> > >
> > > * The target socket's filesystem path was allow-listed using a
> > > LANDLOCK_RULE_PATH_BENEATH rule, *or*:
> > > * The target socket was created in the same Landlock domain in which
> > > LANDLOCK_ACCESS_FS_RESOLVE_UNIX was restricted.
> > >
> > > In case of a denial, connect() and sendmsg() return EACCES, which is
> > > the same error as it is returned if the user does not have the write
> > > bit in the traditional Unix file system permissions of that file.
> > >
> > > This feature was created with substantial discussion and input from
> > > Justin Suess, Tingmao Wang and Mickaël Salaün.
> > >
> > > Cc: Tingmao Wang <m@maowtm.org>
> > > Cc: Justin Suess <utilityemal77@gmail.com>
> > > Cc: Mickaël Salaün <mic@digikod.net>
> > > Suggested-by: Jann Horn <jannh@google.com>
> > > Link: https://github.com/landlock-lsm/linux/issues/36
> > > Signed-off-by: Günther Noack <gnoack3000@gmail.com>
> > > ---
> > > include/uapi/linux/landlock.h | 10 ++
> > > security/landlock/access.h | 11 +-
> > > security/landlock/audit.c | 1 +
> > > security/landlock/fs.c | 102 ++++++++++++++++++-
> > > security/landlock/limits.h | 2 +-
> > > security/landlock/syscalls.c | 2 +-
> > > tools/testing/selftests/landlock/base_test.c | 2 +-
> > > tools/testing/selftests/landlock/fs_test.c | 5 +-
> > > 8 files changed, 128 insertions(+), 7 deletions(-)
>
> > > index 60ff217ab95b..8d0edf94037d 100644
> > > --- a/security/landlock/audit.c
> > > +++ b/security/landlock/audit.c
> > > @@ -37,6 +37,7 @@ static const char *const fs_access_strings[] = {
> > > [BIT_INDEX(LANDLOCK_ACCESS_FS_REFER)] = "fs.refer",
> > > [BIT_INDEX(LANDLOCK_ACCESS_FS_TRUNCATE)] = "fs.truncate",
> > > [BIT_INDEX(LANDLOCK_ACCESS_FS_IOCTL_DEV)] = "fs.ioctl_dev",
> > > + [BIT_INDEX(LANDLOCK_ACCESS_FS_RESOLVE_UNIX)] = "fs.resolve_unix",
> > > };
> > >
> > > static_assert(ARRAY_SIZE(fs_access_strings) == LANDLOCK_NUM_ACCESS_FS);
> > > diff --git a/security/landlock/fs.c b/security/landlock/fs.c
> > > index e764470f588c..76035c6f2bf1 100644
> > > --- a/security/landlock/fs.c
> > > +++ b/security/landlock/fs.c
> > > @@ -27,6 +27,7 @@
> > > #include <linux/lsm_hooks.h>
> > > #include <linux/mount.h>
> > > #include <linux/namei.h>
> > > +#include <linux/net.h>
> > > #include <linux/path.h>
> > > #include <linux/pid.h>
> > > #include <linux/rcupdate.h>
> > > @@ -314,7 +315,8 @@ static struct landlock_object *get_inode_object(struct inode *const inode)
> > > LANDLOCK_ACCESS_FS_WRITE_FILE | \
> > > LANDLOCK_ACCESS_FS_READ_FILE | \
> > > LANDLOCK_ACCESS_FS_TRUNCATE | \
> > > - LANDLOCK_ACCESS_FS_IOCTL_DEV)
> > > + LANDLOCK_ACCESS_FS_IOCTL_DEV | \
> > > + LANDLOCK_ACCESS_FS_RESOLVE_UNIX)
> > > /* clang-format on */
> > >
> > > /*
> > > @@ -1561,6 +1563,103 @@ static int hook_path_truncate(const struct path *const path)
> > > return current_check_access_path(path, LANDLOCK_ACCESS_FS_TRUNCATE);
> > > }
> > >
> > > +/**
> > > + * unmask_scoped_access - Remove access right bits in @masks in all layers
> > > + * where @client and @server have the same domain
> > > + *
> > > + * This does the same as domain_is_scoped(), but unmasks bits in @masks.
> > > + * It can not return early as domain_is_scoped() does.
>
> Why can't we use the same logic as for other scopes?
The other scopes, for which this is implemented in domain_is_scoped(),
do not need to do this layer-by-layer.
I have to admit, in my initial implementation, I was using
domain_is_scoped() directly, and the logic at the end of the hook was
roughly:
--- BUGGY CODE START ---
// ...
if (!domain_is_scoped(..., ..., LANDLOCK_ACCESS_FS_RESOLVE_UNIX))
return 0; /* permitted */
return current_check_access_path(path, LANDLOCK_ACCESS_FS_RESOLVE_UNIX)
}
--- BUGGY CODE END ---
Unfortunately, that is a logic error though -- it implements the formula
Access granted if:
(FOR-ALL l ∈ layers scoped-access-ok(l)) OR (FOR-ALL l ∈ layers path-access-ok(l)) (WRONG!)
but the formula we want is:
Access granted if:
FOR-ALL l ∈ layers (scoped-access-ok(l) OR path-access-ok(l)) (CORRECT!)
This makes a difference in the case where (pseudocode):
1. landlock_restrict_self(RESOLVE_UNIX) // d1
2. create_unix_server("./sock")
3. landlock_restrict_self(RESOLVE_UNIX, rule=Allow(".", RESOLVE_UNIX)) // d2
4. connect_unix("./sock")
,------------------------------------------------d1--,
| |
| ./sock server |
| ^ |
| | |
| ,------------------------------------------d2--, |
| | | | |
| | client | |
| | | |
| '----------------------------------------------' |
| |
'----------------------------------------------------'
(BTW, this scenario is covered in the selftests, that is why there is
a variant of these selftests where instead of applying "no domain", we
apply a domain with an exception rule like in step 3 in the pseudocode
above. Applying that domain should behave the same as applying no
domain at all.)
Intuitively, it is clear that the access should be granted:
- d1 does not restrict access to the server,
because the socket was created within d1 itself.
- d2 does not restrict access to the server,
because it has a rule to allow it
But the "buggy code" logic above comes to a different conclusion:
- the domain_is_scoped() check denies the access, because the server
is in a more privileged domain relative to the client domain.
- the current_check_access_path() check denies the access as well,
because the socket's path is not allow-listed in d1.
In the 'intuitive' reasoning above, we are checking d1 and d2
independently of each other. While Landlock is not implemented like
that internally, we need to stay consistent with it so that domains
compose correctly. The way to do that is to track is access check
results on a per-layer basis again, and that is why
unmask_scoped_access() uses a layer mask for tracking. The original
domain_is_scoped() does not use a layer mask, but that also means that
it can return early in some scenarios -- if for any of the relevant
layer depths, the client and server domains are not the same, it exits
early with failure because it's overall not fulfillable any more. In
the RESOLVE_UNIX case though, we need to remember in which layers we
failed (both high an low ones), because these layers can still be
fulfilled with a PATH_BENEATH rule later.
Summary:
Option 1: We *can* unify this if you want. It just might come at a
small performance penalty for domain_is_scoped(), which now uses the
larger layer mask data structure and can't do the same early returns
any more as before.
Option 2: Alternatively, if we move the two functions into the same
module, we can keep them separate but still test them against each
other to make sure they are in-line:
This invocation should return true...
domain_is_scoped(cli, srv, access)
...in the exactly the same situations where this invocation leaves any
bits set in layer_masks:
landlock_init_layer_masks(dom, access, &layer_masks, LL_KEY_INODE);
unmask_scoped_access(cli, srv, &layer_masks, access);
What do you prefer?
> > > + *
> > > + * @client: Client domain
> > > + * @server: Server domain
> > > + * @masks: Layer access masks to unmask
> > > + * @access: Access bit that controls scoping
> > > + */
> > > +static void unmask_scoped_access(const struct landlock_ruleset *const client,
> > > + const struct landlock_ruleset *const server,
> > > + struct layer_access_masks *const masks,
> > > + const access_mask_t access)
> >
> > This helper should be moved to task.c and factored out with
> > domain_is_scoped(). This should be a dedicated patch.
>
> Well, if domain_is_scoped() can be refactored and made generic, it would
> make more sense to move it to domain.c
>
> >
> > > +{
> > > + int client_layer, server_layer;
> > > + const struct landlock_hierarchy *client_walker, *server_walker;
> > > +
> > > + if (WARN_ON_ONCE(!client))
> > > + return; /* should not happen */
> > > +
> > > + if (!server)
> > > + return; /* server has no Landlock domain; nothing to clear */
> > > +
> > > + client_layer = client->num_layers - 1;
> > > + client_walker = client->hierarchy;
> > > + server_layer = server->num_layers - 1;
> > > + server_walker = server->hierarchy;
> > > +
> > > + /*
> > > + * Clears the access bits at all layers where the client domain is the
> > > + * same as the server domain. We start the walk at min(client_layer,
> > > + * server_layer). The layer bits until there can not be cleared because
> > > + * either the client or the server domain is missing.
> > > + */
> > > + for (; client_layer > server_layer; client_layer--)
> > > + client_walker = client_walker->parent;
> > > +
> > > + for (; server_layer > client_layer; server_layer--)
> > > + server_walker = server_walker->parent;
> > > +
> > > + for (; client_layer >= 0; client_layer--) {
> > > + if (masks->access[client_layer] & access &&
> > > + client_walker == server_walker)
> > > + masks->access[client_layer] &= ~access;
> > > +
> > > + client_walker = client_walker->parent;
> > > + server_walker = server_walker->parent;
> > > + }
> > > +}
^ permalink raw reply
* Re: [PATCH v5 1/9] lsm: Add LSM hook security_unix_find
From: Justin Suess @ 2026-02-19 13:26 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Günther Noack, John Johansen, Paul Moore, James Morris,
Serge E . Hallyn, Tingmao Wang, linux-security-module,
Samasth Norway Ananda, Matthieu Buffet, Mikhail Ivanov,
konstantin.meskhidze, Demi Marie Obenour, Alyssa Ross, Jann Horn,
Tahera Fahimi, Simon Horman, netdev, Alexander Viro,
Christian Brauner
In-Reply-To: <20260217.Nei3Aeg8bo6h@digikod.net>
On Wed, Feb 18, 2026 at 10:36:37AM +0100, Mickaël Salaün wrote:
> On Sun, Feb 15, 2026 at 11:51:49AM +0100, Günther Noack wrote:
> > From: Justin Suess <utilityemal77@gmail.com>
> >
> > Add a LSM hook security_unix_find.
> >
> > This hook is called to check the path of a named unix socket before a
> > connection is initiated. The peer socket may be inspected as well.
> >
> > Why existing hooks are unsuitable:
> >
> > Existing socket hooks, security_unix_stream_connect(),
> > security_unix_may_send(), and security_socket_connect() don't provide
> > TOCTOU-free / namespace independent access to the paths of sockets.
> >
> > (1) We cannot resolve the path from the struct sockaddr in existing hooks.
> > This requires another path lookup. A change in the path between the
> > two lookups will cause a TOCTOU bug.
> >
> > (2) We cannot use the struct path from the listening socket, because it
> > may be bound to a path in a different namespace than the caller,
> > resulting in a path that cannot be referenced at policy creation time.
> >
> > Cc: Günther Noack <gnoack3000@gmail.com>
> > Cc: Tingmao Wang <m@maowtm.org>
> > Signed-off-by: Justin Suess <utilityemal77@gmail.com>
> > ---
> > include/linux/lsm_hook_defs.h | 5 +++++
> > include/linux/security.h | 11 +++++++++++
> > net/unix/af_unix.c | 8 ++++++++
> > security/security.c | 20 ++++++++++++++++++++
> > 4 files changed, 44 insertions(+)
> >
> > diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
> > index 8c42b4bde09c..7a0fd3dbfa29 100644
> > --- a/include/linux/lsm_hook_defs.h
> > +++ b/include/linux/lsm_hook_defs.h
> > @@ -317,6 +317,11 @@ LSM_HOOK(int, 0, post_notification, const struct cred *w_cred,
> > LSM_HOOK(int, 0, watch_key, struct key *key)
> > #endif /* CONFIG_SECURITY && CONFIG_KEY_NOTIFICATIONS */
> >
> > +#if defined(CONFIG_SECURITY_NETWORK) && defined(CONFIG_SECURITY_PATH)
> > +LSM_HOOK(int, 0, unix_find, const struct path *path, struct sock *other,
> > + int flags)
> > +#endif /* CONFIG_SECURITY_NETWORK && CONFIG_SECURITY_PATH */
> > +
> > #ifdef CONFIG_SECURITY_NETWORK
> > LSM_HOOK(int, 0, unix_stream_connect, struct sock *sock, struct sock *other,
> > struct sock *newsk)
> > diff --git a/include/linux/security.h b/include/linux/security.h
> > index 83a646d72f6f..99a33d8eb28d 100644
> > --- a/include/linux/security.h
> > +++ b/include/linux/security.h
> > @@ -1931,6 +1931,17 @@ static inline int security_mptcp_add_subflow(struct sock *sk, struct sock *ssk)
> > }
> > #endif /* CONFIG_SECURITY_NETWORK */
> >
> > +#if defined(CONFIG_SECURITY_NETWORK) && defined(CONFIG_SECURITY_PATH)
> > +
> > +int security_unix_find(const struct path *path, struct sock *other, int flags);
> > +
> > +#else /* CONFIG_SECURITY_NETWORK && CONFIG_SECURITY_PATH */
> > +static inline int security_unix_find(const struct path *path, struct sock *other, int flags)
> > +{
> > + return 0;
> > +}
> > +#endif /* CONFIG_SECURITY_NETWORK && CONFIG_SECURITY_PATH */
> > +
> > #ifdef CONFIG_SECURITY_INFINIBAND
> > int security_ib_pkey_access(void *sec, u64 subnet_prefix, u16 pkey);
> > int security_ib_endport_manage_subnet(void *sec, const char *name, u8 port_num);
> > diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
> > index d0511225799b..369812b79dd8 100644
> > --- a/net/unix/af_unix.c
> > +++ b/net/unix/af_unix.c
> > @@ -1230,6 +1230,14 @@ static struct sock *unix_find_bsd(struct sockaddr_un *sunaddr, int addr_len,
> > if (!sk)
> > goto path_put;
> >
> > + /*
> > + * We call the hook because we know that the inode is a socket and we
> > + * hold a valid reference to it via the path.
> > + */
> > + err = security_unix_find(&path, sk, flags);
> > + if (err)
> > + goto sock_put;
> > +
> > err = -EPROTOTYPE;
> > if (sk->sk_type == type)
>
> I think this hook call should be moved here, just before the
> touch_atime() call for consistency with the socket type check, and to
> avoid doing useless check in the hook.
>
Agreed. One less annoyance for end users of the hook is a win. I'll
resend the hook with the call moved into this if (sk->sk_type == type)
block for the next version.
> > touch_atime(&path);
> > diff --git a/security/security.c b/security/security.c
> > index 67af9228c4e9..c73196b8db4b 100644
> > --- a/security/security.c
> > +++ b/security/security.c
> > @@ -4731,6 +4731,26 @@ int security_mptcp_add_subflow(struct sock *sk, struct sock *ssk)
> >
> > #endif /* CONFIG_SECURITY_NETWORK */
> >
> > +#if defined(CONFIG_SECURITY_NETWORK) && defined(CONFIG_SECURITY_PATH)
> > +/**
> > + * security_unix_find() - Check if a named AF_UNIX socket can connect
> > + * @path: path of the socket being connected to
> > + * @other: peer sock
> > + * @flags: flags associated with the socket
> > + *
> > + * This hook is called to check permissions before connecting to a named
> > + * AF_UNIX socket.
> > + *
> > + * Return: Returns 0 if permission is granted.
> > + */
> > +int security_unix_find(const struct path *path, struct sock *other, int flags)
> > +{
> > + return call_int_hook(unix_find, path, other, flags);
> > +}
> > +EXPORT_SYMBOL(security_unix_find);
> > +
> > +#endif /* CONFIG_SECURITY_NETWORK && CONFIG_SECURITY_PATH */
> > +
> > #ifdef CONFIG_SECURITY_INFINIBAND
> > /**
> > * security_ib_pkey_access() - Check if access to an IB pkey is allowed
> > --
> > 2.52.0
> >
> >
^ permalink raw reply
* Re: [PATCH v2 v2] evm: check return values of crypto_shash functions
From: Roberto Sassu @ 2026-02-19 12:36 UTC (permalink / raw)
To: Daniel Hodges
Cc: zohar, roberto.sassu, dmitry.kasatkin, eric.snowberg, paul,
jmorris, serge, linux-integrity, linux-security-module,
linux-kernel
In-Reply-To: <6ce273a26b396232f3ee64a980575562e766c501.camel@huaweicloud.com>
On Thu, 2026-02-19 at 10:26 +0100, Roberto Sassu wrote:
> On Thu, 2026-02-05 at 21:42 -0500, Daniel Hodges wrote:
> > The crypto_shash_update() and crypto_shash_final() functions can fail
> > and return error codes, but their return values were not being checked
> > in several places in security/integrity/evm/evm_crypto.c:
> >
> > - hmac_add_misc() ignored returns from crypto_shash_update() and
> > crypto_shash_final()
> > - evm_calc_hmac_or_hash() ignored returns from crypto_shash_update()
> > - evm_init_hmac() ignored returns from crypto_shash_update()
> >
> > If these hash operations fail silently, the resulting HMAC could be
> > invalid or incomplete, which could weaken the integrity verification
> > security that EVM provides.
> >
> > This patch converts hmac_add_misc() from void to int return type and
> > adds proper error checking and propagation for all crypto_shash_*
> > function calls. All callers are updated to handle the new return values.
> > Additionally, error messages are logged when cryptographic operations
> > fail to provide visibility into the failure rather than silently
> > returning error codes.
> >
> > Fixes: 66dbc325afce ("evm: re-release")
> > Signed-off-by: Daniel Hodges <git@danielhodges.dev>
>
> After fixing the minor issue below:
Already did it. The patch is here (after fixing a conflict with
0496fc9cdc38 "evm: Use ordered xattrs list to calculate HMAC in
evm_init_hmac()"):
https://github.com/robertosassu/linux/commit/d5aba42198b602c6de002ef02a4e6cc1d75652d7
Roberto
> Reviewed-by: Roberto Sassu <roberto.sassu@huawei.com>
>
> > ---
> > security/integrity/evm/evm_crypto.c | 55 ++++++++++++++++++++++-------
> > 1 file changed, 42 insertions(+), 13 deletions(-)
> >
> > diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
> > index a5e730ffda57..402eb1ca64ce 100644
> > --- a/security/integrity/evm/evm_crypto.c
> > +++ b/security/integrity/evm/evm_crypto.c
> > @@ -139,7 +139,7 @@ static struct shash_desc *init_desc(char type, uint8_t hash_algo)
> > * (Additional directory/file metadata needs to be added for more complete
> > * protection.)
> > */
> > -static void hmac_add_misc(struct shash_desc *desc, struct inode *inode,
> > +static int hmac_add_misc(struct shash_desc *desc, struct inode *inode,
> > char type, char *digest)
> > {
> > struct h_misc {
> > @@ -149,6 +149,7 @@ static void hmac_add_misc(struct shash_desc *desc, struct inode *inode,
> > gid_t gid;
> > umode_t mode;
> > } hmac_misc;
> > + int error;
> >
> > memset(&hmac_misc, 0, sizeof(hmac_misc));
> > /* Don't include the inode or generation number in portable
> > @@ -169,14 +170,28 @@ static void hmac_add_misc(struct shash_desc *desc, struct inode *inode,
> > hmac_misc.uid = from_kuid(&init_user_ns, inode->i_uid);
> > hmac_misc.gid = from_kgid(&init_user_ns, inode->i_gid);
> > hmac_misc.mode = inode->i_mode;
> > - crypto_shash_update(desc, (const u8 *)&hmac_misc, sizeof(hmac_misc));
> > + error = crypto_shash_update(desc, (const u8 *)&hmac_misc, sizeof(hmac_misc));
> > + if (error) {
> > + pr_err("crypto_shash_update() failed: %d\n", error);
> > + return error;
> > + }
> > if ((evm_hmac_attrs & EVM_ATTR_FSUUID) &&
> > - type != EVM_XATTR_PORTABLE_DIGSIG)
> > - crypto_shash_update(desc, (u8 *)&inode->i_sb->s_uuid, UUID_SIZE);
> > - crypto_shash_final(desc, digest);
> > + type != EVM_XATTR_PORTABLE_DIGSIG) {
> > + error = crypto_shash_update(desc, (u8 *)&inode->i_sb->s_uuid, UUID_SIZE);
> > + if (error) {
> > + pr_err("crypto_shash_update() failed: %d\n", error);
> > + return error;
> > + }
> > + }
> > + error = crypto_shash_final(desc, digest);
> > + if (error) {
> > + pr_err("crypto_shash_final() failed: %d\n", error);
> > + return error;
> > + }
> >
> > pr_debug("hmac_misc: (%zu) [%*phN]\n", sizeof(struct h_misc),
> > (int)sizeof(struct h_misc), &hmac_misc);
> > + return 0;
> > }
> >
> > /*
> > @@ -260,9 +275,12 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
> >
> > if ((req_xattr_name && req_xattr_value)
> > && !strcmp(xattr->name, req_xattr_name)) {
> > - error = 0;
> > - crypto_shash_update(desc, (const u8 *)req_xattr_value,
> > + error = crypto_shash_update(desc, (const u8 *)req_xattr_value,
> > req_xattr_value_len);
>
> Please align this.
>
> Thanks
>
> Roberto
>
> > + if (error) {
> > + pr_err("crypto_shash_update() failed: %d\n", error);
> > + goto out;
> > + }
> > if (is_ima)
> > ima_present = true;
> >
> > @@ -286,15 +304,20 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
> > pr_debug("file %s: xattr %s size mismatch (kernel: %d, user: %d)\n",
> > dentry->d_name.name, xattr->name, size,
> > user_space_size);
> > - error = 0;
> > xattr_size = size;
> > - crypto_shash_update(desc, (const u8 *)xattr_value, xattr_size);
> > + error = crypto_shash_update(desc, (const u8 *)xattr_value, xattr_size);
> > + if (error) {
> > + pr_err("crypto_shash_update() failed: %d\n", error);
> > + goto out;
> > + }
> > if (is_ima)
> > ima_present = true;
> >
> > dump_security_xattr(xattr->name, xattr_value, xattr_size);
> > }
> > - hmac_add_misc(desc, inode, type, data->digest);
> > + error = hmac_add_misc(desc, inode, type, data->digest);
> > + if (error)
> > + goto out;
> >
> > if (inode != d_backing_inode(dentry) && iint) {
> > if (IS_I_VERSION(inode))
> > @@ -401,6 +424,7 @@ int evm_init_hmac(struct inode *inode, const struct xattr *xattrs,
> > {
> > struct shash_desc *desc;
> > const struct xattr *xattr;
> > + int error;
> >
> > desc = init_desc(EVM_XATTR_HMAC, HASH_ALGO_SHA1);
> > if (IS_ERR(desc)) {
> > @@ -412,12 +436,17 @@ int evm_init_hmac(struct inode *inode, const struct xattr *xattrs,
> > if (!evm_protected_xattr(xattr->name))
> > continue;
> >
> > - crypto_shash_update(desc, xattr->value, xattr->value_len);
> > + error = crypto_shash_update(desc, xattr->value, xattr->value_len);
> > + if (error) {
> > + pr_err("crypto_shash_update() failed: %d\n", error);
> > + goto out;
> > + }
> > }
> >
> > - hmac_add_misc(desc, inode, EVM_XATTR_HMAC, hmac_val);
> > + error = hmac_add_misc(desc, inode, EVM_XATTR_HMAC, hmac_val);
> > +out:
> > kfree(desc);
> > - return 0;
> > + return error;
> > }
> >
> > /*
^ permalink raw reply
* [PATCH] task: delete task_euid()
From: Alice Ryhl @ 2026-02-19 12:14 UTC (permalink / raw)
To: Paul Moore, Serge Hallyn, Jonathan Corbet, Greg Kroah-Hartman,
Shuah Khan, Alex Shi, Yanteng Si, Dongliang Mu
Cc: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, Danilo Krummrich,
linux-security-module, linux-doc, linux-kernel, rust-for-linux,
Jann Horn, Alice Ryhl
task_euid() is a very weird operation. You can see how weird it is by
grepping for task_euid() - binder is its only user. task_euid() obtains
the objective effective UID - it looks at the credentials of the task
for purposes of acting on it as an object, but then accesses the
effective UID (which the credentials.7 man page describes as "[...] used
by the kernel to determine the permissions that the process will have
when accessing shared resources [...]").
Since usage in Binder has now been removed, get rid of the resulting
dead code.
Changes to the zh_CN translation was carried out with the help of
Gemini and Google Translate.
Suggested-by: Jann Horn <jannh@google.com>
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
Depends on these two changes:
https://lore.kernel.org/all/20260212-rust-uid-v1-1-deff4214c766@google.com/
https://lore.kernel.org/all/20260213-binder-uid-v1-0-7b795ae05523@google.com/
---
Documentation/security/credentials.rst | 6 ++----
Documentation/translations/zh_CN/security/credentials.rst | 6 ++----
include/linux/cred.h | 1 -
rust/helpers/task.c | 5 -----
rust/kernel/task.rs | 10 ----------
5 files changed, 4 insertions(+), 24 deletions(-)
diff --git a/Documentation/security/credentials.rst b/Documentation/security/credentials.rst
index d0191c8b8060edb7b272402c019cff941ec22743..81d3b5737d85bde9b77bff94dfb93ed8037b2302 100644
--- a/Documentation/security/credentials.rst
+++ b/Documentation/security/credentials.rst
@@ -393,16 +393,14 @@ the credentials so obtained when they're finished with.
The result of ``__task_cred()`` should not be passed directly to
``get_cred()`` as this may race with ``commit_cred()``.
-There are a couple of convenience functions to access bits of another task's
-credentials, hiding the RCU magic from the caller::
+There is a convenience function to access bits of another task's credentials,
+hiding the RCU magic from the caller::
uid_t task_uid(task) Task's real UID
- uid_t task_euid(task) Task's effective UID
If the caller is holding the RCU read lock at the time anyway, then::
__task_cred(task)->uid
- __task_cred(task)->euid
should be used instead. Similarly, if multiple aspects of a task's credentials
need to be accessed, RCU read lock should be used, ``__task_cred()`` called,
diff --git a/Documentation/translations/zh_CN/security/credentials.rst b/Documentation/translations/zh_CN/security/credentials.rst
index 88fcd9152ffe91d79fc10bfc7b2a37d301b4938a..f0b2efec342438b81be415dc513622c961bb7e59 100644
--- a/Documentation/translations/zh_CN/security/credentials.rst
+++ b/Documentation/translations/zh_CN/security/credentials.rst
@@ -337,15 +337,13 @@ const指针上操作，因此不需要进行类型转换，但需要临时放弃
``__task_cred()`` 的结果不应直接传递给 ``get_cred()`` ，
因为这可能与 ``commit_cred()`` 发生竞争条件。
-还有一些方便的函数可以访问另一个任务凭据的特定部分，将RCU操作对调用方隐藏起来::
+有一个方便的函数可用于访问另一个任务凭据的特定部分，从而对调用方隐藏RCU机制::
uid_t task_uid(task) Task's real UID
- uid_t task_euid(task) Task's effective UID
-如果调用方在此时已经持有RCU读锁，则应使用::
+如果调用方在此时已经持有RCU读锁，则应改为使用::
__task_cred(task)->uid
- __task_cred(task)->euid
类似地，如果需要访问任务凭据的多个方面，应使用RCU读锁，调用 ``__task_cred()``
函数，将结果存储在临时指针中，然后从临时指针中调用凭据的各个方面，最后释放锁。
diff --git a/include/linux/cred.h b/include/linux/cred.h
index ed1609d78cd7b16ed1434c937176495a4f38cf6e..b40ec3c72ee6673c7be5210a1667e3912cba9620 100644
--- a/include/linux/cred.h
+++ b/include/linux/cred.h
@@ -367,7 +367,6 @@ DEFINE_FREE(put_cred, struct cred *, if (!IS_ERR_OR_NULL(_T)) put_cred(_T))
})
#define task_uid(task) (task_cred_xxx((task), uid))
-#define task_euid(task) (task_cred_xxx((task), euid))
#define task_ucounts(task) (task_cred_xxx((task), ucounts))
#define current_cred_xxx(xxx) \
diff --git a/rust/helpers/task.c b/rust/helpers/task.c
index c0e1a06ede78c0b0641707b52a82725543e2c02c..b46b1433a67e8eb341a7ee32ca4247b304bf675f 100644
--- a/rust/helpers/task.c
+++ b/rust/helpers/task.c
@@ -28,11 +28,6 @@ __rust_helper kuid_t rust_helper_task_uid(struct task_struct *task)
return task_uid(task);
}
-__rust_helper kuid_t rust_helper_task_euid(struct task_struct *task)
-{
- return task_euid(task);
-}
-
#ifndef CONFIG_USER_NS
__rust_helper uid_t rust_helper_from_kuid(struct user_namespace *to, kuid_t uid)
{
diff --git a/rust/kernel/task.rs b/rust/kernel/task.rs
index e07d0ddd76f6917adc91ca3d17bb7719153ee17f..169ff1dde9363afc8914b431fe31f2238b213ada 100644
--- a/rust/kernel/task.rs
+++ b/rust/kernel/task.rs
@@ -218,16 +218,6 @@ pub fn uid(&self) -> Kuid {
Kuid::from_raw(unsafe { bindings::task_uid(self.as_ptr()) })
}
- /// Returns the objective effective UID of the given task.
- ///
- /// You should probably not be using this; the effective UID is normally
- /// only relevant in subjective credentials.
- #[inline]
- pub fn euid(&self) -> Kuid {
- // SAFETY: It's always safe to call `task_euid` on a valid task.
- Kuid::from_raw(unsafe { bindings::task_euid(self.as_ptr()) })
- }
-
/// Determines whether the given task has pending signals.
#[inline]
pub fn signal_pending(&self) -> bool {
---
base-commit: 2961f841b025fb234860bac26dfb7fa7cb0fb122
change-id: 20260219-remove-task-euid-19e4b00beebe
prerequisite-change-id: 20260212-rust-uid-f1b3a45c8084:v1
prerequisite-patch-id: 7ec4933af3a7f4c6bb0403c34a6dd41306836295
prerequisite-change-id: 20260213-binder-uid-a24ede5026a8:v1
prerequisite-patch-id: 7be0128bd8902879bb271d0587ac98bf242cf612
prerequisite-patch-id: 4a9d0f595d2084b3f8982a2d0d8b3df35b9fae0e
Best regards,
--
Alice Ryhl <aliceryhl@google.com>
^ permalink raw reply related
* Re: [PATCH v5 2/9] landlock: Control pathname UNIX domain socket resolution by path
From: Mickaël Salaün @ 2026-02-19 9:45 UTC (permalink / raw)
To: Günther Noack
Cc: John Johansen, Tingmao Wang, Justin Suess, Jann Horn,
linux-security-module, Samasth Norway Ananda, Matthieu Buffet,
Mikhail Ivanov, konstantin.meskhidze, Demi Marie Obenour,
Alyssa Ross, Tahera Fahimi
In-Reply-To: <20260217.lievaS8eeng8@digikod.net>
On Wed, Feb 18, 2026 at 10:37:16AM +0100, Mickaël Salaün wrote:
> On Sun, Feb 15, 2026 at 11:51:50AM +0100, Günther Noack wrote:
> > * Add a new access right LANDLOCK_ACCESS_FS_RESOLVE_UNIX, which
> > controls the look up operations for named UNIX domain sockets. The
> > resolution happens during connect() and sendmsg() (depending on
> > socket type).
> > * Hook into the path lookup in unix_find_bsd() in af_unix.c, using a
> > LSM hook. Make policy decisions based on the new access rights
> > * Increment the Landlock ABI version.
> > * Minor test adaptions to keep the tests working.
> >
> > With this access right, access is granted if either of the following
> > conditions is met:
> >
> > * The target socket's filesystem path was allow-listed using a
> > LANDLOCK_RULE_PATH_BENEATH rule, *or*:
> > * The target socket was created in the same Landlock domain in which
> > LANDLOCK_ACCESS_FS_RESOLVE_UNIX was restricted.
> >
> > In case of a denial, connect() and sendmsg() return EACCES, which is
> > the same error as it is returned if the user does not have the write
> > bit in the traditional Unix file system permissions of that file.
> >
> > This feature was created with substantial discussion and input from
> > Justin Suess, Tingmao Wang and Mickaël Salaün.
> >
> > Cc: Tingmao Wang <m@maowtm.org>
> > Cc: Justin Suess <utilityemal77@gmail.com>
> > Cc: Mickaël Salaün <mic@digikod.net>
> > Suggested-by: Jann Horn <jannh@google.com>
> > Link: https://github.com/landlock-lsm/linux/issues/36
> > Signed-off-by: Günther Noack <gnoack3000@gmail.com>
> > ---
> > include/uapi/linux/landlock.h | 10 ++
> > security/landlock/access.h | 11 +-
> > security/landlock/audit.c | 1 +
> > security/landlock/fs.c | 102 ++++++++++++++++++-
> > security/landlock/limits.h | 2 +-
> > security/landlock/syscalls.c | 2 +-
> > tools/testing/selftests/landlock/base_test.c | 2 +-
> > tools/testing/selftests/landlock/fs_test.c | 5 +-
> > 8 files changed, 128 insertions(+), 7 deletions(-)
> > index 60ff217ab95b..8d0edf94037d 100644
> > --- a/security/landlock/audit.c
> > +++ b/security/landlock/audit.c
> > @@ -37,6 +37,7 @@ static const char *const fs_access_strings[] = {
> > [BIT_INDEX(LANDLOCK_ACCESS_FS_REFER)] = "fs.refer",
> > [BIT_INDEX(LANDLOCK_ACCESS_FS_TRUNCATE)] = "fs.truncate",
> > [BIT_INDEX(LANDLOCK_ACCESS_FS_IOCTL_DEV)] = "fs.ioctl_dev",
> > + [BIT_INDEX(LANDLOCK_ACCESS_FS_RESOLVE_UNIX)] = "fs.resolve_unix",
> > };
> >
> > static_assert(ARRAY_SIZE(fs_access_strings) == LANDLOCK_NUM_ACCESS_FS);
> > diff --git a/security/landlock/fs.c b/security/landlock/fs.c
> > index e764470f588c..76035c6f2bf1 100644
> > --- a/security/landlock/fs.c
> > +++ b/security/landlock/fs.c
> > @@ -27,6 +27,7 @@
> > #include <linux/lsm_hooks.h>
> > #include <linux/mount.h>
> > #include <linux/namei.h>
> > +#include <linux/net.h>
> > #include <linux/path.h>
> > #include <linux/pid.h>
> > #include <linux/rcupdate.h>
> > @@ -314,7 +315,8 @@ static struct landlock_object *get_inode_object(struct inode *const inode)
> > LANDLOCK_ACCESS_FS_WRITE_FILE | \
> > LANDLOCK_ACCESS_FS_READ_FILE | \
> > LANDLOCK_ACCESS_FS_TRUNCATE | \
> > - LANDLOCK_ACCESS_FS_IOCTL_DEV)
> > + LANDLOCK_ACCESS_FS_IOCTL_DEV | \
> > + LANDLOCK_ACCESS_FS_RESOLVE_UNIX)
> > /* clang-format on */
> >
> > /*
> > @@ -1561,6 +1563,103 @@ static int hook_path_truncate(const struct path *const path)
> > return current_check_access_path(path, LANDLOCK_ACCESS_FS_TRUNCATE);
> > }
> >
> > +/**
> > + * unmask_scoped_access - Remove access right bits in @masks in all layers
> > + * where @client and @server have the same domain
> > + *
> > + * This does the same as domain_is_scoped(), but unmasks bits in @masks.
> > + * It can not return early as domain_is_scoped() does.
Why can't we use the same logic as for other scopes?
> > + *
> > + * @client: Client domain
> > + * @server: Server domain
> > + * @masks: Layer access masks to unmask
> > + * @access: Access bit that controls scoping
> > + */
> > +static void unmask_scoped_access(const struct landlock_ruleset *const client,
> > + const struct landlock_ruleset *const server,
> > + struct layer_access_masks *const masks,
> > + const access_mask_t access)
>
> This helper should be moved to task.c and factored out with
> domain_is_scoped(). This should be a dedicated patch.
Well, if domain_is_scoped() can be refactored and made generic, it would
make more sense to move it to domain.c
>
> > +{
> > + int client_layer, server_layer;
> > + const struct landlock_hierarchy *client_walker, *server_walker;
> > +
> > + if (WARN_ON_ONCE(!client))
> > + return; /* should not happen */
> > +
> > + if (!server)
> > + return; /* server has no Landlock domain; nothing to clear */
> > +
> > + client_layer = client->num_layers - 1;
> > + client_walker = client->hierarchy;
> > + server_layer = server->num_layers - 1;
> > + server_walker = server->hierarchy;
> > +
> > + /*
> > + * Clears the access bits at all layers where the client domain is the
> > + * same as the server domain. We start the walk at min(client_layer,
> > + * server_layer). The layer bits until there can not be cleared because
> > + * either the client or the server domain is missing.
> > + */
> > + for (; client_layer > server_layer; client_layer--)
> > + client_walker = client_walker->parent;
> > +
> > + for (; server_layer > client_layer; server_layer--)
> > + server_walker = server_walker->parent;
> > +
> > + for (; client_layer >= 0; client_layer--) {
> > + if (masks->access[client_layer] & access &&
> > + client_walker == server_walker)
> > + masks->access[client_layer] &= ~access;
> > +
> > + client_walker = client_walker->parent;
> > + server_walker = server_walker->parent;
> > + }
> > +}
^ permalink raw reply
* Re: [PATCH v2 v2] evm: check return values of crypto_shash functions
From: Roberto Sassu @ 2026-02-19 9:26 UTC (permalink / raw)
To: Daniel Hodges
Cc: zohar, roberto.sassu, dmitry.kasatkin, eric.snowberg, paul,
jmorris, serge, linux-integrity, linux-security-module,
linux-kernel
In-Reply-To: <20260206024240.19059-1-git@danielhodges.dev>
On Thu, 2026-02-05 at 21:42 -0500, Daniel Hodges wrote:
> The crypto_shash_update() and crypto_shash_final() functions can fail
> and return error codes, but their return values were not being checked
> in several places in security/integrity/evm/evm_crypto.c:
>
> - hmac_add_misc() ignored returns from crypto_shash_update() and
> crypto_shash_final()
> - evm_calc_hmac_or_hash() ignored returns from crypto_shash_update()
> - evm_init_hmac() ignored returns from crypto_shash_update()
>
> If these hash operations fail silently, the resulting HMAC could be
> invalid or incomplete, which could weaken the integrity verification
> security that EVM provides.
>
> This patch converts hmac_add_misc() from void to int return type and
> adds proper error checking and propagation for all crypto_shash_*
> function calls. All callers are updated to handle the new return values.
> Additionally, error messages are logged when cryptographic operations
> fail to provide visibility into the failure rather than silently
> returning error codes.
>
> Fixes: 66dbc325afce ("evm: re-release")
> Signed-off-by: Daniel Hodges <git@danielhodges.dev>
After fixing the minor issue below:
Reviewed-by: Roberto Sassu <roberto.sassu@huawei.com>
> ---
> security/integrity/evm/evm_crypto.c | 55 ++++++++++++++++++++++-------
> 1 file changed, 42 insertions(+), 13 deletions(-)
>
> diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
> index a5e730ffda57..402eb1ca64ce 100644
> --- a/security/integrity/evm/evm_crypto.c
> +++ b/security/integrity/evm/evm_crypto.c
> @@ -139,7 +139,7 @@ static struct shash_desc *init_desc(char type, uint8_t hash_algo)
> * (Additional directory/file metadata needs to be added for more complete
> * protection.)
> */
> -static void hmac_add_misc(struct shash_desc *desc, struct inode *inode,
> +static int hmac_add_misc(struct shash_desc *desc, struct inode *inode,
> char type, char *digest)
> {
> struct h_misc {
> @@ -149,6 +149,7 @@ static void hmac_add_misc(struct shash_desc *desc, struct inode *inode,
> gid_t gid;
> umode_t mode;
> } hmac_misc;
> + int error;
>
> memset(&hmac_misc, 0, sizeof(hmac_misc));
> /* Don't include the inode or generation number in portable
> @@ -169,14 +170,28 @@ static void hmac_add_misc(struct shash_desc *desc, struct inode *inode,
> hmac_misc.uid = from_kuid(&init_user_ns, inode->i_uid);
> hmac_misc.gid = from_kgid(&init_user_ns, inode->i_gid);
> hmac_misc.mode = inode->i_mode;
> - crypto_shash_update(desc, (const u8 *)&hmac_misc, sizeof(hmac_misc));
> + error = crypto_shash_update(desc, (const u8 *)&hmac_misc, sizeof(hmac_misc));
> + if (error) {
> + pr_err("crypto_shash_update() failed: %d\n", error);
> + return error;
> + }
> if ((evm_hmac_attrs & EVM_ATTR_FSUUID) &&
> - type != EVM_XATTR_PORTABLE_DIGSIG)
> - crypto_shash_update(desc, (u8 *)&inode->i_sb->s_uuid, UUID_SIZE);
> - crypto_shash_final(desc, digest);
> + type != EVM_XATTR_PORTABLE_DIGSIG) {
> + error = crypto_shash_update(desc, (u8 *)&inode->i_sb->s_uuid, UUID_SIZE);
> + if (error) {
> + pr_err("crypto_shash_update() failed: %d\n", error);
> + return error;
> + }
> + }
> + error = crypto_shash_final(desc, digest);
> + if (error) {
> + pr_err("crypto_shash_final() failed: %d\n", error);
> + return error;
> + }
>
> pr_debug("hmac_misc: (%zu) [%*phN]\n", sizeof(struct h_misc),
> (int)sizeof(struct h_misc), &hmac_misc);
> + return 0;
> }
>
> /*
> @@ -260,9 +275,12 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
>
> if ((req_xattr_name && req_xattr_value)
> && !strcmp(xattr->name, req_xattr_name)) {
> - error = 0;
> - crypto_shash_update(desc, (const u8 *)req_xattr_value,
> + error = crypto_shash_update(desc, (const u8 *)req_xattr_value,
> req_xattr_value_len);
Please align this.
Thanks
Roberto
> + if (error) {
> + pr_err("crypto_shash_update() failed: %d\n", error);
> + goto out;
> + }
> if (is_ima)
> ima_present = true;
>
> @@ -286,15 +304,20 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
> pr_debug("file %s: xattr %s size mismatch (kernel: %d, user: %d)\n",
> dentry->d_name.name, xattr->name, size,
> user_space_size);
> - error = 0;
> xattr_size = size;
> - crypto_shash_update(desc, (const u8 *)xattr_value, xattr_size);
> + error = crypto_shash_update(desc, (const u8 *)xattr_value, xattr_size);
> + if (error) {
> + pr_err("crypto_shash_update() failed: %d\n", error);
> + goto out;
> + }
> if (is_ima)
> ima_present = true;
>
> dump_security_xattr(xattr->name, xattr_value, xattr_size);
> }
> - hmac_add_misc(desc, inode, type, data->digest);
> + error = hmac_add_misc(desc, inode, type, data->digest);
> + if (error)
> + goto out;
>
> if (inode != d_backing_inode(dentry) && iint) {
> if (IS_I_VERSION(inode))
> @@ -401,6 +424,7 @@ int evm_init_hmac(struct inode *inode, const struct xattr *xattrs,
> {
> struct shash_desc *desc;
> const struct xattr *xattr;
> + int error;
>
> desc = init_desc(EVM_XATTR_HMAC, HASH_ALGO_SHA1);
> if (IS_ERR(desc)) {
> @@ -412,12 +436,17 @@ int evm_init_hmac(struct inode *inode, const struct xattr *xattrs,
> if (!evm_protected_xattr(xattr->name))
> continue;
>
> - crypto_shash_update(desc, xattr->value, xattr->value_len);
> + error = crypto_shash_update(desc, xattr->value, xattr->value_len);
> + if (error) {
> + pr_err("crypto_shash_update() failed: %d\n", error);
> + goto out;
> + }
> }
>
> - hmac_add_misc(desc, inode, EVM_XATTR_HMAC, hmac_val);
> + error = hmac_add_misc(desc, inode, EVM_XATTR_HMAC, hmac_val);
> +out:
> kfree(desc);
> - return 0;
> + return error;
> }
>
> /*
^ permalink raw reply
* Re: [PATCH v4] ima_fs: Avoid creating measurement lists for unsupported hash algos
From: Roberto Sassu @ 2026-02-19 8:54 UTC (permalink / raw)
To: dima, Mimi Zohar, Roberto Sassu, Dmitry Kasatkin, Eric Snowberg,
Paul Moore, James Morris, Serge E. Hallyn, Silvia Sisinni,
Enrico Bravi
Cc: linux-integrity, linux-security-module, linux-kernel, stable,
Dmitry Safonov
In-Reply-To: <701de3f87f0f6bde97872dd0c5bf150bfc1f2713.camel@huaweicloud.com>
On Tue, 2026-01-27 at 16:20 +0100, Roberto Sassu wrote:
> On Tue, 2026-01-27 at 15:03 +0000, Dmitry Safonov via B4 Relay wrote:
> > From: Dmitry Safonov <dima@arista.com>
> >
> > ima_init_crypto() skips initializing ima_algo_array[i] if the algorithm
> > from ima_tpm_chip->allocated_banks[i].crypto_id is not supported.
> > It seems avoid adding the unsupported algorithm to ima_algo_array will
> > break all the logic that relies on indexing by NR_BANKS(ima_tpm_chip).
>
> The patch looks good, although I didn't try yet myself.
>
> I would make the commit message slightly better, with a more fluid
> explanation.
>
> ima_tpm_chip->allocated_banks[i].crypto_id is initialized to
> HASH_ALGO__LAST if the TPM algorithm is not supported. However there
> are places relying on the algorithm to be valid because it is accessed
> by hash_algo_name[].
>
> Thus solve the problem by creating a file name that does not depend on
> the crypto algorithm to be initialized, ...
>
> Also print the template entry digest as populated by IMA.
>
> Something along these lines.
>
> Also, I have a preference for lower case instead of capital case for
> the file name, given the other names.
Hi Dmitry
do you have time to make these small changes, so that we queue the
patch for the next kernel?
Thanks
Roberto
> Could you also avoid the >, otherwise the mailer thinks it is a reply?
>
> Thanks
>
> Roberto
>
> > On 6.12.40 I observe the following read out-of-bounds in hash_algo_name:
> >
> > > ==================================================================
> > > BUG: KASAN: global-out-of-bounds in create_securityfs_measurement_lists+0x396/0x440
> > > Read of size 8 at addr ffffffff83e18138 by task swapper/0/1
> > >
> > > CPU: 4 UID: 0 PID: 1 Comm: swapper/0 Not tainted 6.12.40 #3
> > > Call Trace:
> > > <TASK>
> > > dump_stack_lvl+0x61/0x90
> > > print_report+0xc4/0x580
> > > ? kasan_addr_to_slab+0x26/0x80
> > > ? create_securityfs_measurement_lists+0x396/0x440
> > > kasan_report+0xc2/0x100
> > > ? create_securityfs_measurement_lists+0x396/0x440
> > > create_securityfs_measurement_lists+0x396/0x440
> > > ima_fs_init+0xa3/0x300
> > > ima_init+0x7d/0xd0
> > > init_ima+0x28/0x100
> > > do_one_initcall+0xa6/0x3e0
> > > kernel_init_freeable+0x455/0x740
> > > kernel_init+0x24/0x1d0
> > > ret_from_fork+0x38/0x80
> > > ret_from_fork_asm+0x11/0x20
> > > </TASK>
> > >
> > > The buggy address belongs to the variable:
> > > hash_algo_name+0xb8/0x420
> > >
> > > The buggy address belongs to the physical page:
> > > page: refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x107ce18
> > > flags: 0x8000000000002000(reserved|zone=2)
> > > raw: 8000000000002000 ffffea0041f38608 ffffea0041f38608 0000000000000000
> > > raw: 0000000000000000 0000000000000000 00000001ffffffff 0000000000000000
> > > page dumped because: kasan: bad access detected
> > >
> > > Memory state around the buggy address:
> > > ffffffff83e18000: 00 01 f9 f9 f9 f9 f9 f9 00 01 f9 f9 f9 f9 f9 f9
> > > ffffffff83e18080: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > > > ffffffff83e18100: 00 00 00 00 00 00 00 f9 f9 f9 f9 f9 00 05 f9 f9
> > > ^
> > > ffffffff83e18180: f9 f9 f9 f9 00 00 00 00 00 00 00 04 f9 f9 f9 f9
> > > ffffffff83e18200: 00 00 00 00 00 00 00 00 04 f9 f9 f9 f9 f9 f9 f9
> > > ==================================================================
> >
> > Seems like the TPM chip supports sha3_256, which isn't yet in
> > tpm_algorithms:
> > > tpm tpm0: TPM with unsupported bank algorithm 0x0027
> >
> > Use TPM_ALG_<ID> as a postfix for file names for unsupported hashing algorithms.
> >
> > This is how it looks on the test machine I have:
> > > # ls -1 /sys/kernel/security/ima/
> > > ascii_runtime_measurements
> > > ascii_runtime_measurements_TPM_ALG_27
> > > ascii_runtime_measurements_sha1
> > > ascii_runtime_measurements_sha256
> > > binary_runtime_measurements
> > > binary_runtime_measurements_TPM_ALG_27
> > > binary_runtime_measurements_sha1
> > > binary_runtime_measurements_sha256
> > > policy
> > > runtime_measurements_count
> > > violations
> >
> > Fixes: 9fa8e7625008 ("ima: add crypto agility support for template-hash algorithm")
> > Signed-off-by: Dmitry Safonov <dima@arista.com>
> > Cc: Enrico Bravi <enrico.bravi@polito.it>
> > Cc: Silvia Sisinni <silvia.sisinni@polito.it>
> > Cc: Roberto Sassu <roberto.sassu@huawei.com>
> > Cc: Mimi Zohar <zohar@linux.ibm.com>
> > ---
> > Changes in v4:
> > - Use ima_tpm_chip->allocated_banks[algo_idx].digest_size instead of hash_digest_size[algo]
> > (Roberto Sassu)
> > - Link to v3: https://lore.kernel.org/r/20260127-ima-oob-v3-1-1dd09f4c2a6a@arista.com
> > Testing note: I test it on v6.12.40 kernel backport, which slightly differs as
> > lookup_template_data_hash_algo() was yet present.
> >
> > Changes in v3:
> > - Now fix the spelling *for real* (sorry, messed it up in v2)
> > - Link to v2: https://lore.kernel.org/r/20260127-ima-oob-v2-1-f38a18c850cf@arista.com
> >
> > Changes in v2:
> > - Instead of skipping unknown algorithms, add files under their TPM_ALG_ID (Roberto Sassu)
> > - Fix spelling (Roberto Sassu)
> > - Copy @stable on the fix
> > - Link to v1: https://lore.kernel.org/r/20260127-ima-oob-v1-1-2d42f3418e57@arista.com
> > ---
> > security/integrity/ima/ima_fs.c | 34 ++++++++++++++++++----------------
> > 1 file changed, 18 insertions(+), 16 deletions(-)
> >
> > diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
> > index 012a58959ff0..9a00a0547619 100644
> > --- a/security/integrity/ima/ima_fs.c
> > +++ b/security/integrity/ima/ima_fs.c
> > @@ -132,16 +132,12 @@ int ima_measurements_show(struct seq_file *m, void *v)
> > char *template_name;
> > u32 pcr, namelen, template_data_len; /* temporary fields */
> > bool is_ima_template = false;
> > - enum hash_algo algo;
> > int i, algo_idx;
> >
> > algo_idx = ima_sha1_idx;
> > - algo = HASH_ALGO_SHA1;
> >
> > - if (m->file != NULL) {
> > + if (m->file != NULL)
> > algo_idx = (unsigned long)file_inode(m->file)->i_private;
> > - algo = ima_algo_array[algo_idx].algo;
> > - }
> >
> > /* get entry */
> > e = qe->entry;
> > @@ -160,7 +156,8 @@ int ima_measurements_show(struct seq_file *m, void *v)
> > ima_putc(m, &pcr, sizeof(e->pcr));
> >
> > /* 2nd: template digest */
> > - ima_putc(m, e->digests[algo_idx].digest, hash_digest_size[algo]);
> > + ima_putc(m, e->digests[algo_idx].digest,
> > + ima_tpm_chip->allocated_banks[algo_idx].digest_size);
> >
> > /* 3rd: template name size */
> > namelen = !ima_canonical_fmt ? strlen(template_name) :
> > @@ -229,16 +226,12 @@ static int ima_ascii_measurements_show(struct seq_file *m, void *v)
> > struct ima_queue_entry *qe = v;
> > struct ima_template_entry *e;
> > char *template_name;
> > - enum hash_algo algo;
> > int i, algo_idx;
> >
> > algo_idx = ima_sha1_idx;
> > - algo = HASH_ALGO_SHA1;
> >
> > - if (m->file != NULL) {
> > + if (m->file != NULL)
> > algo_idx = (unsigned long)file_inode(m->file)->i_private;
> > - algo = ima_algo_array[algo_idx].algo;
> > - }
> >
> > /* get entry */
> > e = qe->entry;
> > @@ -252,7 +245,8 @@ static int ima_ascii_measurements_show(struct seq_file *m, void *v)
> > seq_printf(m, "%2d ", e->pcr);
> >
> > /* 2nd: template hash */
> > - ima_print_digest(m, e->digests[algo_idx].digest, hash_digest_size[algo]);
> > + ima_print_digest(m, e->digests[algo_idx].digest,
> > + ima_tpm_chip->allocated_banks[algo_idx].digest_size);
> >
> > /* 3th: template name */
> > seq_printf(m, " %s", template_name);
> > @@ -404,16 +398,24 @@ static int __init create_securityfs_measurement_lists(void)
> > char file_name[NAME_MAX + 1];
> > struct dentry *dentry;
> >
> > - sprintf(file_name, "ascii_runtime_measurements_%s",
> > - hash_algo_name[algo]);
> > + if (algo == HASH_ALGO__LAST)
> > + sprintf(file_name, "ascii_runtime_measurements_TPM_ALG_%x",
> > + ima_tpm_chip->allocated_banks[i].alg_id);
> > + else
> > + sprintf(file_name, "ascii_runtime_measurements_%s",
> > + hash_algo_name[algo]);
> > dentry = securityfs_create_file(file_name, S_IRUSR | S_IRGRP,
> > ima_dir, (void *)(uintptr_t)i,
> > &ima_ascii_measurements_ops);
> > if (IS_ERR(dentry))
> > return PTR_ERR(dentry);
> >
> > - sprintf(file_name, "binary_runtime_measurements_%s",
> > - hash_algo_name[algo]);
> > + if (algo == HASH_ALGO__LAST)
> > + sprintf(file_name, "binary_runtime_measurements_TPM_ALG_%x",
> > + ima_tpm_chip->allocated_banks[i].alg_id);
> > + else
> > + sprintf(file_name, "binary_runtime_measurements_%s",
> > + hash_algo_name[algo]);
> > dentry = securityfs_create_file(file_name, S_IRUSR | S_IRGRP,
> > ima_dir, (void *)(uintptr_t)i,
> > &ima_measurements_ops);
> >
> > ---
> > base-commit: 63804fed149a6750ffd28610c5c1c98cce6bd377
> > change-id: 20260127-ima-oob-9fa83a634d7b
> >
> > Best regards,
^ permalink raw reply
* Re: [PATCH] ima: check return value of crypto_shash_final() in boot aggregate
From: Roberto Sassu @ 2026-02-19 8:56 UTC (permalink / raw)
To: Daniel Hodges, Mimi Zohar, Roberto Sassu, Dmitry Kasatkin
Cc: Eric Snowberg, Paul Moore, James Morris, Serge E . Hallyn,
linux-integrity, linux-security-module, linux-kernel
In-Reply-To: <20260201024015.2862236-1-hodgesd@meta.com>
On Sat, 2026-01-31 at 18:40 -0800, Daniel Hodges wrote:
> The return value of crypto_shash_final() is not checked in
> ima_calc_boot_aggregate_tfm(). If the hash finalization fails, the
> function returns success and a corrupted boot aggregate digest could
> be used for IMA measurements.
>
> Capture the return value and propagate any error to the caller.
>
> Fixes: 76bb28f6126f ("ima: use new crypto_shash API instead of old crypto_hash")
> Signed-off-by: Daniel Hodges <hodgesd@meta.com>
Reviewed-by: Roberto Sassu <roberto.sassu@huawei.com>
Thanks
Roberto
> ---
> security/integrity/ima/ima_crypto.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
> index 6f5696d999d0..8ae7821a65c2 100644
> --- a/security/integrity/ima/ima_crypto.c
> +++ b/security/integrity/ima/ima_crypto.c
> @@ -825,21 +825,21 @@ static int ima_calc_boot_aggregate_tfm(char *digest, u16 alg_id,
> * non-SHA1 boot_aggregate digests to avoid ambiguity.
> */
> if (alg_id != TPM_ALG_SHA1) {
> for (i = TPM_PCR8; i < TPM_PCR10; i++) {
> ima_pcrread(i, &d);
> rc = crypto_shash_update(shash, d.digest,
> crypto_shash_digestsize(tfm));
> }
> }
> if (!rc)
> - crypto_shash_final(shash, digest);
> + rc = crypto_shash_final(shash, digest);
> return rc;
> }
>
> int ima_calc_boot_aggregate(struct ima_digest_data *hash)
> {
> struct crypto_shash *tfm;
> u16 crypto_id, alg_id;
> int rc, i, bank_idx = -1;
>
> for (i = 0; i < ima_tpm_chip->nr_allocated_banks; i++) {
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox