Rust for Linux List
 help / color / mirror / Atom feed
* [PATCH v5] rust: Return Option from page_align and ensure no usize overflow
@ 2025-12-01  5:45 Brendan Shephard
  2025-12-01 10:15 ` Alice Ryhl
  0 siblings, 1 reply; 3+ messages in thread
From: Brendan Shephard @ 2025-12-01  5:45 UTC (permalink / raw)
  To: aliceryhl, miguel.ojeda.sandonis, dakr, acourbot, daniel.almeida
  Cc: rust-for-linux

Change `page_align()` to return `Option<usize>` to allow validation
of the provided `addr` value. This ensures that any value that is
within one `PAGE_SIZE` of `usize::MAX` will not panic, and instead
returns `None` to indicate overflow.

Signed-off-by: Brendan Shephard <bshephar@bne-home.net>
---
Changes in v2:
- Reworded commit message to follow the imperative form.
- Expanded the documentation to explain the `Some` and `None` return cases.
- Added a period at the end of the documentation comment.
- Link to v1 (and v2): https://lore.kernel.org/rust-for-linux/aSheTh-T1oroAUHR@fedora/T/#t

Changes in v3:
- Fix documentation layout for better rustdoc rendering
- Add doc examples and doctest
- Ensure function is always inlined for performance optimisation
- Restructure function so that early return is the None case and the
  default is the happy path.

Changes in v4:
- Fix rustdoc missing comment (//) prefix
- Rebase on master
- Link to v3: https://lore.kernel.org/rust-for-linux/aSoY31U3uDI2y7V1@fedora/T/#u

Changes in v5:
- Use kernel `PAGE_SIZE` for all doctest examples
- Backtick the backtickable works in example comment
- Add new example for `usize::MAX` input value
- Newline before # Examples
- Link to v4: https://lore.kernel.org/rust-for-linux/aSzDj1htLp11eCWF@fedora/T/#t

rust/kernel/page.rs | 36 ++++++++++++++++++++++++++++++------
 1 file changed, 30 insertions(+), 6 deletions(-)

diff --git a/rust/kernel/page.rs b/rust/kernel/page.rs
index 432fc0297d4a..4a0cfa32a5d6 100644
--- a/rust/kernel/page.rs
+++ b/rust/kernel/page.rs
@@ -27,12 +27,36 @@

 /// Round up the given number to the next multiple of [`PAGE_SIZE`].
 ///
-/// It is incorrect to pass an address where the next multiple of [`PAGE_SIZE`] doesn't fit in a
-/// [`usize`].
-pub const fn page_align(addr: usize) -> usize {
-    // Parentheses around `PAGE_SIZE - 1` to avoid triggering overflow sanitizers in the wrong
-    // cases.
-    (addr + (PAGE_SIZE - 1)) & PAGE_MASK
+/// Returns a page aligned [`usize`] in cases where the value can be aligned. Otherwise, returns [`None`]
+/// if the aligned size will overflow a [`usize`].
+///
+/// # Examples
+///
+/// ```rust
+/// use kernel::page::{page_align, PAGE_SIZE};
+/// // Case 1: Already aligned
+/// assert_eq!(page_align(0x0), Some(0x0));
+/// assert_eq!(page_align(PAGE_SIZE), Some(PAGE_SIZE));
+///
+/// // Case 2: Needs alignment up
+/// assert_eq!(page_align(0x1), Some(PAGE_SIZE));
+/// assert_eq!(page_align(PAGE_SIZE + 1), Some(2 * PAGE_SIZE));
+///
+/// // Case 3: Requested address causes overflow (returns None)
+/// // The check asserts that `None` is returned when a value is requested within one `PAGE_SIZE` of
+/// // `usize::MAX`.
+/// let overflow_addr = usize::MAX - (PAGE_SIZE / 2);
+/// assert_eq!(page_align(overflow_addr), None);
+///
+/// // Case 4: Requested address size of `usize::MAX` would overflow, and subsequently returns `None`.
+/// assert_eq!(page_align(usize::MAX), None);
+/// ```
+#[inline(always)]
+pub const fn page_align(addr: usize) -> Option<usize> {
+    let Some(sum) = addr.checked_add(PAGE_SIZE - 1) else {
+        return None;
+    };
+    Some(sum & PAGE_MASK)
 }

 /// Representation of a non-owning reference to a [`Page`].
--
2.51.1

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

* Re: [PATCH v5] rust: Return Option from page_align and ensure no usize overflow
  2025-12-01  5:45 [PATCH v5] rust: Return Option from page_align and ensure no usize overflow Brendan Shephard
@ 2025-12-01 10:15 ` Alice Ryhl
  2025-12-02  9:37   ` Brendan Shephard
  0 siblings, 1 reply; 3+ messages in thread
From: Alice Ryhl @ 2025-12-01 10:15 UTC (permalink / raw)
  To: Brendan Shephard
  Cc: miguel.ojeda.sandonis, dakr, acourbot, daniel.almeida,
	rust-for-linux

On Mon, Dec 01, 2025 at 03:45:52PM +1000, Brendan Shephard wrote:
> Change `page_align()` to return `Option<usize>` to allow validation
> of the provided `addr` value. This ensures that any value that is
> within one `PAGE_SIZE` of `usize::MAX` will not panic, and instead
> returns `None` to indicate overflow.
> 
> Signed-off-by: Brendan Shephard <bshephar@bne-home.net>
> ---
> Changes in v2:
> - Reworded commit message to follow the imperative form.
> - Expanded the documentation to explain the `Some` and `None` return cases.
> - Added a period at the end of the documentation comment.
> - Link to v1 (and v2): https://lore.kernel.org/rust-for-linux/aSheTh-T1oroAUHR@fedora/T/#t
> 
> Changes in v3:
> - Fix documentation layout for better rustdoc rendering
> - Add doc examples and doctest
> - Ensure function is always inlined for performance optimisation
> - Restructure function so that early return is the None case and the
>   default is the happy path.
> 
> Changes in v4:
> - Fix rustdoc missing comment (//) prefix
> - Rebase on master
> - Link to v3: https://lore.kernel.org/rust-for-linux/aSoY31U3uDI2y7V1@fedora/T/#u
> 
> Changes in v5:
> - Use kernel `PAGE_SIZE` for all doctest examples
> - Backtick the backtickable works in example comment
> - Add new example for `usize::MAX` input value
> - Newline before # Examples
> - Link to v4: https://lore.kernel.org/rust-for-linux/aSzDj1htLp11eCWF@fedora/T/#t
> 
> rust/kernel/page.rs | 36 ++++++++++++++++++++++++++++++------
>  1 file changed, 30 insertions(+), 6 deletions(-)
> 
> diff --git a/rust/kernel/page.rs b/rust/kernel/page.rs
> index 432fc0297d4a..4a0cfa32a5d6 100644
> --- a/rust/kernel/page.rs
> +++ b/rust/kernel/page.rs
> @@ -27,12 +27,36 @@
> 
>  /// Round up the given number to the next multiple of [`PAGE_SIZE`].
>  ///
> -/// It is incorrect to pass an address where the next multiple of [`PAGE_SIZE`] doesn't fit in a
> -/// [`usize`].
> -pub const fn page_align(addr: usize) -> usize {
> -    // Parentheses around `PAGE_SIZE - 1` to avoid triggering overflow sanitizers in the wrong
> -    // cases.
> -    (addr + (PAGE_SIZE - 1)) & PAGE_MASK
> +/// Returns a page aligned [`usize`] in cases where the value can be aligned. Otherwise, returns [`None`]
> +/// if the aligned size will overflow a [`usize`].

The first line of doc-comments is shown on the module docs. I think it'd
be nice to shorten the first line.

/// Rounds up to the next multiple of [`PAGE_SIZE`].
///
/// Returns `None` on integer overflow.

People reading the module docs will get what they need from "Rounds up
to the next multiple of PAGE_SIZE.", and if they want to know details
such as overflow behavior they can read the page specific to this
method.

Alice

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

* Re: [PATCH v5] rust: Return Option from page_align and ensure no usize overflow
  2025-12-01 10:15 ` Alice Ryhl
@ 2025-12-02  9:37   ` Brendan Shephard
  0 siblings, 0 replies; 3+ messages in thread
From: Brendan Shephard @ 2025-12-02  9:37 UTC (permalink / raw)
  To: Alice Ryhl
  Cc: miguel.ojeda.sandonis, dakr, acourbot, daniel.almeida,
	rust-for-linux

On Mon, Dec 01, 2025 at 10:15:43AM +0000, Alice Ryhl wrote:
> >  /// Round up the given number to the next multiple of [`PAGE_SIZE`].
> >  ///
> > -/// It is incorrect to pass an address where the next multiple of [`PAGE_SIZE`] doesn't fit in a
> > -/// [`usize`].
> > -pub const fn page_align(addr: usize) -> usize {
> > -    // Parentheses around `PAGE_SIZE - 1` to avoid triggering overflow sanitizers in the wrong
> > -    // cases.
> > -    (addr + (PAGE_SIZE - 1)) & PAGE_MASK
> > +/// Returns a page aligned [`usize`] in cases where the value can be aligned. Otherwise, returns [`None`]
> > +/// if the aligned size will overflow a [`usize`].
> 
> The first line of doc-comments is shown on the module docs. I think it'd
> be nice to shorten the first line.
> 
> /// Rounds up to the next multiple of [`PAGE_SIZE`].
> ///
> /// Returns `None` on integer overflow.
> 
> People reading the module docs will get what they need from "Rounds up
> to the next multiple of PAGE_SIZE.", and if they want to know details
> such as overflow behavior they can read the page specific to this
> method.
> 
> Alice
> 
> 

Seems reasonable, v6 sent with this adjustment:
https://lore.kernel.org/rust-for-linux/aS6y5blasMX0KcD7@fedora/T/#u

Thanks!

Brendan

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

end of thread, other threads:[~2025-12-02  9:37 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-01  5:45 [PATCH v5] rust: Return Option from page_align and ensure no usize overflow Brendan Shephard
2025-12-01 10:15 ` Alice Ryhl
2025-12-02  9:37   ` Brendan Shephard

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