From 22fad1a0e8cd38b1a9163a143fb31f5988dedbc7 Mon Sep 17 00:00:00 2001 From: Wedson Almeida Filho Date: Fri, 19 Mar 2021 01:34:42 +0000 Subject: [PATCH 1/5] Allow drivers to implement ioctls. --- rust/kernel/file_operations.rs | 134 ++++++++++++++++++++++++++++++++- 1 file changed, 132 insertions(+), 2 deletions(-) diff --git a/rust/kernel/file_operations.rs b/rust/kernel/file_operations.rs index 712a42fca4e5ef..7dae28eb9f289f 100644 --- a/rust/kernel/file_operations.rs +++ b/rust/kernel/file_operations.rs @@ -21,14 +21,26 @@ pub struct File { } impl File { + /// Constructors a new [`struct file`] wrapper. + /// + /// # Safety + /// + /// The pointer `ptr` must be non-null and valid for the lifetime of the object. unsafe fn from_ptr(ptr: *const bindings::file) -> File { File { ptr } } /// Returns the current seek/cursor/pointer position (`struct file::f_pos`). pub fn pos(&self) -> u64 { + // SAFETY: `File::ptr` is guaranteed to be valid by the constructor's requirement. unsafe { (*self.ptr).f_pos as u64 } } + + /// Returns whether the file is in blocking mode. + pub fn is_blocking(&self) -> bool { + // SAFETY: `File::ptr` is guaranteed to be valid by the constructor's requirement. + unsafe { (*self.ptr).f_flags & bindings::O_NONBLOCK == 0 } + } } /// Equivalent to [`std::io::SeekFrom`]. @@ -138,6 +150,30 @@ unsafe extern "C" fn llseek_callback( } } +unsafe extern "C" fn unlocked_ioctl_callback( + file: *mut bindings::file, + cmd: c_types::c_uint, + arg: c_types::c_ulong, +) -> c_types::c_long { + from_kernel_result! { + let f = &*((*file).private_data as *const T); + let ret = T::ioctl(f, &File::from_ptr(file), cmd as _, arg as _)?; + Ok(ret as _) + } +} + +unsafe extern "C" fn compat_ioctl_callback( + file: *mut bindings::file, + cmd: c_types::c_uint, + arg: c_types::c_ulong, +) -> c_types::c_long { + from_kernel_result! { + let f = &*((*file).private_data as *const T); + let ret = T::compat_ioctl(f, &File::from_ptr(file), cmd as _, arg as _)?; + Ok(ret as _) + } +} + unsafe extern "C" fn fsync_callback( file: *mut bindings::file, start: bindings::loff_t, @@ -177,7 +213,11 @@ impl FileOperationsVtable { }, check_flags: None, - compat_ioctl: None, + compat_ioctl: if T::TO_USE.compat_ioctl { + Some(compat_ioctl_callback::) + } else { + None + }, copy_file_range: None, fallocate: None, fadvise: None, @@ -205,7 +245,11 @@ impl FileOperationsVtable { show_fdinfo: None, splice_read: None, splice_write: None, - unlocked_ioctl: None, + unlocked_ioctl: if T::TO_USE.ioctl { + Some(unlocked_ioctl_callback::) + } else { + None + }, write_iter: None, }; } @@ -221,6 +265,12 @@ pub struct ToUse { /// The `llseek` field of [`struct file_operations`]. pub seek: bool, + /// The `unlocked_ioctl` field of [`struct file_operations`]. + pub ioctl: bool, + + /// The `compat_ioctl` field of [`struct file_operations`]. + pub compat_ioctl: bool, + /// The `fsync` field of [`struct file_operations`]. pub fsync: bool, } @@ -231,6 +281,8 @@ pub const USE_NONE: ToUse = ToUse { read: false, write: false, seek: false, + ioctl: false, + compat_ioctl: false, fsync: false, }; @@ -249,6 +301,70 @@ macro_rules! declare_file_operations { }; } +/// Allows the handling of ioctls defined with the `_IO`, `_IOR`, `_IOW`, and `_IOWR` macros. +/// +/// For each macro, there is a handler function that takes the appropriate types as arguments. +pub trait IoctlHandler: Sync { + /// Handles ioctls defined with the `_IO` macro, that is, with no buffer as argument. + fn pure(&self, _file: &File, _cmd: u32, _arg: usize) -> KernelResult { + Err(Error::EINVAL) + } + + /// Handles ioctls defined with the `_IOR` macro, that is, with an output buffer provided as + /// argument. + fn read(&self, _file: &File, _cmd: u32, _writer: &mut UserSlicePtrWriter) -> KernelResult { + Err(Error::EINVAL) + } + + /// Handles ioctls defined with the `_IOW` macro, that is, with an input buffer provided as + /// argument. + fn write( + &self, + _file: &File, + _cmd: u32, + _reader: &mut UserSlicePtrReader, + ) -> KernelResult { + Err(Error::EINVAL) + } + + /// Handles ioctls defined with the `_IOWR` macro, that is, with a buffer for both input and + /// output provided as argument. + fn read_write(&self, _file: &File, _cmd: u32, _data: UserSlicePtr) -> KernelResult { + Err(Error::EINVAL) + } +} + +/// Dispatches the given ioctl to the appropriate handler based on the value of `cmd`. It also +/// creates a [`UserSlicePtr`], [`UserSlicePtrReader`], or [`UserSlicePtrWriter`] depending on the +/// direction of the buffer of the command. +/// +/// It is meant to be used in implementations of [`FileOperations::ioctl`] and +/// [`FileOperations::compat_ioctl`]. +pub fn dispatch_ioctl( + handler: &T, + file: &File, + cmd: u32, + arg: usize, +) -> KernelResult { + let dir = (cmd >> bindings::_IOC_DIRSHIFT) & bindings::_IOC_DIRMASK; + if dir == bindings::_IOC_NONE { + return T::pure(handler, file, cmd, arg); + } + + let size = (cmd >> bindings::_IOC_SIZESHIFT) & bindings::_IOC_SIZEMASK; + + // SAFETY: The caller is reponsible for setting the appropriate `fs`. Additionally, we only + // create one instance of the user slice, so TOCTOU issues are not possible. + let data = unsafe { UserSlicePtr::new(arg as _, size as _) }?; + const READ_WRITE: u32 = bindings::_IOC_READ | bindings::_IOC_WRITE; + match dir { + bindings::_IOC_WRITE => T::write(handler, file, cmd, &mut data.reader()), + bindings::_IOC_READ => T::read(handler, file, cmd, &mut data.writer()), + READ_WRITE => T::read_write(handler, file, cmd, data), + _ => Err(Error::EINVAL), + } +} + /// Corresponds to the kernel's `struct file_operations`. /// /// You implement this trait whenever you would create a `struct file_operations`. @@ -296,6 +412,20 @@ pub trait FileOperations: Sync + Sized { Err(Error::EINVAL) } + /// Performs IO control operations that are specific to the file. + /// + /// Corresponds to the `unlocked_ioctl` function pointer in `struct file_operations`. + fn ioctl(&self, _file: &File, _cmd: u32, _arg: usize) -> KernelResult { + Err(Error::EINVAL) + } + + /// Performs 32-bit IO control operations on that are specific to the file on 64-bit kernels. + /// + /// Corresponds to the `compat_ioctl` function pointer in `struct file_operations`. + fn compat_ioctl(&self, _file: &File, _cmd: u32, _arg: usize) -> KernelResult { + Err(Error::EINVAL) + } + /// Syncs pending changes to this file. /// /// Corresponds to the `fsync` function pointer in `struct file_operations`. From c4c18ab93c9aab3762082e8921c806b29f041d3d Mon Sep 17 00:00:00 2001 From: Wedson Almeida Filho Date: Fri, 19 Mar 2021 18:02:36 +0000 Subject: [PATCH 2/5] Update ioctl support. --- rust/kernel/file_operations.rs | 75 ++++++++++++++++++++-------------- 1 file changed, 44 insertions(+), 31 deletions(-) diff --git a/rust/kernel/file_operations.rs b/rust/kernel/file_operations.rs index 7dae28eb9f289f..502cf19e3d99ce 100644 --- a/rust/kernel/file_operations.rs +++ b/rust/kernel/file_operations.rs @@ -157,7 +157,7 @@ unsafe extern "C" fn unlocked_ioctl_callback( ) -> c_types::c_long { from_kernel_result! { let f = &*((*file).private_data as *const T); - let ret = T::ioctl(f, &File::from_ptr(file), cmd as _, arg as _)?; + let ret = T::ioctl(f, &File::from_ptr(file), IoctlCommand(cmd as _), arg as _)?; Ok(ret as _) } } @@ -169,7 +169,7 @@ unsafe extern "C" fn compat_ioctl_callback( ) -> c_types::c_long { from_kernel_result! { let f = &*((*file).private_data as *const T); - let ret = T::compat_ioctl(f, &File::from_ptr(file), cmd as _, arg as _)?; + let ret = T::compat_ioctl(f, &File::from_ptr(file), IoctlCommand(cmd as _), arg as _)?; Ok(ret as _) } } @@ -334,34 +334,47 @@ pub trait IoctlHandler: Sync { } } -/// Dispatches the given ioctl to the appropriate handler based on the value of `cmd`. It also -/// creates a [`UserSlicePtr`], [`UserSlicePtrReader`], or [`UserSlicePtrWriter`] depending on the -/// direction of the buffer of the command. +/// Represents an ioctl command. /// -/// It is meant to be used in implementations of [`FileOperations::ioctl`] and -/// [`FileOperations::compat_ioctl`]. -pub fn dispatch_ioctl( - handler: &T, - file: &File, - cmd: u32, - arg: usize, -) -> KernelResult { - let dir = (cmd >> bindings::_IOC_DIRSHIFT) & bindings::_IOC_DIRMASK; - if dir == bindings::_IOC_NONE { - return T::pure(handler, file, cmd, arg); - } - - let size = (cmd >> bindings::_IOC_SIZESHIFT) & bindings::_IOC_SIZEMASK; - - // SAFETY: The caller is reponsible for setting the appropriate `fs`. Additionally, we only - // create one instance of the user slice, so TOCTOU issues are not possible. - let data = unsafe { UserSlicePtr::new(arg as _, size as _) }?; - const READ_WRITE: u32 = bindings::_IOC_READ | bindings::_IOC_WRITE; - match dir { - bindings::_IOC_WRITE => T::write(handler, file, cmd, &mut data.reader()), - bindings::_IOC_READ => T::read(handler, file, cmd, &mut data.writer()), - READ_WRITE => T::read_write(handler, file, cmd, data), - _ => Err(Error::EINVAL), +/// It can use the components of an ioctl command to dispatch ioctls using +/// [`IoctlCommand::dispatch`]. +pub struct IoctlCommand(u32); + +impl IoctlCommand { + /// Dispatches the given ioctl to the appropriate handler based on the value of the command. It + /// also creates a [`UserSlicePtr`], [`UserSlicePtrReader`], or [`UserSlicePtrWriter`] + /// depending on the direction of the buffer of the command. + /// + /// It is meant to be used in implementations of [`FileOperations::ioctl`] and + /// [`FileOperations::compat_ioctl`]. + /// + /// # Safety + /// + /// The caller must ensure that `fs` is set to a value compatible with the pointer in `arg`. + /// This is because this function uses [`UserSlicePtr`], which calls `access_ok`. + pub unsafe fn dispatch( + &self, + handler: &T, + file: &File, + arg: usize, + ) -> KernelResult { + let dir = (self.0 >> bindings::_IOC_DIRSHIFT) & bindings::_IOC_DIRMASK; + if dir == bindings::_IOC_NONE { + return T::pure(handler, file, self.0, arg); + } + + let size = (self.0 >> bindings::_IOC_SIZESHIFT) & bindings::_IOC_SIZEMASK; + + // SAFETY: We only create one instance of the user slice, so TOCTOU issues are not possible. + // The `set_fs` requirements are imposed on the caller. + let data = UserSlicePtr::new(arg as _, size as _)?; + const READ_WRITE: u32 = bindings::_IOC_READ | bindings::_IOC_WRITE; + match dir { + bindings::_IOC_WRITE => T::write(handler, file, self.0, &mut data.reader()), + bindings::_IOC_READ => T::read(handler, file, self.0, &mut data.writer()), + READ_WRITE => T::read_write(handler, file, self.0, data), + _ => Err(Error::EINVAL), + } } } @@ -415,14 +428,14 @@ pub trait FileOperations: Sync + Sized { /// Performs IO control operations that are specific to the file. /// /// Corresponds to the `unlocked_ioctl` function pointer in `struct file_operations`. - fn ioctl(&self, _file: &File, _cmd: u32, _arg: usize) -> KernelResult { + fn ioctl(&self, _file: &File, _cmd: IoctlCommand, _arg: usize) -> KernelResult { Err(Error::EINVAL) } /// Performs 32-bit IO control operations on that are specific to the file on 64-bit kernels. /// /// Corresponds to the `compat_ioctl` function pointer in `struct file_operations`. - fn compat_ioctl(&self, _file: &File, _cmd: u32, _arg: usize) -> KernelResult { + fn compat_ioctl(&self, _file: &File, _cmd: IoctlCommand, _arg: usize) -> KernelResult { Err(Error::EINVAL) } From 472e4e964431453a4f78208ecd9bfc40e384dbdc Mon Sep 17 00:00:00 2001 From: Wedson Almeida Filho Date: Fri, 19 Mar 2021 18:25:59 +0000 Subject: [PATCH 3/5] Add invariant annotations. --- rust/kernel/file_operations.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/rust/kernel/file_operations.rs b/rust/kernel/file_operations.rs index 502cf19e3d99ce..3f90167ca66050 100644 --- a/rust/kernel/file_operations.rs +++ b/rust/kernel/file_operations.rs @@ -16,6 +16,10 @@ use crate::error::{Error, KernelResult}; use crate::user_ptr::{UserSlicePtr, UserSlicePtrReader, UserSlicePtrWriter}; /// Wraps the kernel's `struct file`. +/// +/// # Invariants +/// +/// The pointer [`File::ptr`] is non-null and valid. pub struct File { ptr: *const bindings::file, } @@ -27,18 +31,19 @@ impl File { /// /// The pointer `ptr` must be non-null and valid for the lifetime of the object. unsafe fn from_ptr(ptr: *const bindings::file) -> File { + // INVARIANTS: the safety contract ensures the type invariant will hold. File { ptr } } /// Returns the current seek/cursor/pointer position (`struct file::f_pos`). pub fn pos(&self) -> u64 { - // SAFETY: `File::ptr` is guaranteed to be valid by the constructor's requirement. + // SAFETY: `File::ptr` is guaranteed to be valid by the type invariants. unsafe { (*self.ptr).f_pos as u64 } } /// Returns whether the file is in blocking mode. pub fn is_blocking(&self) -> bool { - // SAFETY: `File::ptr` is guaranteed to be valid by the constructor's requirement. + // SAFETY: `File::ptr` is guaranteed to be valid by the type invariants. unsafe { (*self.ptr).f_flags & bindings::O_NONBLOCK == 0 } } } From 30b99f88e12993dcec87e8b91841fdc6e786331b Mon Sep 17 00:00:00 2001 From: Wedson Almeida Filho Date: Fri, 19 Mar 2021 21:19:50 +0000 Subject: [PATCH 4/5] Add `IoctlCommand::raw` and fix typo in comment. --- rust/kernel/file_operations.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/rust/kernel/file_operations.rs b/rust/kernel/file_operations.rs index 3f90167ca66050..75258cab5e43d0 100644 --- a/rust/kernel/file_operations.rs +++ b/rust/kernel/file_operations.rs @@ -25,7 +25,7 @@ pub struct File { } impl File { - /// Constructors a new [`struct file`] wrapper. + /// Constructs a new [`struct file`] wrapper. /// /// # Safety /// @@ -381,6 +381,11 @@ impl IoctlCommand { _ => Err(Error::EINVAL), } } + + /// Returns the raw 32-bit value of the command. + pub fn raw(&self) -> u32 { + self.0 + } } /// Corresponds to the kernel's `struct file_operations`. From 6cc9ab5605c43f388406568f935011491b8bfb05 Mon Sep 17 00:00:00 2001 From: Wedson Almeida Filho Date: Fri, 19 Mar 2021 22:57:05 +0000 Subject: [PATCH 5/5] Make `dispatch` safe. --- rust/kernel/file_operations.rs | 80 ++++++++++++++++++++++------------ 1 file changed, 51 insertions(+), 29 deletions(-) diff --git a/rust/kernel/file_operations.rs b/rust/kernel/file_operations.rs index 75258cab5e43d0..094cdd0fa12728 100644 --- a/rust/kernel/file_operations.rs +++ b/rust/kernel/file_operations.rs @@ -162,7 +162,9 @@ unsafe extern "C" fn unlocked_ioctl_callback( ) -> c_types::c_long { from_kernel_result! { let f = &*((*file).private_data as *const T); - let ret = T::ioctl(f, &File::from_ptr(file), IoctlCommand(cmd as _), arg as _)?; + // SAFETY: This function is called by the kernel, so it must set `fs` appropriately. + let mut cmd = IoctlCommand::new(cmd as _, arg as _); + let ret = T::ioctl(f, &File::from_ptr(file), &mut cmd)?; Ok(ret as _) } } @@ -174,7 +176,9 @@ unsafe extern "C" fn compat_ioctl_callback( ) -> c_types::c_long { from_kernel_result! { let f = &*((*file).private_data as *const T); - let ret = T::compat_ioctl(f, &File::from_ptr(file), IoctlCommand(cmd as _), arg as _)?; + // SAFETY: This function is called by the kernel, so it must set `fs` appropriately. + let mut cmd = IoctlCommand::new(cmd as _, arg as _); + let ret = T::compat_ioctl(f, &File::from_ptr(file), &mut cmd)?; Ok(ret as _) } } @@ -343,48 +347,66 @@ pub trait IoctlHandler: Sync { /// /// It can use the components of an ioctl command to dispatch ioctls using /// [`IoctlCommand::dispatch`]. -pub struct IoctlCommand(u32); +pub struct IoctlCommand { + cmd: u32, + arg: usize, + user_slice: Option, +} impl IoctlCommand { + /// Constructs a new [`IoctlCommand`]. + /// + /// # Safety + /// + /// The caller must ensure that `fs` is compatible with `arg` and the original caller's + /// context. For example, if the original caller is from userland (e.g., through the ioctl + /// syscall), then `arg` is untrusted and `fs` should therefore be `USER_DS`. + unsafe fn new(cmd: u32, arg: usize) -> Self { + let user_slice = { + let dir = (cmd >> bindings::_IOC_DIRSHIFT) & bindings::_IOC_DIRMASK; + if dir == bindings::_IOC_NONE { + None + } else { + let size = (cmd >> bindings::_IOC_SIZESHIFT) & bindings::_IOC_SIZEMASK; + + // SAFETY: We only create one instance of the user slice, so TOCTOU issues are not + // possible. The `set_fs` requirements are imposed on the caller. + UserSlicePtr::new(arg as _, size as _).ok() + } + }; + + Self { + cmd, + arg, + user_slice, + } + } + /// Dispatches the given ioctl to the appropriate handler based on the value of the command. It /// also creates a [`UserSlicePtr`], [`UserSlicePtrReader`], or [`UserSlicePtrWriter`] /// depending on the direction of the buffer of the command. /// /// It is meant to be used in implementations of [`FileOperations::ioctl`] and /// [`FileOperations::compat_ioctl`]. - /// - /// # Safety - /// - /// The caller must ensure that `fs` is set to a value compatible with the pointer in `arg`. - /// This is because this function uses [`UserSlicePtr`], which calls `access_ok`. - pub unsafe fn dispatch( - &self, - handler: &T, - file: &File, - arg: usize, - ) -> KernelResult { - let dir = (self.0 >> bindings::_IOC_DIRSHIFT) & bindings::_IOC_DIRMASK; + pub fn dispatch(&mut self, handler: &T, file: &File) -> KernelResult { + let dir = (self.cmd >> bindings::_IOC_DIRSHIFT) & bindings::_IOC_DIRMASK; if dir == bindings::_IOC_NONE { - return T::pure(handler, file, self.0, arg); + return T::pure(handler, file, self.cmd, self.arg); } - let size = (self.0 >> bindings::_IOC_SIZESHIFT) & bindings::_IOC_SIZEMASK; - - // SAFETY: We only create one instance of the user slice, so TOCTOU issues are not possible. - // The `set_fs` requirements are imposed on the caller. - let data = UserSlicePtr::new(arg as _, size as _)?; + let data = self.user_slice.take().ok_or(Error::EFAULT)?; const READ_WRITE: u32 = bindings::_IOC_READ | bindings::_IOC_WRITE; match dir { - bindings::_IOC_WRITE => T::write(handler, file, self.0, &mut data.reader()), - bindings::_IOC_READ => T::read(handler, file, self.0, &mut data.writer()), - READ_WRITE => T::read_write(handler, file, self.0, data), + bindings::_IOC_WRITE => T::write(handler, file, self.cmd, &mut data.reader()), + bindings::_IOC_READ => T::read(handler, file, self.cmd, &mut data.writer()), + READ_WRITE => T::read_write(handler, file, self.cmd, data), _ => Err(Error::EINVAL), } } - /// Returns the raw 32-bit value of the command. - pub fn raw(&self) -> u32 { - self.0 + /// Returns the raw 32-bit value of the command and the ptr-sized argument. + pub fn raw(&self) -> (u32, usize) { + (self.cmd, self.arg) } } @@ -438,14 +460,14 @@ pub trait FileOperations: Sync + Sized { /// Performs IO control operations that are specific to the file. /// /// Corresponds to the `unlocked_ioctl` function pointer in `struct file_operations`. - fn ioctl(&self, _file: &File, _cmd: IoctlCommand, _arg: usize) -> KernelResult { + fn ioctl(&self, _file: &File, _cmd: &mut IoctlCommand) -> KernelResult { Err(Error::EINVAL) } /// Performs 32-bit IO control operations on that are specific to the file on 64-bit kernels. /// /// Corresponds to the `compat_ioctl` function pointer in `struct file_operations`. - fn compat_ioctl(&self, _file: &File, _cmd: IoctlCommand, _arg: usize) -> KernelResult { + fn compat_ioctl(&self, _file: &File, _cmd: &mut IoctlCommand) -> KernelResult { Err(Error::EINVAL) }