public inbox for rust-for-linux@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v8] rust: Return Option from page_align and ensure no usize overflow
@ 2025-12-23  5:56 Brendan Shephard
  2025-12-23 20:14 ` Danilo Krummrich
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Brendan Shephard @ 2025-12-23  5:56 UTC (permalink / raw)
  To: acourbot, aliceryhl, miguel.ojeda.sandonis, dakr, daniel.almeida
  Cc: rust-for-linux, Brendan Shephard

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>
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
---
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 words 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

Changes in v6:
- Simplified Doc text
- Link to v5: https://lore.kernel.org/rust-for-linux/aS1qz6WjjP8E49pY@google.com/T/#t
 rust/kernel/page.rs | 37 ++++++++++++++++++++++++++++++-------
 1 file changed, 30 insertions(+), 7 deletions(-)

Changes in v7:
- Addressed nits by Reviewer Alexandre Courbot
- Link to v6: https://lore.kernel.org/rust-for-linux/DEOIMG7Z0J8B.3OOY2A989T3G6@nvidia.com/#R

Changes in v8:
- Consistent periods at end of sentences
- Removed unnecessary tests and comments
- Link to v7: https://lore.kernel.org/rust-for-linux/20251204224006.353646-2-bshephar@bne-home.net/

 rust/kernel/page.rs | 33 ++++++++++++++++++++++++++-------
 1 file changed, 26 insertions(+), 7 deletions(-)

diff --git a/rust/kernel/page.rs b/rust/kernel/page.rs
index 432fc0297d4a..2ab58dcf34c7 100644
--- a/rust/kernel/page.rs
+++ b/rust/kernel/page.rs
@@ -25,14 +25,33 @@
 /// A bitmask that gives the page containing a given address.
 pub const PAGE_MASK: usize = !(PAGE_SIZE - 1);
 
-/// Round up the given number to the next multiple of [`PAGE_SIZE`].
+/// Rounds up 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 [`None`] on integer overflow.
+///
+/// # Examples
+///
+/// ```
+/// use kernel::page::{page_align, PAGE_SIZE};
+///
+/// // Requested address is already aligned.
+/// assert_eq!(page_align(0x0), Some(0x0));
+/// assert_eq!(page_align(PAGE_SIZE), Some(PAGE_SIZE));
+///
+/// // Requested address needs alignment up.
+/// assert_eq!(page_align(0x1), Some(PAGE_SIZE));
+/// assert_eq!(page_align(PAGE_SIZE + 1), Some(2 * PAGE_SIZE));
+///
+/// // Requested address causes overflow (returns None).
+/// let overflow_addr = usize::MAX - (PAGE_SIZE / 2);
+/// assert_eq!(page_align(overflow_addr), 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`].

base-commit: e6640487845061255af9614ec0a192e4fafa486e
-- 
2.51.1


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

* Re: [PATCH v8] rust: Return Option from page_align and ensure no usize overflow
  2025-12-23  5:56 [PATCH v8] rust: Return Option from page_align and ensure no usize overflow Brendan Shephard
@ 2025-12-23 20:14 ` Danilo Krummrich
  2025-12-29 14:11   ` Alice Ryhl
  2025-12-29 14:22 ` Miguel Ojeda
  2025-12-29 14:46 ` Danilo Krummrich
  2 siblings, 1 reply; 6+ messages in thread
From: Danilo Krummrich @ 2025-12-23 20:14 UTC (permalink / raw)
  To: aliceryhl
  Cc: acourbot, Brendan Shephard, miguel.ojeda.sandonis, daniel.almeida,
	rust-for-linux

On Tue Dec 23, 2025 at 6:56 AM CET, 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>
> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
> Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>
> Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>

Alice, unless you have any concerns I'd take this through the drm-rust tree, so
I can pick up the corresponding nova-drm patch as well.

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

* Re: [PATCH v8] rust: Return Option from page_align and ensure no usize overflow
  2025-12-23 20:14 ` Danilo Krummrich
@ 2025-12-29 14:11   ` Alice Ryhl
  0 siblings, 0 replies; 6+ messages in thread
From: Alice Ryhl @ 2025-12-29 14:11 UTC (permalink / raw)
  To: Danilo Krummrich
  Cc: acourbot, Brendan Shephard, miguel.ojeda.sandonis, daniel.almeida,
	rust-for-linux

On Tue, Dec 23, 2025 at 9:14 PM Danilo Krummrich <dakr@kernel.org> wrote:
>
> On Tue Dec 23, 2025 at 6:56 AM CET, 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>
> > Reviewed-by: Alice Ryhl <aliceryhl@google.com>
> > Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>
> > Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
>
> Alice, unless you have any concerns I'd take this through the drm-rust tree, so
> I can pick up the corresponding nova-drm patch as well.

Sounds good to me.

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

* Re: [PATCH v8] rust: Return Option from page_align and ensure no usize overflow
  2025-12-23  5:56 [PATCH v8] rust: Return Option from page_align and ensure no usize overflow Brendan Shephard
  2025-12-23 20:14 ` Danilo Krummrich
@ 2025-12-29 14:22 ` Miguel Ojeda
  2025-12-29 14:46 ` Danilo Krummrich
  2 siblings, 0 replies; 6+ messages in thread
From: Miguel Ojeda @ 2025-12-29 14:22 UTC (permalink / raw)
  To: Brendan Shephard
  Cc: acourbot, aliceryhl, dakr, daniel.almeida, rust-for-linux

On Tue, Dec 23, 2025 at 6:57 AM Brendan Shephard <bshephar@bne-home.net> 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>
> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
> Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>
> Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>

Looks good, thanks for having added the examples and going through all
the feedback -- welcome!

> +/// // Requested address causes overflow (returns None).

`None`

> +/// let overflow_addr = usize::MAX - (PAGE_SIZE / 2);
> +/// assert_eq!(page_align(overflow_addr), None);

I still think this could eventually just test the top address more
easily (and then the extra line we gain could be used to test a one
plus the previous page), but it is fine as it is, let's merge it as-is
:)

Thanks!

Cheers,
Miguel

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

* Re: [PATCH v8] rust: Return Option from page_align and ensure no usize overflow
  2025-12-23  5:56 [PATCH v8] rust: Return Option from page_align and ensure no usize overflow Brendan Shephard
  2025-12-23 20:14 ` Danilo Krummrich
  2025-12-29 14:22 ` Miguel Ojeda
@ 2025-12-29 14:46 ` Danilo Krummrich
  2025-12-30  8:14   ` Brendan Shephard
  2 siblings, 1 reply; 6+ messages in thread
From: Danilo Krummrich @ 2025-12-29 14:46 UTC (permalink / raw)
  To: Brendan Shephard
  Cc: acourbot, aliceryhl, miguel.ojeda.sandonis, daniel.almeida,
	rust-for-linux

On Tue Dec 23, 2025 at 6:56 AM CET, 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>
> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
> Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>
> Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>

Applied to drm-rust-next, thanks!

    [ Use kernel vertical style for imports; use markdown in comments.
      - Danilo ]

Brendan, can you please send a follow-up patch addressing [1] and [2]?

[1] https://lore.kernel.org/all/CANiq72kcMLXz=xyZeKC0=j_e0BzJEY3wGpBTTxfJsc6EZhCnXA@mail.gmail.com/
[2] https://lore.kernel.org/rust-for-linux/CANiq72mLPvB_6Ow3bW5-V4-km=RyA59chQ1g1x9qUt2P-zZweg@mail.gmail.com/

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

* Re: [PATCH v8] rust: Return Option from page_align and ensure no usize overflow
  2025-12-29 14:46 ` Danilo Krummrich
@ 2025-12-30  8:14   ` Brendan Shephard
  0 siblings, 0 replies; 6+ messages in thread
From: Brendan Shephard @ 2025-12-30  8:14 UTC (permalink / raw)
  To: Danilo Krummrich
  Cc: acourbot, aliceryhl, miguel.ojeda.sandonis, daniel.almeida,
	rust-for-linux

On Mon, Dec 29, 2025 at 03:46:09PM +0100, Danilo Krummrich wrote:
> On Tue Dec 23, 2025 at 6:56 AM CET, 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>
> > Reviewed-by: Alice Ryhl <aliceryhl@google.com>
> > Reviewed-by: Alexandre Courbot <acourbot@nvidia.com>
> > Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
> 
> Applied to drm-rust-next, thanks!
> 
>     [ Use kernel vertical style for imports; use markdown in comments.
>       - Danilo ]
> 
> Brendan, can you please send a follow-up patch addressing [1] and [2]?
> 
> [1] https://lore.kernel.org/all/CANiq72kcMLXz=xyZeKC0=j_e0BzJEY3wGpBTTxfJsc6EZhCnXA@mail.gmail.com/
> [2] https://lore.kernel.org/rust-for-linux/CANiq72mLPvB_6Ow3bW5-V4-km=RyA59chQ1g1x9qUt2P-zZweg@mail.gmail.com/
> 

Hey Danilo,

Thanks! I can indeed send a follow-up to address those.

Cheers,
Brendan

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

end of thread, other threads:[~2025-12-30  8:15 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-23  5:56 [PATCH v8] rust: Return Option from page_align and ensure no usize overflow Brendan Shephard
2025-12-23 20:14 ` Danilo Krummrich
2025-12-29 14:11   ` Alice Ryhl
2025-12-29 14:22 ` Miguel Ojeda
2025-12-29 14:46 ` Danilo Krummrich
2025-12-30  8:14   ` Brendan Shephard

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