* [PATCH v7] rust: Return Option from page_align and ensure no usize overflow
@ 2025-12-04 22:40 Brendan Shephard
2025-12-18 13:13 ` Alexandre Courbot
0 siblings, 1 reply; 3+ messages in thread
From: Brendan Shephard @ 2025-12-04 22:40 UTC (permalink / raw)
To: aliceryhl, miguel.ojeda.sandonis, dakr, acourbot, 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
rust/kernel/page.rs | 37 ++++++++++++++++++++++++++++++-------
1 file changed, 30 insertions(+), 7 deletions(-)
diff --git a/rust/kernel/page.rs b/rust/kernel/page.rs
index 432fc0297d4a..3c94654b190a 100644
--- a/rust/kernel/page.rs
+++ b/rust/kernel/page.rs
@@ -25,14 +25,37 @@
/// 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)
+/// // 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);
+///
+/// // Requested address 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`].
base-commit: e6640487845061255af9614ec0a192e4fafa486e
--
2.52.0
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH v7] rust: Return Option from page_align and ensure no usize overflow
2025-12-04 22:40 [PATCH v7] rust: Return Option from page_align and ensure no usize overflow Brendan Shephard
@ 2025-12-18 13:13 ` Alexandre Courbot
2025-12-23 6:03 ` Brendan Shephard
0 siblings, 1 reply; 3+ messages in thread
From: Alexandre Courbot @ 2025-12-18 13:13 UTC (permalink / raw)
To: Brendan Shephard, aliceryhl, miguel.ojeda.sandonis, dakr,
acourbot, daniel.almeida
Cc: rust-for-linux
On Fri Dec 5, 2025 at 7:40 AM JST, 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>
> ---
> 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
>
> rust/kernel/page.rs | 37 ++++++++++++++++++++++++++++++-------
> 1 file changed, 30 insertions(+), 7 deletions(-)
>
> diff --git a/rust/kernel/page.rs b/rust/kernel/page.rs
> index 432fc0297d4a..3c94654b190a 100644
> --- a/rust/kernel/page.rs
> +++ b/rust/kernel/page.rs
> @@ -25,14 +25,37 @@
> /// 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};
Add an extra line to separate the imports from the code.
> +/// // Requested address is already aligned
Please consistently finish sentences with a `.`.
> +/// 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)
> +/// // The check asserts that `None` is returned when a value is requested within one `PAGE_SIZE` of
> +/// // `usize::MAX`.
We can see what the check does by looking at it, so this last sentence
feels heavy and unnecessary.
> +/// let overflow_addr = usize::MAX - (PAGE_SIZE / 2);
> +/// assert_eq!(page_align(overflow_addr), None);
> +///
> +/// // Requested address of `usize::MAX` would overflow, and subsequently returns `None`.
> +/// assert_eq!(page_align(usize::MAX), None);
This test is basically equivalent to the previous one and can be
discarded imho.
I have checked the tests and rustdoc fwiw, and saw no issue, so this
looks good to go after these last details are fixed.
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH v7] rust: Return Option from page_align and ensure no usize overflow
2025-12-18 13:13 ` Alexandre Courbot
@ 2025-12-23 6:03 ` Brendan Shephard
0 siblings, 0 replies; 3+ messages in thread
From: Brendan Shephard @ 2025-12-23 6:03 UTC (permalink / raw)
To: Alexandre Courbot
Cc: aliceryhl, miguel.ojeda.sandonis, dakr, daniel.almeida,
rust-for-linux
On Thu, Dec 18, 2025 at 10:13:23PM +0900, Alexandre Courbot wrote:
> On Fri Dec 5, 2025 at 7:40 AM JST, Brendan Shephard wrote:
> > +/// Returns [`None`] on integer overflow.
> > +///
> > +/// # Examples
> > +///
> > +/// ```
> > +/// use kernel::page::{page_align, PAGE_SIZE};
>
> Add an extra line to separate the imports from the code.
>
> > +/// // Requested address is already aligned
>
> Please consistently finish sentences with a `.`.
>
> > +/// 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)
> > +/// // The check asserts that `None` is returned when a value is requested within one `PAGE_SIZE` of
> > +/// // `usize::MAX`.
>
> We can see what the check does by looking at it, so this last sentence
> feels heavy and unnecessary.
>
> > +/// let overflow_addr = usize::MAX - (PAGE_SIZE / 2);
> > +/// assert_eq!(page_align(overflow_addr), None);
> > +///
> > +/// // Requested address of `usize::MAX` would overflow, and subsequently returns `None`.
> > +/// assert_eq!(page_align(usize::MAX), None);
>
> This test is basically equivalent to the previous one and can be
> discarded imho.
>
> I have checked the tests and rustdoc fwiw, and saw no issue, so this
> looks good to go after these last details are fixed.
>
>
Yeah, sounds good. I fixed up those and sent a new revision:
https://lore.kernel.org/rust-for-linux/20251223055647.9761-1-bshephar@bne-home.net/T/#u
Thanks again! Hope you enjoy your holidays.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-12-23 6:03 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-04 22:40 [PATCH v7] rust: Return Option from page_align and ensure no usize overflow Brendan Shephard
2025-12-18 13:13 ` Alexandre Courbot
2025-12-23 6:03 ` Brendan Shephard
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox