rust-for-linux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3] rust: num: bounded: mark __new as unsafe
@ 2025-12-04  3:38 Hsiu Che Yu
  2025-12-04 14:13 ` Alexandre Courbot
  0 siblings, 1 reply; 4+ messages in thread
From: Hsiu Che Yu @ 2025-12-04  3:38 UTC (permalink / raw)
  To: Alexandre Courbot
  Cc: Hsiu Che Yu, Miguel Ojeda, Yury Norov, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Danilo Krummrich, rust-for-linux, linux-kernel

The `Bounded::__new()` constructor relies on the caller to ensure the
value can be represented within N bits. Failing to uphold this
requirement breaks the type invariant. Mark it as unsafe and document
this requirement in a Safety section to make the contract explicit.

Update all call sites to use unsafe blocks and change their comments
from `INVARIANT:` to `SAFETY:`, as they are now justifying unsafe
operations rather than establishing type invariants.

Fixes: 01e345e82ec3a ("rust: num: add Bounded integer wrapping type")
Link: https://lore.kernel.org/all/aS1qC_ol2XEpZ44b@google.com/
Reported-by: Miguel Ojeda <ojeda@kernel.org>
Closes: https://github.com/Rust-for-Linux/linux/issues/1211
Signed-off-by: Hsiu Che Yu <yu.whisper.personal@gmail.com>
---
Changes in v3:
- Add `Link:` tag (suggested by Alexandre Courbot)
- Remove empty line before Signed-off-by

Changes in v2:
- Mark `Bounded::__new` as unsafe and add Safety documentation
- Update all call sites with unsafe blocks and SAFETY comments

Link to v2: https://lore.kernel.org/rust-for-linux/20251202032541.78497-1-yu.whisper.personal@gmail.com/
Link to v1: https://lore.kernel.org/rust-for-linux/20251201062516.45495-1-yu.whisper.personal@gmail.com/
---
 rust/kernel/num/bounded.rs | 34 +++++++++++++++++++---------------
 1 file changed, 19 insertions(+), 15 deletions(-)

diff --git a/rust/kernel/num/bounded.rs b/rust/kernel/num/bounded.rs
index f870080af8ac..5838c84f8a53 100644
--- a/rust/kernel/num/bounded.rs
+++ b/rust/kernel/num/bounded.rs
@@ -259,9 +259,9 @@ pub const fn new<const VALUE: $type>() -> Self {
                     assert!(fits_within!(VALUE, $type, N));
                 }
 
-                // INVARIANT: `fits_within` confirmed that `VALUE` can be represented within
+                // SAFETY: `fits_within` confirmed that `VALUE` can be represented within
                 // `N` bits.
-                Self::__new(VALUE)
+                unsafe { Self::__new(VALUE) }
             }
         }
         )*
@@ -284,7 +284,11 @@ impl<T, const N: u32> Bounded<T, N>
     ///
     /// The caller remains responsible for checking, either statically or dynamically, that `value`
     /// can be represented as a `T` using at most `N` bits.
-    const fn __new(value: T) -> Self {
+    ///
+    /// # Safety
+    ///
+    /// The caller must ensure that `value` can be represented within `N` bits.
+    const unsafe fn __new(value: T) -> Self {
         // Enforce the type invariants.
         const {
             // `N` cannot be zero.
@@ -328,8 +332,8 @@ const fn __new(value: T) -> Self {
     /// ```
     pub fn try_new(value: T) -> Option<Self> {
         fits_within(value, N).then(|| {
-            // INVARIANT: `fits_within` confirmed that `value` can be represented within `N` bits.
-            Self::__new(value)
+            // SAFETY: `fits_within` confirmed that `value` can be represented within `N` bits.
+            unsafe { Self::__new(value) }
         })
     }
 
@@ -370,8 +374,8 @@ pub fn from_expr(expr: T) -> Self {
             "Requested value larger than maximal representable value."
         );
 
-        // INVARIANT: `fits_within` confirmed that `expr` can be represented within `N` bits.
-        Self::__new(expr)
+        // SAFETY: `fits_within` confirmed that `expr` can be represented within `N` bits.
+        unsafe { Self::__new(expr) }
     }
 
     /// Returns the wrapped value as the backing type.
@@ -410,9 +414,9 @@ pub const fn extend<const M: u32>(self) -> Bounded<T, M> {
             );
         }
 
-        // INVARIANT: The value did fit within `N` bits, so it will all the more fit within
+        // SAFETY: The value did fit within `N` bits, so it will all the more fit within
         // the larger `M` bits.
-        Bounded::__new(self.0)
+        unsafe { Bounded::__new(self.0) }
     }
 
     /// Attempts to shrink the number of bits usable for `self`.
@@ -466,9 +470,9 @@ pub fn cast<U>(self) -> Bounded<U, N>
         // `U` and `T` have the same sign, hence this conversion cannot fail.
         let value = unsafe { U::try_from(self.get()).unwrap_unchecked() };
 
-        // INVARIANT: Although the backing type has changed, the value is still represented within
+        // SAFETY: Although the backing type has changed, the value is still represented within
         // `N` bits, and with the same signedness.
-        Bounded::__new(value)
+        unsafe { Bounded::__new(value) }
     }
 }
 
@@ -944,9 +948,9 @@ impl<T, const N: u32> From<$type> for Bounded<T, N>
             Self: AtLeastXBits<{ <$type as Integer>::BITS as usize }>,
         {
             fn from(value: $type) -> Self {
-                // INVARIANT: The trait bound on `Self` guarantees that `N` bits is
+                // SAFETY: The trait bound on `Self` guarantees that `N` bits is
                 // enough to hold any value of the source type.
-                Self::__new(T::from(value))
+                unsafe { Self::__new(T::from(value)) }
             }
         }
         )*
@@ -1051,8 +1055,8 @@ impl<T, const N: u32> From<bool> for Bounded<T, N>
     T: Integer + From<bool>,
 {
     fn from(value: bool) -> Self {
-        // INVARIANT: A boolean can be represented using a single bit, and thus fits within any
+        // SAFETY: A boolean can be represented using a single bit, and thus fits within any
         // integer type for any `N` > 0.
-        Self::__new(T::from(value))
+        unsafe { Self::__new(T::from(value)) }
     }
 }
-- 
2.43.0


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

* Re: [PATCH v3] rust: num: bounded: mark __new as unsafe
  2025-12-04  3:38 [PATCH v3] rust: num: bounded: mark __new as unsafe Hsiu Che Yu
@ 2025-12-04 14:13 ` Alexandre Courbot
  2025-12-04 14:28   ` Miguel Ojeda
  0 siblings, 1 reply; 4+ messages in thread
From: Alexandre Courbot @ 2025-12-04 14:13 UTC (permalink / raw)
  To: Hsiu Che Yu, Alexandre Courbot, Miguel Ojeda
  Cc: Yury Norov, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
	Danilo Krummrich, rust-for-linux, linux-kernel

On Thu Dec 4, 2025 at 12:38 PM JST, Hsiu Che Yu wrote:
> The `Bounded::__new()` constructor relies on the caller to ensure the
> value can be represented within N bits. Failing to uphold this
> requirement breaks the type invariant. Mark it as unsafe and document
> this requirement in a Safety section to make the contract explicit.
>
> Update all call sites to use unsafe blocks and change their comments
> from `INVARIANT:` to `SAFETY:`, as they are now justifying unsafe
> operations rather than establishing type invariants.
>
> Fixes: 01e345e82ec3a ("rust: num: add Bounded integer wrapping type")
> Link: https://lore.kernel.org/all/aS1qC_ol2XEpZ44b@google.com/
> Reported-by: Miguel Ojeda <ojeda@kernel.org>
> Closes: https://github.com/Rust-for-Linux/linux/issues/1211
> Signed-off-by: Hsiu Che Yu <yu.whisper.personal@gmail.com>

I believe I gave my acked-by on v2, but just to make sure it isn't
missed:

Acked-by: Alexandre Courbot <acourbot@nvidia.com>

Miguel, do you prefer to take this directly, or should I gather all the
`num/*` patches and send you a PR in one go sometime later?

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

* Re: [PATCH v3] rust: num: bounded: mark __new as unsafe
  2025-12-04 14:13 ` Alexandre Courbot
@ 2025-12-04 14:28   ` Miguel Ojeda
  2025-12-04 14:45     ` Alexandre Courbot
  0 siblings, 1 reply; 4+ messages in thread
From: Miguel Ojeda @ 2025-12-04 14:28 UTC (permalink / raw)
  To: Alexandre Courbot
  Cc: Hsiu Che Yu, Miguel Ojeda, Yury Norov, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Danilo Krummrich, rust-for-linux, linux-kernel

On Thu, Dec 4, 2025 at 3:13 PM Alexandre Courbot <acourbot@nvidia.com> wrote:
>
> Miguel, do you prefer to take this directly, or should I gather all the
> `num/*` patches and send you a PR in one go sometime later?

Just to avoid forgetting later, which ones do you mean? i.e. I have
these in my radar:

    [PATCH v3] rust: num: bounded: mark __new as unsafe
    [PATCH] rust: num: fix typos in Bounded documentation
    [PATCH] rust: num: bounded: rename `try_into_bitint` to `try_into_bounded`
    One in [PATCH v2 0/7] rust: build_assert: document and fix use
with function arguments

None seems urgent, i.e. they can be added to `rust-fixes` after -rc1 I
think, right?

Thanks!

Cheers,
Miguel

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

* Re: [PATCH v3] rust: num: bounded: mark __new as unsafe
  2025-12-04 14:28   ` Miguel Ojeda
@ 2025-12-04 14:45     ` Alexandre Courbot
  0 siblings, 0 replies; 4+ messages in thread
From: Alexandre Courbot @ 2025-12-04 14:45 UTC (permalink / raw)
  To: Miguel Ojeda, Alexandre Courbot
  Cc: Hsiu Che Yu, Miguel Ojeda, Yury Norov, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Andreas Hindborg, Alice Ryhl,
	Trevor Gross, Danilo Krummrich, rust-for-linux, linux-kernel

On Thu Dec 4, 2025 at 11:28 PM JST, Miguel Ojeda wrote:
> On Thu, Dec 4, 2025 at 3:13 PM Alexandre Courbot <acourbot@nvidia.com> wrote:
>>
>> Miguel, do you prefer to take this directly, or should I gather all the
>> `num/*` patches and send you a PR in one go sometime later?
>
> Just to avoid forgetting later, which ones do you mean? i.e. I have
> these in my radar:
>
>     [PATCH v3] rust: num: bounded: mark __new as unsafe
>     [PATCH] rust: num: fix typos in Bounded documentation
>     [PATCH] rust: num: bounded: rename `try_into_bitint` to `try_into_bounded`
>     One in [PATCH v2 0/7] rust: build_assert: document and fix use
> with function arguments
>
> None seems urgent, i.e. they can be added to `rust-fixes` after -rc1 I
> think, right?

Oh yes, actually I was thinking about rust-next rather than rust-fixes,
but that works as well. My point was more about taking the mental burden
of tracking this off your shoulders. :)

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

end of thread, other threads:[~2025-12-04 14:46 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-04  3:38 [PATCH v3] rust: num: bounded: mark __new as unsafe Hsiu Che Yu
2025-12-04 14:13 ` Alexandre Courbot
2025-12-04 14:28   ` Miguel Ojeda
2025-12-04 14:45     ` Alexandre Courbot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).