Re: [PATCH] rust: Return Option from page_align and ensure no usize overflow

[Brendan Shephard <bshephar@bne-home.net>]

On Thu, Nov 27, 2025 at 10:44:16AM -0300, Daniel Almeida wrote:
> 
> 
> > On 27 Nov 2025, at 10:07, Brendan Shephard <bshephar@bne-home.net> wrote:
> > 
> > Changed page_align() to return Option<usize> which allows for validation
> 
> nit: imperative voice here
> 
Yeah, my bad. I'll fix that in v2.

> > of the provided addr value. This ensures that any value that is provided
> > within one PAGE_SIZE of usize::MAX will not panic and instead page_align
> > will return None.
> > 
> > Callers of page_align() should raise a EINVAL when they receive None
> > from page_align().
> > 
> > Signed-off-by: Brendan Shephard <bshephar@bne-home.net>
> > ---
> > rust/kernel/page.rs | 13 ++++++-------
> > 1 file changed, 6 insertions(+), 7 deletions(-)
> > 
> > diff --git a/rust/kernel/page.rs b/rust/kernel/page.rs
> > index 432fc0297d4a..b78473b67003 100644
> > --- a/rust/kernel/page.rs
> > +++ b/rust/kernel/page.rs
> > @@ -26,13 +26,12 @@
> > pub const PAGE_MASK: usize = !(PAGE_SIZE - 1);
> > 
> > /// 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`].
> 
> Can you document the return value? I’d find it weird that this returns Option otherwise.
>
Sounds like a good plan. I'll document both return Options in v2. None
if the resulting value would overflow a usize::MAX, or Some usize that
is page aligned.

> > -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
> > +/// Return None in cases where the next multiple of [`PAGE_SIZE`] would overflow a [`usize`]
> > +pub const fn page_align(addr: usize) -> Option<usize> {
> > +    if let Some(sum) = addr.checked_add(PAGE_SIZE - 1) {
> > +        return Some(sum & PAGE_MASK);
> > +    }
> > +    None
> > }
> > 
> > /// Representation of a non-owning reference to a [`Page`].
> > --
> > 2.51.1
> > 
> 
> With the change above:
> 
> Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
>