Commit 74fc3493 authored by Alice Ryhl's avatar Alice Ryhl Committed by Greg Kroah-Hartman
Browse files

rust: miscdevice: change how f_ops vtable is constructed 



I was helping someone with writing a new Rust abstraction, and we were
using the miscdevice abstraction as an example. While doing this, it
became clear to me that the way I implemented the f_ops vtable is
confusing to new Rust users, and that the approach used by the block
abstractions is less confusing.

Thus, update the miscdevice abstractions to use the same approach as
rust/kernel/block/mq/operations.rs.

Sorry about the large diff. This changes the indentation of a large
amount of code.

Reviewed-by: default avatarChristian Schrefl <chrisi.schrefl@gmail.com>
Signed-off-by: default avatarAlice Ryhl <aliceryhl@google.com>
Link: https://lore.kernel.org/r/20250227-miscdevice-fops-change-v1-1-c9e9b75d67eb@google.com


Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent 264ff841

rust/kernel/miscdevice.rs

+143 −154
Original line number Diff line number Diff line
@@ -35,7 +35,7 @@ pub const fn into_raw<T: MiscDevice>(self) -> bindings::miscdevice { 
        let mut result: bindings::miscdevice = unsafe { MaybeUninit::zeroed().assume_init() };

        result.minor = bindings::MISC_DYNAMIC_MINOR as _;

        result.name = self.name.as_char_ptr();

        result.fops = create_vtable::<T>();

        result.fops = MiscdeviceVTable::<T>::build();

        result

    }

}
@@ -160,48 +160,15 @@ fn show_fdinfo( 
    }

}



const fn create_vtable<T: MiscDevice>() -> &'static bindings::file_operations {

    const fn maybe_fn<T: Copy>(check: bool, func: T) -> Option<T> {

        if check {

            Some(func)

        } else {

            None

        }

    }



    struct VtableHelper<T: MiscDevice> {

        _t: PhantomData<T>,

    }

    impl<T: MiscDevice> VtableHelper<T> {

        const VTABLE: bindings::file_operations = bindings::file_operations {

            open: Some(fops_open::<T>),

            release: Some(fops_release::<T>),

            unlocked_ioctl: maybe_fn(T::HAS_IOCTL, fops_ioctl::<T>),

            #[cfg(CONFIG_COMPAT)]

            compat_ioctl: if T::HAS_COMPAT_IOCTL {

                Some(fops_compat_ioctl::<T>)

            } else if T::HAS_IOCTL {

                Some(bindings::compat_ptr_ioctl)

            } else {

                None

            },

            show_fdinfo: maybe_fn(T::HAS_SHOW_FDINFO, fops_show_fdinfo::<T>),

            // SAFETY: All zeros is a valid value for `bindings::file_operations`.

            ..unsafe { MaybeUninit::zeroed().assume_init() }

        };

    }



    &VtableHelper::<T>::VTABLE

}

/// A vtable for the file operations of a Rust miscdevice.

struct MiscdeviceVTable<T: MiscDevice>(PhantomData<T>);



impl<T: MiscDevice> MiscdeviceVTable<T> {

    /// # Safety

    ///

    /// `file` and `inode` must be the file and inode for a file that is undergoing initialization.

    /// The file must be associated with a `MiscDeviceRegistration<T>`.

unsafe extern "C" fn fops_open<T: MiscDevice>(

    inode: *mut bindings::inode,

    raw_file: *mut bindings::file,

) -> c_int {

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

        // SAFETY: The pointers are valid and for a file being opened.

        let ret = unsafe { bindings::generic_file_open(inode, raw_file) };

        if ret != 0 {
@@ -212,8 +179,9 @@ impl<T: MiscDevice> VtableHelper<T> { 
        let misc_ptr = unsafe { (*raw_file).private_data };



        // SAFETY: This is a miscdevice, so `misc_open()` set the private data to a pointer to the

    // associated `struct miscdevice` before calling into this method. Furthermore, `misc_open()`

    // ensures that the miscdevice can't be unregistered and freed during this call to `fops_open`.

        // associated `struct miscdevice` before calling into this method. Furthermore,

        // `misc_open()` ensures that the miscdevice can't be unregistered and freed during this

        // call to `fops_open`.

        let misc = unsafe { &*misc_ptr.cast::<MiscDeviceRegistration<T>>() };



        // SAFETY:
@@ -226,9 +194,10 @@ impl<T: MiscDevice> VtableHelper<T> { 
            Err(err) => return err.to_errno(),

        };



    // This overwrites the private data with the value specified by the user, changing the type of

    // this file's private data. All future accesses to the private data is performed by other

    // fops_* methods in this file, which all correctly cast the private data to the new type.

        // This overwrites the private data with the value specified by the user, changing the type

        // of this file's private data. All future accesses to the private data is performed by

        // other fops_* methods in this file, which all correctly cast the private data to the new

        // type.

        //

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

        unsafe { (*raw_file).private_data = ptr.into_foreign() };
@@ -238,12 +207,9 @@ impl<T: MiscDevice> VtableHelper<T> { 


    /// # Safety

    ///

/// `file` and `inode` must be the file and inode for a file that is being released. The file must

/// be associated with a `MiscDeviceRegistration<T>`.

unsafe extern "C" fn fops_release<T: MiscDevice>(

    _inode: *mut bindings::inode,

    file: *mut bindings::file,

) -> c_int {

    /// `file` and `inode` must be the file and inode for a file that is being released. The file

    /// 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 };

        // SAFETY: The release call of a file owns the private data.
@@ -260,11 +226,7 @@ impl<T: MiscDevice> VtableHelper<T> { 
    /// # Safety

    ///

    /// `file` must be a valid file that is associated with a `MiscDeviceRegistration<T>`.

unsafe extern "C" fn fops_ioctl<T: MiscDevice>(

    file: *mut bindings::file,

    cmd: c_uint,

    arg: c_ulong,

) -> c_long {

    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 };

        // SAFETY: Ioctl calls can borrow the private data of the file.
@@ -285,7 +247,7 @@ impl<T: MiscDevice> VtableHelper<T> { 
    ///

    /// `file` must be a valid file that is associated with a `MiscDeviceRegistration<T>`.

    #[cfg(CONFIG_COMPAT)]

unsafe extern "C" fn fops_compat_ioctl<T: MiscDevice>(

    unsafe extern "C" fn compat_ioctl(

        file: *mut bindings::file,

        cmd: c_uint,

        arg: c_ulong,
@@ -310,10 +272,7 @@ impl<T: MiscDevice> VtableHelper<T> { 
    ///

    /// - `file` must be a valid file that is associated with a `MiscDeviceRegistration<T>`.

    /// - `seq_file` must be a valid `struct seq_file` that we can write to.

unsafe extern "C" fn fops_show_fdinfo<T: MiscDevice>(

    seq_file: *mut bindings::seq_file,

    file: *mut bindings::file,

) {

    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 };

        // SAFETY: Ioctl calls can borrow the private data of the file.
@@ -322,9 +281,39 @@ impl<T: MiscDevice> VtableHelper<T> { 
        // * The file is valid for the duration of this call.

        // * There is no active fdget_pos region on the file on this thread.

        let file = unsafe { File::from_raw_file(file) };

    // SAFETY: The caller ensures that the pointer is valid and exclusive for the duration in which

    // this method is called.

        // SAFETY: The caller ensures that the pointer is valid and exclusive for the duration in

        // which this method is called.

        let m = unsafe { SeqFile::from_raw(seq_file) };



        T::show_fdinfo(device, m, file);

    }



    const VTABLE: bindings::file_operations = bindings::file_operations {

        open: Some(Self::open),

        release: Some(Self::release),

        unlocked_ioctl: if T::HAS_IOCTL {

            Some(Self::ioctl)

        } else {

            None

        },

        #[cfg(CONFIG_COMPAT)]

        compat_ioctl: if T::HAS_COMPAT_IOCTL {

            Some(Self::compat_ioctl)

        } else if T::HAS_IOCTL {

            Some(bindings::compat_ptr_ioctl)

        } else {

            None

        },

        show_fdinfo: if T::HAS_SHOW_FDINFO {

            Some(Self::show_fdinfo)

        } else {

            None

        },

        // SAFETY: All zeros is a valid value for `bindings::file_operations`.

        ..unsafe { MaybeUninit::zeroed().assume_init() }

    };



    const fn build() -> &'static bindings::file_operations {

        &Self::VTABLE

    }

}