mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 0/2] rust: add NonEmpty
@ 2026-10-12 11:46 Lorenzo Delgado
  2026-10-12 11:46 ` [PATCH 1/2] rust: non_empty: " Lorenzo Delgado
  2026-10-12 11:46 ` [PATCH 2/2] rust: uaccess: use NonEmpty in strcpy_into_buf() Lorenzo Delgado
  0 siblings, 2 replies; 5+ messages in thread
From: Lorenzo Delgado @ 2026-10-12 11:46 UTC (permalink / raw)
  Cc: linux-kernel, rust-for-linux, Thorsten Blum, Francis Laniel,
	Lorenzo Delgado

Some functions check that a container is not empty and later rely on it
through `unwrap_unchecked()`, `unwrap()` or an error path that can't be
taken. Gary suggested a non-empty type for this while reviewing Thorsten
Blum's uaccess patch [1], and Miguel filed it as #1262 [2].

Patch 1 adds `NonEmpty<C>` in `kernel::non_empty`, with only what the
first user needs: slices, `try_from_mut()` and `last_mut()`. Patch 2
uses it in `strcpy_into_buf()`, which drops an `unsafe` block.

Some notes:

- `IsEmpty` is a safe trait. Unsafe code only relies on the
  implementations in `kernel::non_empty`, and the docs require the
  result not to change while the container is only reachable through a
  shared reference. If a generic accessor ever needs to rely on it, it
  can become an `unsafe trait` then.

- Since APIs aren't added without users, other accessors (`first()`,
  `try_from_ref()`, ...) and containers are left for later. I plan to
  send follow-ups for `net/phy.rs` and `scatterlist.rs`, which both
  reject an empty slice and then rely on it.

- I put it in its own `kernel::non_empty` module. Would a broader home
  like `kernel::collections` be better?

Francis Laniel posted a slice-only RFC [3]. Following Gary's suggestion
on the issue, this version is generic over the container.

[1] https://lore.kernel.org/rust-for-linux/DM0D9TERQAM3.2Y3HV6P68E2N8@garyguo.net/
[2] https://github.com/Rust-for-Linux/linux/issues/1262
[3] https://lore.kernel.org/rust-for-linux/20261011191540.56199-1-laniel_francis@npnm.eu/

Signed-off-by: Lorenzo Delgado <lnsdev@proton.me>
---
Lorenzo Delgado (2):
      rust: non_empty: add NonEmpty
      rust: uaccess: use NonEmpty in strcpy_into_buf()

 rust/kernel/lib.rs       |   1 +
 rust/kernel/non_empty.rs | 100 +++++++++++++++++++++++++++++++++++++++++++++++
 rust/kernel/uaccess.rs   |  11 ++----
 3 files changed, 105 insertions(+), 7 deletions(-)
---
base-commit: 2dee3d3adcadcc57d62ba6608cd02f50b0c10264
change-id: 20261011-nonempty-slice-022084ba9a5a

Best regards,
--  
Lorenzo Delgado <lnsdev@proton.me>



^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/2] rust: non_empty: add NonEmpty
  2026-10-12 11:46 [PATCH 0/2] rust: add NonEmpty Lorenzo Delgado
@ 2026-10-12 11:46 ` Lorenzo Delgado
  2026-10-12 11:46 ` [PATCH 2/2] rust: uaccess: use NonEmpty in strcpy_into_buf() Lorenzo Delgado
  1 sibling, 0 replies; 5+ messages in thread
From: Lorenzo Delgado @ 2026-10-12 11:46 UTC (permalink / raw)
  Cc: linux-kernel, rust-for-linux, Thorsten Blum, Francis Laniel,
	Lorenzo Delgado

Some functions reject an empty container up front and later rely on it
having at least one element. The type system does not see that check,
so the later access ends up as an `unwrap_unchecked()` with a SAFETY
comment pointing back at it, or as an `unwrap()` or an error path that
can never be taken. `strcpy_into_buf()` in uaccess is one such case.

Add `NonEmpty<C>`, a transparent wrapper that can only be created from
a container its `IsEmpty` implementation reports as non-empty. It
dereferences to the container, and a method whose `Option` only exists
for the empty case can be shadowed by a version that returns the value
directly.

`DerefMut` is implemented per container, only where a mutable reference
cannot change the length. For a growable container it would let
`clear()` break the invariant.

For now only slices are supported, with the one constructor and
accessor that the uaccess conversion needs. Other containers and
accessors can follow with their users.

Suggested-by: Gary Guo <gary@garyguo.net>
Link: https://lore.kernel.org/rust-for-linux/DM0D9TERQAM3.2Y3HV6P68E2N8@garyguo.net/
Link: https://github.com/Rust-for-Linux/linux/issues/1262
Signed-off-by: Lorenzo Delgado <lnsdev@proton.me>
---
 rust/kernel/lib.rs       |   1 +
 rust/kernel/non_empty.rs | 100 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 101 insertions(+)

diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 7225abc64084..8e1928deac35 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -105,6 +105,7 @@
 pub mod module_param;
 #[cfg(CONFIG_NET)]
 pub mod net;
+pub mod non_empty;
 pub mod num;
 pub mod of;
 #[cfg(CONFIG_PM_OPP)]
diff --git a/rust/kernel/non_empty.rs b/rust/kernel/non_empty.rs
new file mode 100644
index 000000000000..9617414f51f9
--- /dev/null
+++ b/rust/kernel/non_empty.rs
@@ -0,0 +1,100 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Containers that are known not to be empty.
+
+use core::ops::{
+    Deref,
+    DerefMut, //
+};
+
+/// A container that can report whether it holds any element.
+///
+/// The result must not change while the container is only reachable through a shared reference.
+///
+/// Unsafe code may only rely on the result for a container type whose implementation it trusts,
+/// such as the ones in this module, since a safe implementation can return anything.
+pub trait IsEmpty {
+    /// Returns `true` if the container holds no element.
+    fn is_empty(&self) -> bool;
+}
+
+impl<T> IsEmpty for [T] {
+    #[inline]
+    fn is_empty(&self) -> bool {
+        <[T]>::is_empty(self)
+    }
+}
+
+/// A container that holds at least one element.
+///
+/// [`NonEmpty`] dereferences to the container it wraps, so the container's API stays available.
+/// Methods whose result is only optional because the container could be empty can be shadowed by
+/// versions that return the result directly. They are added as users need them. For now that is
+/// only [`NonEmpty::last_mut`] on a slice, which returns `&mut T` instead of `Option<&mut T>`.
+///
+/// # Invariants
+///
+/// The wrapped container is not empty. Unsafe code may rely on this only for container types
+/// whose [`IsEmpty`] implementation is in this module.
+///
+/// # Examples
+///
+/// ```
+/// use kernel::non_empty::NonEmpty;
+///
+/// let mut buf = [1u8, 2, 3];
+/// let slice = NonEmpty::try_from_mut(&mut buf[..]).ok_or(EINVAL)?;
+/// *slice.last_mut() = 0;
+/// assert_eq!(slice.len(), 3);
+/// assert_eq!(buf, [1, 2, 0]);
+///
+/// let mut empty: [u8; 0] = [];
+/// assert!(NonEmpty::try_from_mut(&mut empty[..]).is_none());
+/// # Ok::<(), Error>(())
+/// ```
+#[repr(transparent)]
+#[derive(Debug)]
+pub struct NonEmpty<C: ?Sized>(C);
+
+impl<C: ?Sized + IsEmpty> NonEmpty<C> {
+    /// Creates a [`NonEmpty`] from a mutable reference to a container, or returns [`None`] if the
+    /// container is empty.
+    #[inline]
+    pub fn try_from_mut(container: &mut C) -> Option<&mut Self> {
+        if container.is_empty() {
+            return None;
+        }
+
+        // INVARIANT: `container` was checked to be non-empty above.
+        // SAFETY: `NonEmpty<C>` is transparent to `C`, so the cast keeps both the layout and the
+        // pointer metadata, and the pointer comes from a valid `&mut C` with the same lifetime.
+        Some(unsafe { &mut *(core::ptr::from_mut(container) as *mut Self) })
+    }
+}
+
+impl<T> NonEmpty<[T]> {
+    /// Returns a mutable reference to the last element of the slice.
+    #[inline]
+    pub fn last_mut(&mut self) -> &mut T {
+        // SAFETY: By the type invariant, the slice is not empty, so it has a last element.
+        unsafe { self.0.last_mut().unwrap_unchecked() }
+    }
+}
+
+impl<C: ?Sized> Deref for NonEmpty<C> {
+    type Target = C;
+
+    #[inline]
+    fn deref(&self) -> &C {
+        &self.0
+    }
+}
+
+// `DerefMut` is only sound for containers whose length cannot change through a mutable
+// reference. For a growable container such as `KVec`, `clear()` would empty it.
+impl<T> DerefMut for NonEmpty<[T]> {
+    #[inline]
+    fn deref_mut(&mut self) -> &mut [T] {
+        &mut self.0
+    }
+}

-- 
2.55.0



^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 2/2] rust: uaccess: use NonEmpty in strcpy_into_buf()
  2026-10-12 11:46 [PATCH 0/2] rust: add NonEmpty Lorenzo Delgado
  2026-10-12 11:46 ` [PATCH 1/2] rust: non_empty: " Lorenzo Delgado
@ 2026-10-12 11:46 ` Lorenzo Delgado
  1 sibling, 0 replies; 5+ messages in thread
From: Lorenzo Delgado @ 2026-10-12 11:46 UTC (permalink / raw)
  Cc: linux-kernel, rust-for-linux, Thorsten Blum, Francis Laniel,
	Lorenzo Delgado

`strcpy_into_buf()` returns `EINVAL` for an empty buffer and later
writes the NUL terminator through `last_mut().unwrap_unchecked()`,
relying on that earlier check.

Turn the buffer into a `NonEmpty<[u8]>` at the start instead. The
emptiness check and its `EINVAL` stay the same, but `last_mut()` now
returns the element directly, so the `unsafe` block goes away.

The `MaybeUninit` cast still starts from `[u8]`, so its SAFETY comment
does not depend on the layout of the wrapper.

Suggested-by: Gary Guo <gary@garyguo.net>
Link: https://lore.kernel.org/rust-for-linux/DM0D9TERQAM3.2Y3HV6P68E2N8@garyguo.net/
Link: https://github.com/Rust-for-Linux/linux/issues/1262
Signed-off-by: Lorenzo Delgado <lnsdev@proton.me>
---
 rust/kernel/uaccess.rs | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/rust/kernel/uaccess.rs b/rust/kernel/uaccess.rs
index 5f6c4d7a1a51..044c20e87ee6 100644
--- a/rust/kernel/uaccess.rs
+++ b/rust/kernel/uaccess.rs
@@ -11,6 +11,7 @@
     error::Result,
     ffi::{c_char, c_void},
     fs::file,
+    non_empty::NonEmpty,
     prelude::*,
     ptr::KnownSize,
     transmute::{AsBytes, FromBytes},
@@ -390,13 +391,11 @@ pub fn read_all<A: Allocator>(mut self, buf: &mut Vec<u8, A>, flags: Flags) -> R
     /// copied).
     #[doc(alias = "strncpy_from_user")]
     pub fn strcpy_into_buf<'buf>(self, buf: &'buf mut [u8]) -> Result<&'buf CStr> {
-        if buf.is_empty() {
-            return Err(EINVAL);
-        }
+        let buf = NonEmpty::try_from_mut(buf).ok_or(EINVAL)?;
 
         // SAFETY: The types are compatible and `strncpy_from_user` doesn't write uninitialized
         // bytes to `buf`.
-        let mut dst = unsafe { &mut *(core::ptr::from_mut(buf) as *mut [MaybeUninit<u8>]) };
+        let mut dst = unsafe { &mut *(core::ptr::from_mut::<[u8]>(buf) as *mut [MaybeUninit<u8>]) };
 
         // We never read more than `self.length` bytes.
         if dst.len() > self.length {
@@ -422,9 +421,7 @@ pub fn strcpy_into_buf<'buf>(self, buf: &'buf mut [u8]) -> Result<&'buf CStr> {
             // This means that we filled the buffer exactly. In this case, we add a NUL-terminator
             // and return it. Unlike the `len < dst.len()` branch, don't modify `len` because it
             // already represents the length including the NUL-terminator.
-            //
-            // SAFETY: Due to the check at the beginning, the buffer is not empty.
-            unsafe { *buf.last_mut().unwrap_unchecked() = 0 };
+            *buf.last_mut() = 0;
         }
 
         // This method consumes `self`, so it can only be called once, thus we do not need to

-- 
2.55.0



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 2/2] rust: uaccess: use NonEmpty in strcpy_into_buf()
  2026-10-12 11:53 ` [PATCH 2/2] rust: uaccess: use NonEmpty in strcpy_into_buf() Lorenzo Delgado
@ 2026-10-12 12:22   ` Miguel Ojeda
  0 siblings, 0 replies; 5+ messages in thread
From: Miguel Ojeda @ 2026-10-12 12:22 UTC (permalink / raw)
  To: Lorenzo Delgado
  Cc: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Danilo Krummrich, Daniel Almeida, Tamir Duberstein,
	Alexandre Courbot, Onur Özkan, linux-kernel, rust-for-linux,
	Thorsten Blum, Francis Laniel

On Mon, Oct 12, 2026 at 1:54 PM Lorenzo Delgado <lnsdev@proton.me> wrote:
>
> Turn the buffer into a `NonEmpty<[u8]>` at the start instead. The

Hmm... I thought the idea was to request a `NonEmpty` in the signature.

Is it not possible for some reason?

Cheers,
Miguel

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 2/2] rust: uaccess: use NonEmpty in strcpy_into_buf()
  2026-10-12 11:53 [PATCH 0/2] rust: add NonEmpty Lorenzo Delgado
@ 2026-10-12 11:53 ` Lorenzo Delgado
  2026-10-12 12:22   ` Miguel Ojeda
  0 siblings, 1 reply; 5+ messages in thread
From: Lorenzo Delgado @ 2026-10-12 11:53 UTC (permalink / raw)
  To: Miguel Ojeda, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Danilo Krummrich, Daniel Almeida, Tamir Duberstein,
	Alexandre Courbot, Onur Özkan
  Cc: Lorenzo Delgado, linux-kernel, rust-for-linux, Thorsten Blum,
	Francis Laniel

`strcpy_into_buf()` returns `EINVAL` for an empty buffer and later
writes the NUL terminator through `last_mut().unwrap_unchecked()`,
relying on that earlier check.

Turn the buffer into a `NonEmpty<[u8]>` at the start instead. The
emptiness check and its `EINVAL` stay the same, but `last_mut()` now
returns the element directly, so the `unsafe` block goes away.

The `MaybeUninit` cast still starts from `[u8]`, so its SAFETY comment
does not depend on the layout of the wrapper.

Suggested-by: Gary Guo <gary@garyguo.net>
Link: https://lore.kernel.org/rust-for-linux/DM0D9TERQAM3.2Y3HV6P68E2N8@garyguo.net/
Link: https://github.com/Rust-for-Linux/linux/issues/1262
Signed-off-by: Lorenzo Delgado <lnsdev@proton.me>
---
 rust/kernel/uaccess.rs | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/rust/kernel/uaccess.rs b/rust/kernel/uaccess.rs
index 5f6c4d7a1a51..044c20e87ee6 100644
--- a/rust/kernel/uaccess.rs
+++ b/rust/kernel/uaccess.rs
@@ -11,6 +11,7 @@
     error::Result,
     ffi::{c_char, c_void},
     fs::file,
+    non_empty::NonEmpty,
     prelude::*,
     ptr::KnownSize,
     transmute::{AsBytes, FromBytes},
@@ -390,13 +391,11 @@ pub fn read_all<A: Allocator>(mut self, buf: &mut Vec<u8, A>, flags: Flags) -> R
     /// copied).
     #[doc(alias = "strncpy_from_user")]
     pub fn strcpy_into_buf<'buf>(self, buf: &'buf mut [u8]) -> Result<&'buf CStr> {
-        if buf.is_empty() {
-            return Err(EINVAL);
-        }
+        let buf = NonEmpty::try_from_mut(buf).ok_or(EINVAL)?;
 
         // SAFETY: The types are compatible and `strncpy_from_user` doesn't write uninitialized
         // bytes to `buf`.
-        let mut dst = unsafe { &mut *(core::ptr::from_mut(buf) as *mut [MaybeUninit<u8>]) };
+        let mut dst = unsafe { &mut *(core::ptr::from_mut::<[u8]>(buf) as *mut [MaybeUninit<u8>]) };
 
         // We never read more than `self.length` bytes.
         if dst.len() > self.length {
@@ -422,9 +421,7 @@ pub fn strcpy_into_buf<'buf>(self, buf: &'buf mut [u8]) -> Result<&'buf CStr> {
             // This means that we filled the buffer exactly. In this case, we add a NUL-terminator
             // and return it. Unlike the `len < dst.len()` branch, don't modify `len` because it
             // already represents the length including the NUL-terminator.
-            //
-            // SAFETY: Due to the check at the beginning, the buffer is not empty.
-            unsafe { *buf.last_mut().unwrap_unchecked() = 0 };
+            *buf.last_mut() = 0;
         }
 
         // This method consumes `self`, so it can only be called once, thus we do not need to

-- 
2.55.0



^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-10-12 12:22 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-12 11:46 [PATCH 0/2] rust: add NonEmpty Lorenzo Delgado
2026-10-12 11:46 ` [PATCH 1/2] rust: non_empty: " Lorenzo Delgado
2026-10-12 11:46 ` [PATCH 2/2] rust: uaccess: use NonEmpty in strcpy_into_buf() Lorenzo Delgado
2026-10-12 11:53 [PATCH 0/2] rust: add NonEmpty Lorenzo Delgado
2026-10-12 11:53 ` [PATCH 2/2] rust: uaccess: use NonEmpty in strcpy_into_buf() Lorenzo Delgado
2026-10-12 12:22   ` Miguel Ojeda

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

all inboxes | Powered by JetHome®