Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
to consider:
- [High] Insufficient `# Safety` requirements for `Page::from_raw` leading to 
unsoundness.
--

commit e6799a35c42d6949e4edb57a0982fc37d7af34f7
Author: Andreas Hindborg <[email protected]>

rust: page: add `from_raw()`

Add a method to `Page` that allows construction of an instance from `struct
page` pointer.

> diff --git a/rust/kernel/page.rs b/rust/kernel/page.rs
> index a2610774c7499..cd394b0656c0c 100644
> --- a/rust/kernel/page.rs
> +++ b/rust/kernel/page.rs
> @@ -146,6 +146,20 @@ pub fn nid(&self) -> i32 {
>          unsafe { bindings::page_to_nid(self.as_ptr()) }
>      }
>  
> +    /// Create a `&Page` from a raw `struct page` pointer.
> +    ///
> +    /// # Safety
> +    ///
> +    /// `ptr` must be convertible to a shared reference with a lifetime of 
> `'a`.
> +    #[inline]
> +    pub unsafe fn from_raw<'a>(ptr: *const bindings::page) -> &'a Self {
> +        // INVARIANT: By the function safety requirements, `ptr` refers to a 
> valid `struct page`, so
> +        // the returned reference upholds the type invariant of `Page`.

[Severity: High]
Are the safety requirements for Page::from_raw() sufficient?

The documented requirement allows passing a pointer to a valid struct page
that has a zero reference count or is unmanaged (e.g. stack allocated or
reserved).

If safe Rust code later casts this &Page to an ARef<Page> (via .into()
or .clone()), this invokes inc_ref() (get_page()) on an unmanaged or
0-refcount page.

Couldn't this cause memory corruption or trigger a VM_BUG_ON_PAGE? Should
the safety documentation and type invariant require that the page is managed
by the page reference count (i.e. has a non-zero refcount)?

> +        // SAFETY: By function safety requirements, `ptr` is not null and is 
> convertible to a shared
> +        // reference.
> +        unsafe { &*ptr.cast() }
> +    }

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=8

Reply via email to