Commit 12717ebe authored by Andreas Hindborg's avatar Andreas Hindborg Committed by Miguel Ojeda
Browse files

rust: types: add FOREIGN_ALIGN to ForeignOwnable 



The current implementation of `ForeignOwnable` is leaking the type of the
opaque pointer to consumers of the API. This allows consumers of the opaque
pointer to rely on the information that can be extracted from the pointer
type.

To prevent this, change the API to the version suggested by Maira
Canal (link below): Remove `ForeignOwnable::PointedTo` in favor of a
constant, which specifies the alignment of the pointers returned by
`into_foreign`.

With this change, `ArcInner` no longer needs `pub` visibility, so change it
to private.

Suggested-by: default avatarAlice Ryhl <aliceryhl@google.com>
Suggested-by: default avatarMaíra Canal <mcanal@igalia.com>
Link: https://lore.kernel.org/r/20240309235927.168915-3-mcanal@igalia.com


Acked-by: default avatarDanilo Krummrich <dakr@kernel.org>
Reviewed-by: default avatarBenno Lossin <lossin@kernel.org>
Signed-off-by: default avatarAndreas Hindborg <a.hindborg@kernel.org>
Reviewed-by: default avatarAlice Ryhl <aliceryhl@google.com>
Link: https://lore.kernel.org/r/20250612-pointed-to-v3-1-b009006d86a1@kernel.org


Signed-off-by: default avatarMiguel Ojeda <ojeda@kernel.org>
parent b6f88506

rust/kernel/alloc/kbox.rs

+23 −18
Original line number Diff line number Diff line
@@ -15,6 +15,7 @@ 
use core::ptr::NonNull;

use core::result::Result;



use crate::ffi::c_void;

use crate::init::InPlaceInit;

use crate::types::ForeignOwnable;

use pin_init::{InPlaceWrite, Init, PinInit, ZeroableOption};
@@ -398,70 +399,74 @@ fn try_init<E>(init: impl Init<T, E>, flags: Flags) -> Result<Self, E> 
    }

}



// SAFETY: The `into_foreign` function returns a pointer that is well-aligned.

// SAFETY: The pointer returned by `into_foreign` comes from a well aligned

// pointer to `T`.

unsafe impl<T: 'static, A> ForeignOwnable for Box<T, A>

where

    A: Allocator,

{

    type PointedTo = T;

    const FOREIGN_ALIGN: usize = core::mem::align_of::<T>();

    type Borrowed<'a> = &'a T;

    type BorrowedMut<'a> = &'a mut T;



    fn into_foreign(self) -> *mut Self::PointedTo {

        Box::into_raw(self)

    fn into_foreign(self) -> *mut c_void {

        Box::into_raw(self).cast()

    }



    unsafe fn from_foreign(ptr: *mut Self::PointedTo) -> Self {

    unsafe fn from_foreign(ptr: *mut c_void) -> Self {

        // SAFETY: The safety requirements of this function ensure that `ptr` comes from a previous

        // call to `Self::into_foreign`.

        unsafe { Box::from_raw(ptr) }

        unsafe { Box::from_raw(ptr.cast()) }

    }



    unsafe fn borrow<'a>(ptr: *mut Self::PointedTo) -> &'a T {

    unsafe fn borrow<'a>(ptr: *mut c_void) -> &'a T {

        // SAFETY: The safety requirements of this method ensure that the object remains alive and

        // immutable for the duration of 'a.

        unsafe { &*ptr }

        unsafe { &*ptr.cast() }

    }



    unsafe fn borrow_mut<'a>(ptr: *mut Self::PointedTo) -> &'a mut T {

    unsafe fn borrow_mut<'a>(ptr: *mut c_void) -> &'a mut T {

        let ptr = ptr.cast();

        // SAFETY: The safety requirements of this method ensure that the pointer is valid and that

        // nothing else will access the value for the duration of 'a.

        unsafe { &mut *ptr }

    }

}



// SAFETY: The `into_foreign` function returns a pointer that is well-aligned.

// SAFETY: The pointer returned by `into_foreign` comes from a well aligned

// pointer to `T`.

unsafe impl<T: 'static, A> ForeignOwnable for Pin<Box<T, A>>

where

    A: Allocator,

{

    type PointedTo = T;

    const FOREIGN_ALIGN: usize = core::mem::align_of::<T>();

    type Borrowed<'a> = Pin<&'a T>;

    type BorrowedMut<'a> = Pin<&'a mut T>;



    fn into_foreign(self) -> *mut Self::PointedTo {

    fn into_foreign(self) -> *mut c_void {

        // SAFETY: We are still treating the box as pinned.

        Box::into_raw(unsafe { Pin::into_inner_unchecked(self) })

        Box::into_raw(unsafe { Pin::into_inner_unchecked(self) }).cast()

    }



    unsafe fn from_foreign(ptr: *mut Self::PointedTo) -> Self {

    unsafe fn from_foreign(ptr: *mut c_void) -> Self {

        // SAFETY: The safety requirements of this function ensure that `ptr` comes from a previous

        // call to `Self::into_foreign`.

        unsafe { Pin::new_unchecked(Box::from_raw(ptr)) }

        unsafe { Pin::new_unchecked(Box::from_raw(ptr.cast())) }

    }



    unsafe fn borrow<'a>(ptr: *mut Self::PointedTo) -> Pin<&'a T> {

    unsafe fn borrow<'a>(ptr: *mut c_void) -> Pin<&'a T> {

        // SAFETY: The safety requirements for this function ensure that the object is still alive,

        // so it is safe to dereference the raw pointer.

        // The safety requirements of `from_foreign` also ensure that the object remains alive for

        // the lifetime of the returned value.

        let r = unsafe { &*ptr };

        let r = unsafe { &*ptr.cast() };



        // SAFETY: This pointer originates from a `Pin<Box<T>>`.

        unsafe { Pin::new_unchecked(r) }

    }



    unsafe fn borrow_mut<'a>(ptr: *mut Self::PointedTo) -> Pin<&'a mut T> {

    unsafe fn borrow_mut<'a>(ptr: *mut c_void) -> Pin<&'a mut T> {

        let ptr = ptr.cast();

        // SAFETY: The safety requirements for this function ensure that the object is still alive,

        // so it is safe to dereference the raw pointer.

        // The safety requirements of `from_foreign` also ensure that the object remains alive for

rust/kernel/miscdevice.rs

+5 −5
Original line number Diff line number Diff line
@@ -217,7 +217,7 @@ impl<T: MiscDevice> MiscdeviceVTable<T> { 
        // type.

        //

        // SAFETY: The open call of a file can access the private data.

        unsafe { (*raw_file).private_data = ptr.into_foreign().cast() };

        unsafe { (*raw_file).private_data = ptr.into_foreign() };



        0

    }
@@ -228,7 +228,7 @@ impl<T: MiscDevice> MiscdeviceVTable<T> { 
    /// must be associated with a `MiscDeviceRegistration<T>`.

    unsafe extern "C" fn release(_inode: *mut bindings::inode, file: *mut bindings::file) -> c_int {

        // SAFETY: The release call of a file owns the private data.

        let private = unsafe { (*file).private_data }.cast();

        let private = unsafe { (*file).private_data };

        // SAFETY: The release call of a file owns the private data.

        let ptr = unsafe { <T::Ptr as ForeignOwnable>::from_foreign(private) };
@@ -272,7 +272,7 @@ impl<T: MiscDevice> MiscdeviceVTable<T> { 
    /// `file` must be a valid file that is associated with a `MiscDeviceRegistration<T>`.

    unsafe extern "C" fn ioctl(file: *mut bindings::file, cmd: c_uint, arg: c_ulong) -> c_long {

        // SAFETY: The ioctl call of a file can access the private data.

        let private = unsafe { (*file).private_data }.cast();

        let private = unsafe { (*file).private_data };

        // SAFETY: Ioctl calls can borrow the private data of the file.

        let device = unsafe { <T::Ptr as ForeignOwnable>::borrow(private) };
@@ -297,7 +297,7 @@ impl<T: MiscDevice> MiscdeviceVTable<T> { 
        arg: c_ulong,

    ) -> c_long {

        // SAFETY: The compat ioctl call of a file can access the private data.

        let private = unsafe { (*file).private_data }.cast();

        let private = unsafe { (*file).private_data };

        // SAFETY: Ioctl calls can borrow the private data of the file.

        let device = unsafe { <T::Ptr as ForeignOwnable>::borrow(private) };
@@ -318,7 +318,7 @@ impl<T: MiscDevice> MiscdeviceVTable<T> { 
    /// - `seq_file` must be a valid `struct seq_file` that we can write to.

    unsafe extern "C" fn show_fdinfo(seq_file: *mut bindings::seq_file, file: *mut bindings::file) {

        // SAFETY: The release call of a file owns the private data.

        let private = unsafe { (*file).private_data }.cast();

        let private = unsafe { (*file).private_data };

        // SAFETY: Ioctl calls can borrow the private data of the file.

        let device = unsafe { <T::Ptr as ForeignOwnable>::borrow(private) };

        // SAFETY:

rust/kernel/pci.rs

+1 −1
Original line number Diff line number Diff line
@@ -89,7 +89,7 @@ extern "C" fn probe_callback( 
    extern "C" fn remove_callback(pdev: *mut bindings::pci_dev) {

        // SAFETY: The PCI bus only ever calls the remove callback with a valid pointer to a

        // `struct pci_dev`.

        let ptr = unsafe { bindings::pci_get_drvdata(pdev) }.cast();

        let ptr = unsafe { bindings::pci_get_drvdata(pdev) };



        // SAFETY: `remove_callback` is only ever called after a successful call to

        // `probe_callback`, hence it's guaranteed that `ptr` points to a valid and initialized

rust/kernel/platform.rs

+1 −1
Original line number Diff line number Diff line
@@ -81,7 +81,7 @@ extern "C" fn probe_callback(pdev: *mut bindings::platform_device) -> kernel::ff 


    extern "C" fn remove_callback(pdev: *mut bindings::platform_device) {

        // SAFETY: `pdev` is a valid pointer to a `struct platform_device`.

        let ptr = unsafe { bindings::platform_get_drvdata(pdev) }.cast();

        let ptr = unsafe { bindings::platform_get_drvdata(pdev) };



        // SAFETY: `remove_callback` is only ever called after a successful call to

        // `probe_callback`, hence it's guaranteed that `ptr` points to a valid and initialized

rust/kernel/sync/arc.rs

+13 −11
Original line number Diff line number Diff line
@@ -19,6 +19,7 @@ 
use crate::{

    alloc::{AllocError, Flags, KBox},

    bindings,

    ffi::c_void,

    init::InPlaceInit,

    try_init,

    types::{ForeignOwnable, Opaque},
@@ -141,10 +142,9 @@ pub struct Arc<T: ?Sized> { 
    _p: PhantomData<ArcInner<T>>,

}



#[doc(hidden)]

#[pin_data]

#[repr(C)]

pub struct ArcInner<T: ?Sized> {

struct ArcInner<T: ?Sized> {

    refcount: Opaque<bindings::refcount_t>,

    data: T,

}
@@ -373,20 +373,22 @@ pub fn into_unique_or_drop(self) -> Option<Pin<UniqueArc<T>>> { 
    }

}



// SAFETY: The `into_foreign` function returns a pointer that is well-aligned.

// SAFETY: The pointer returned by `into_foreign` comes from a well aligned

// pointer to `ArcInner<T>`.

unsafe impl<T: 'static> ForeignOwnable for Arc<T> {

    type PointedTo = ArcInner<T>;

    const FOREIGN_ALIGN: usize = core::mem::align_of::<ArcInner<T>>();



    type Borrowed<'a> = ArcBorrow<'a, T>;

    type BorrowedMut<'a> = Self::Borrowed<'a>;



    fn into_foreign(self) -> *mut Self::PointedTo {

        ManuallyDrop::new(self).ptr.as_ptr()

    fn into_foreign(self) -> *mut c_void {

        ManuallyDrop::new(self).ptr.as_ptr().cast()

    }



    unsafe fn from_foreign(ptr: *mut Self::PointedTo) -> Self {

    unsafe fn from_foreign(ptr: *mut c_void) -> Self {

        // SAFETY: The safety requirements of this function ensure that `ptr` comes from a previous

        // call to `Self::into_foreign`.

        let inner = unsafe { NonNull::new_unchecked(ptr) };

        let inner = unsafe { NonNull::new_unchecked(ptr.cast::<ArcInner<T>>()) };



        // SAFETY: By the safety requirement of this function, we know that `ptr` came from

        // a previous call to `Arc::into_foreign`, which guarantees that `ptr` is valid and
@@ -394,17 +396,17 @@ unsafe fn from_foreign(ptr: *mut Self::PointedTo) -> Self { 
        unsafe { Self::from_inner(inner) }

    }



    unsafe fn borrow<'a>(ptr: *mut Self::PointedTo) -> ArcBorrow<'a, T> {

    unsafe fn borrow<'a>(ptr: *mut c_void) -> ArcBorrow<'a, T> {

        // SAFETY: The safety requirements of this function ensure that `ptr` comes from a previous

        // call to `Self::into_foreign`.

        let inner = unsafe { NonNull::new_unchecked(ptr) };

        let inner = unsafe { NonNull::new_unchecked(ptr.cast::<ArcInner<T>>()) };



        // SAFETY: The safety requirements of `from_foreign` ensure that the object remains alive

        // for the lifetime of the returned value.

        unsafe { ArcBorrow::new(inner) }

    }



    unsafe fn borrow_mut<'a>(ptr: *mut Self::PointedTo) -> ArcBorrow<'a, T> {

    unsafe fn borrow_mut<'a>(ptr: *mut c_void) -> ArcBorrow<'a, T> {

        // SAFETY: The safety requirements for `borrow_mut` are a superset of the safety

        // requirements for `borrow`.

        unsafe { <Self as ForeignOwnable>::borrow(ptr) }