From 2117c9ffdcf954ab8786565215a1a0789e6fe52e Mon Sep 17 00:00:00 2001 From: Wedson Almeida Filho Date: Thu, 17 Jun 2021 19:18:00 +0100 Subject: [PATCH] rust: update `FileOperations` to use arbitrary type from `PointerWrapper`. This allows `FileOperations` implementers to have arbitrary (wrapped) values as `private_data` (as opposed to only wrapped `Self`). It also allows wrappers to dictate the borrowed type. For now all wrappers of `T` just borrow `&T`, but in a subsequent PR `Ref` will borrow to `&Ref`, which allows implementations to increment the refcount. Signed-off-by: Wedson Almeida Filho --- drivers/android/process.rs | 18 +-- drivers/char/hw_random/bcm2835_rng_rust.rs | 2 +- rust/kernel/file_operations.rs | 136 ++++++++++++++++----- samples/rust/rust_miscdev.rs | 36 +++--- samples/rust/rust_random.rs | 4 +- samples/rust/rust_semaphore.rs | 18 ++- 6 files changed, 146 insertions(+), 68 deletions(-) diff --git a/drivers/android/process.rs b/drivers/android/process.rs index 79a7d7c18ce893..b0683778318ab2 100644 --- a/drivers/android/process.rs +++ b/drivers/android/process.rs @@ -891,15 +891,15 @@ impl FileOperations for Process { } } - fn ioctl(&self, file: &File, cmd: &mut IoctlCommand) -> Result { - cmd.dispatch(self, file) + fn ioctl(this: &Process, file: &File, cmd: &mut IoctlCommand) -> Result { + cmd.dispatch::(this, file) } - fn compat_ioctl(&self, file: &File, cmd: &mut IoctlCommand) -> Result { - cmd.dispatch(self, file) + fn compat_ioctl(this: &Process, file: &File, cmd: &mut IoctlCommand) -> Result { + cmd.dispatch::(this, file) } - fn mmap(&self, _file: &File, vma: &mut bindings::vm_area_struct) -> Result { + fn mmap(this: &Process, _file: &File, vma: &mut bindings::vm_area_struct) -> Result { // TODO: Only group leader is allowed to create mappings. if vma.vm_start == 0 { @@ -914,13 +914,13 @@ impl FileOperations for Process { vma.vm_flags &= !(bindings::VM_MAYWRITE as c_types::c_ulong); // TODO: Set ops. We need to learn when the user unmaps so that we can stop using it. - self.create_mapping(vma) + this.create_mapping(vma) } - fn poll(&self, file: &File, table: &PollTable) -> Result { - let thread = self.get_thread(unsafe { rust_helper_current_pid() })?; + fn poll(this: &Process, file: &File, table: &PollTable) -> Result { + let thread = this.get_thread(unsafe { rust_helper_current_pid() })?; let (from_proc, mut mask) = thread.poll(file, table); - if mask == 0 && from_proc && !self.inner.lock().work.is_empty() { + if mask == 0 && from_proc && !this.inner.lock().work.is_empty() { mask |= bindings::POLLIN; } Ok(mask) diff --git a/drivers/char/hw_random/bcm2835_rng_rust.rs b/drivers/char/hw_random/bcm2835_rng_rust.rs index ade4ee95950d24..62f0fe01035ba6 100644 --- a/drivers/char/hw_random/bcm2835_rng_rust.rs +++ b/drivers/char/hw_random/bcm2835_rng_rust.rs @@ -37,7 +37,7 @@ impl FileOpener<()> for RngDevice { impl FileOperations for RngDevice { kernel::declare_file_operations!(read); - fn read(&self, _: &File, data: &mut T, offset: u64) -> Result { + fn read(_: &Self, _: &File, data: &mut T, offset: u64) -> Result { // Succeed if the caller doesn't provide a buffer or if not at the start. if data.is_empty() || offset != 0 { return Ok(0); diff --git a/rust/kernel/file_operations.rs b/rust/kernel/file_operations.rs index 7891a61695089f..0b300513a6ecd1 100644 --- a/rust/kernel/file_operations.rs +++ b/rust/kernel/file_operations.rs @@ -5,7 +5,7 @@ //! C header: [`include/linux/fs.h`](../../../../include/linux/fs.h) use core::convert::{TryFrom, TryInto}; -use core::{marker, mem, ptr}; +use core::{marker, mem, ops::Deref, ptr}; use alloc::boxed::Box; @@ -99,10 +99,14 @@ unsafe extern "C" fn read_callback( ) -> c_types::c_ssize_t { from_kernel_result! { let mut data = unsafe { UserSlicePtr::new(buf as *mut c_types::c_void, len).writer() }; - let f = unsafe { &*((*file).private_data as *const T) }; + // SAFETY: `private_data` was initialised by `open_callback` with a value returned by + // `T::Wrapper::into_pointer`. `T::Wrapper::from_pointer` is only called by the `release` + // callback, which the C API guarantees that will be called only when all references to + // `file` have been released, so we know it can't be called while this function is running. + let f = unsafe { T::Wrapper::borrow((*file).private_data) }; // No `FMODE_UNSIGNED_OFFSET` support, so `offset` must be in [0, 2^63). // See discussion in https://github.com/fishinabarrel/linux-kernel-module-rust/pull/113 - let read = f.read(unsafe { &FileRef::from_ptr(file) }, &mut data, unsafe { *offset }.try_into()?)?; + let read = T::read(&f, unsafe { &FileRef::from_ptr(file) }, &mut data, unsafe { *offset }.try_into()?)?; unsafe { (*offset) += bindings::loff_t::try_from(read).unwrap() }; Ok(read as _) } @@ -116,8 +120,12 @@ unsafe extern "C" fn read_iter_callback( let mut iter = unsafe { IovIter::from_ptr(raw_iter) }; let file = unsafe { (*iocb).ki_filp }; let offset = unsafe { (*iocb).ki_pos }; - let f = unsafe { &*((*file).private_data as *const T) }; - let read = f.read(unsafe { &FileRef::from_ptr(file) }, &mut iter, offset.try_into()?)?; + // SAFETY: `private_data` was initialised by `open_callback` with a value returned by + // `T::Wrapper::into_pointer`. `T::Wrapper::from_pointer` is only called by the `release` + // callback, which the C API guarantees that will be called only when all references to + // `file` have been released, so we know it can't be called while this function is running. + let f = unsafe { T::Wrapper::borrow((*file).private_data) }; + let read = T::read(&f, unsafe { &FileRef::from_ptr(file) }, &mut iter, offset.try_into()?)?; unsafe { (*iocb).ki_pos += bindings::loff_t::try_from(read).unwrap() }; Ok(read as _) } @@ -131,10 +139,14 @@ unsafe extern "C" fn write_callback( ) -> c_types::c_ssize_t { from_kernel_result! { let mut data = unsafe { UserSlicePtr::new(buf as *mut c_types::c_void, len).reader() }; - let f = unsafe { &*((*file).private_data as *const T) }; + // SAFETY: `private_data` was initialised by `open_callback` with a value returned by + // `T::Wrapper::into_pointer`. `T::Wrapper::from_pointer` is only called by the `release` + // callback, which the C API guarantees that will be called only when all references to + // `file` have been released, so we know it can't be called while this function is running. + let f = unsafe { T::Wrapper::borrow((*file).private_data) }; // No `FMODE_UNSIGNED_OFFSET` support, so `offset` must be in [0, 2^63). // See discussion in https://github.com/fishinabarrel/linux-kernel-module-rust/pull/113 - let written = f.write(unsafe { &FileRef::from_ptr(file) }, &mut data, unsafe { *offset }.try_into()?)?; + let written = T::write(&f, unsafe { &FileRef::from_ptr(file) }, &mut data, unsafe { *offset }.try_into()?)?; unsafe { (*offset) += bindings::loff_t::try_from(written).unwrap() }; Ok(written as _) } @@ -148,8 +160,12 @@ unsafe extern "C" fn write_iter_callback( let mut iter = unsafe { IovIter::from_ptr(raw_iter) }; let file = unsafe { (*iocb).ki_filp }; let offset = unsafe { (*iocb).ki_pos }; - let f = unsafe { &*((*file).private_data as *const T) }; - let written = f.write(unsafe { &FileRef::from_ptr(file) }, &mut iter, offset.try_into()?)?; + // SAFETY: `private_data` was initialised by `open_callback` with a value returned by + // `T::Wrapper::into_pointer`. `T::Wrapper::from_pointer` is only called by the `release` + // callback, which the C API guarantees that will be called only when all references to + // `file` have been released, so we know it can't be called while this function is running. + let f = unsafe { T::Wrapper::borrow((*file).private_data) }; + let written = T::write(&f, unsafe { &FileRef::from_ptr(file) }, &mut iter, offset.try_into()?)?; unsafe { (*iocb).ki_pos += bindings::loff_t::try_from(written).unwrap() }; Ok(written as _) } @@ -178,8 +194,12 @@ unsafe extern "C" fn llseek_callback( bindings::SEEK_END => SeekFrom::End(offset), _ => return Err(Error::EINVAL), }; - let f = unsafe { &*((*file).private_data as *const T) }; - let off = f.seek(unsafe { &FileRef::from_ptr(file) }, off)?; + // SAFETY: `private_data` was initialised by `open_callback` with a value returned by + // `T::Wrapper::into_pointer`. `T::Wrapper::from_pointer` is only called by the `release` + // callback, which the C API guarantees that will be called only when all references to + // `file` have been released, so we know it can't be called while this function is running. + let f = unsafe { T::Wrapper::borrow((*file).private_data) }; + let off = T::seek(&f, unsafe { &FileRef::from_ptr(file) }, off)?; Ok(off as bindings::loff_t) } } @@ -190,10 +210,13 @@ unsafe extern "C" fn unlocked_ioctl_callback( arg: c_types::c_ulong, ) -> c_types::c_long { from_kernel_result! { - let f = unsafe { &*((*file).private_data as *const T) }; - // SAFETY: This function is called by the kernel, so it must set `fs` appropriately. + // SAFETY: `private_data` was initialised by `open_callback` with a value returned by + // `T::Wrapper::into_pointer`. `T::Wrapper::from_pointer` is only called by the `release` + // callback, which the C API guarantees that will be called only when all references to + // `file` have been released, so we know it can't be called while this function is running. + let f = unsafe { T::Wrapper::borrow((*file).private_data) }; let mut cmd = IoctlCommand::new(cmd as _, arg as _); - let ret = f.ioctl(unsafe { &FileRef::from_ptr(file) }, &mut cmd)?; + let ret = T::ioctl(&f, unsafe { &FileRef::from_ptr(file) }, &mut cmd)?; Ok(ret as _) } } @@ -204,10 +227,13 @@ unsafe extern "C" fn compat_ioctl_callback( arg: c_types::c_ulong, ) -> c_types::c_long { from_kernel_result! { - let f = unsafe { &*((*file).private_data as *const T) }; - // SAFETY: This function is called by the kernel, so it must set `fs` appropriately. + // SAFETY: `private_data` was initialised by `open_callback` with a value returned by + // `T::Wrapper::into_pointer`. `T::Wrapper::from_pointer` is only called by the `release` + // callback, which the C API guarantees that will be called only when all references to + // `file` have been released, so we know it can't be called while this function is running. + let f = unsafe { T::Wrapper::borrow((*file).private_data) }; let mut cmd = IoctlCommand::new(cmd as _, arg as _); - let ret = f.compat_ioctl(unsafe { &FileRef::from_ptr(file) }, &mut cmd)?; + let ret = T::compat_ioctl(&f, unsafe { &FileRef::from_ptr(file) }, &mut cmd)?; Ok(ret as _) } } @@ -217,8 +243,12 @@ unsafe extern "C" fn mmap_callback( vma: *mut bindings::vm_area_struct, ) -> c_types::c_int { from_kernel_result! { - let f = unsafe { &*((*file).private_data as *const T) }; - f.mmap(unsafe { &FileRef::from_ptr(file) }, unsafe { &mut *vma })?; + // SAFETY: `private_data` was initialised by `open_callback` with a value returned by + // `T::Wrapper::into_pointer`. `T::Wrapper::from_pointer` is only called by the `release` + // callback, which the C API guarantees that will be called only when all references to + // `file` have been released, so we know it can't be called while this function is running. + let f = unsafe { T::Wrapper::borrow((*file).private_data) }; + T::mmap(&f, unsafe { &FileRef::from_ptr(file) }, unsafe { &mut *vma })?; Ok(0) } } @@ -233,8 +263,12 @@ unsafe extern "C" fn fsync_callback( let start = start.try_into()?; let end = end.try_into()?; let datasync = datasync != 0; - let f = unsafe { &*((*file).private_data as *const T) }; - let res = f.fsync(unsafe { &FileRef::from_ptr(file) }, start, end, datasync)?; + // SAFETY: `private_data` was initialised by `open_callback` with a value returned by + // `T::Wrapper::into_pointer`. `T::Wrapper::from_pointer` is only called by the `release` + // callback, which the C API guarantees that will be called only when all references to + // `file` have been released, so we know it can't be called while this function is running. + let f = unsafe { T::Wrapper::borrow((*file).private_data) }; + let res = T::fsync(&f, unsafe { &FileRef::from_ptr(file) }, start, end, datasync)?; Ok(res.try_into().unwrap()) } } @@ -243,8 +277,12 @@ unsafe extern "C" fn poll_callback( file: *mut bindings::file, wait: *mut bindings::poll_table_struct, ) -> bindings::__poll_t { - let f = unsafe { &*((*file).private_data as *const T) }; - match f.poll(unsafe { &FileRef::from_ptr(file) }, unsafe { + // SAFETY: `private_data` was initialised by `open_callback` with a value returned by + // `T::Wrapper::into_pointer`. `T::Wrapper::from_pointer` is only called by the `release` + // callback, which the C API guarantees that will be called only when all references to `file` + // have been released, so we know it can't be called while this function is running. + let f = unsafe { T::Wrapper::borrow((*file).private_data) }; + match T::poll(&f, unsafe { &FileRef::from_ptr(file) }, unsafe { &PollTable::from_ptr(wait) }) { Ok(v) => v, @@ -549,42 +587,70 @@ pub trait FileOperations: Send + Sync + Sized { /// Reads data from this file to the caller's buffer. /// /// Corresponds to the `read` and `read_iter` function pointers in `struct file_operations`. - fn read(&self, _file: &File, _data: &mut T, _offset: u64) -> Result { + fn read( + _this: &<::Borrowed as Deref>::Target, + _file: &File, + _data: &mut T, + _offset: u64, + ) -> Result { Err(Error::EINVAL) } /// Writes data from the caller's buffer to this file. /// /// Corresponds to the `write` and `write_iter` function pointers in `struct file_operations`. - fn write(&self, _file: &File, _data: &mut T, _offset: u64) -> Result { + fn write( + _this: &<::Borrowed as Deref>::Target, + _file: &File, + _data: &mut T, + _offset: u64, + ) -> Result { Err(Error::EINVAL) } /// Changes the position of the file. /// /// Corresponds to the `llseek` function pointer in `struct file_operations`. - fn seek(&self, _file: &File, _offset: SeekFrom) -> Result { + fn seek( + _this: &<::Borrowed as Deref>::Target, + _file: &File, + _offset: SeekFrom, + ) -> Result { 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: &mut IoctlCommand) -> Result { + fn ioctl( + _this: &<::Borrowed as Deref>::Target, + _file: &File, + _cmd: &mut IoctlCommand, + ) -> Result { 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: &mut IoctlCommand) -> Result { + fn compat_ioctl( + _this: &<::Borrowed as Deref>::Target, + _file: &File, + _cmd: &mut IoctlCommand, + ) -> Result { Err(Error::EINVAL) } /// Syncs pending changes to this file. /// /// Corresponds to the `fsync` function pointer in `struct file_operations`. - fn fsync(&self, _file: &File, _start: u64, _end: u64, _datasync: bool) -> Result { + fn fsync( + _this: &<::Borrowed as Deref>::Target, + _file: &File, + _start: u64, + _end: u64, + _datasync: bool, + ) -> Result { Err(Error::EINVAL) } @@ -592,7 +658,11 @@ pub trait FileOperations: Send + Sync + Sized { /// /// Corresponds to the `mmap` function pointer in `struct file_operations`. /// TODO: wrap `vm_area_struct` so that we don't have to expose it. - fn mmap(&self, _file: &File, _vma: &mut bindings::vm_area_struct) -> Result { + fn mmap( + _this: &<::Borrowed as Deref>::Target, + _file: &File, + _vma: &mut bindings::vm_area_struct, + ) -> Result { Err(Error::EINVAL) } @@ -600,7 +670,11 @@ pub trait FileOperations: Send + Sync + Sized { /// changes. /// /// Corresponds to the `poll` function pointer in `struct file_operations`. - fn poll(&self, _file: &File, _table: &PollTable) -> Result { + fn poll( + _this: &<::Borrowed as Deref>::Target, + _file: &File, + _table: &PollTable, + ) -> Result { Ok(bindings::POLLIN | bindings::POLLOUT | bindings::POLLRDNORM | bindings::POLLWRNORM) } } diff --git a/samples/rust/rust_miscdev.rs b/samples/rust/rust_miscdev.rs index 0f96e3bec23e5e..0103ea9746196e 100644 --- a/samples/rust/rust_miscdev.rs +++ b/samples/rust/rust_miscdev.rs @@ -55,35 +55,36 @@ impl SharedState { } } -struct Token { - shared: Pin>, -} +struct Token; impl FileOpener>> for Token { fn open(shared: &Pin>) -> Result { - Ok(Box::try_new(Self { - shared: shared.clone(), - })?) + Ok(shared.clone()) } } impl FileOperations for Token { - type Wrapper = Box; + type Wrapper = Pin>; kernel::declare_file_operations!(read, write); - fn read(&self, _: &File, data: &mut T, offset: u64) -> Result { + fn read( + shared: &SharedState, + _: &File, + data: &mut T, + offset: u64, + ) -> Result { // Succeed if the caller doesn't provide a buffer or if not at the start. if data.is_empty() || offset != 0 { return Ok(0); } { - let mut inner = self.shared.inner.lock(); + let mut inner = shared.inner.lock(); // Wait until we are allowed to decrement the token count or a signal arrives. while inner.token_count == 0 { - if self.shared.state_changed.wait(&mut inner) { + if shared.state_changed.wait(&mut inner) { return Err(Error::EINTR); } } @@ -93,20 +94,25 @@ impl FileOperations for Token { } // Notify a possible writer waiting. - self.shared.state_changed.notify_all(); + shared.state_changed.notify_all(); // Write a one-byte 1 to the reader. data.write_slice(&[1u8; 1])?; Ok(1) } - fn write(&self, _: &File, data: &mut T, _offset: u64) -> Result { + fn write( + shared: &SharedState, + _: &File, + data: &mut T, + _offs: u64, + ) -> Result { { - let mut inner = self.shared.inner.lock(); + let mut inner = shared.inner.lock(); // Wait until we are allowed to increment the token count or a signal arrives. while inner.token_count == MAX_TOKENS { - if self.shared.state_changed.wait(&mut inner) { + if shared.state_changed.wait(&mut inner) { return Err(Error::EINTR); } } @@ -116,7 +122,7 @@ impl FileOperations for Token { } // Notify a possible reader waiting. - self.shared.state_changed.notify_all(); + shared.state_changed.notify_all(); Ok(data.len()) } } diff --git a/samples/rust/rust_random.rs b/samples/rust/rust_random.rs index 2b42956073c6d8..135325b6ca0a4b 100644 --- a/samples/rust/rust_random.rs +++ b/samples/rust/rust_random.rs @@ -21,7 +21,7 @@ struct RandomFile; impl FileOperations for RandomFile { kernel::declare_file_operations!(read, write, read_iter, write_iter); - fn read(&self, file: &File, buf: &mut T, _offset: u64) -> Result { + fn read(_this: &Self, file: &File, buf: &mut T, _: u64) -> Result { let total_len = buf.len(); let mut chunkbuf = [0; 256]; @@ -39,7 +39,7 @@ impl FileOperations for RandomFile { Ok(total_len) } - fn write(&self, _file: &File, buf: &mut T, _offset: u64) -> Result { + fn write(_this: &Self, _file: &File, buf: &mut T, _: u64) -> Result { let total_len = buf.len(); let mut chunkbuf = [0; 256]; while !buf.is_empty() { diff --git a/samples/rust/rust_semaphore.rs b/samples/rust/rust_semaphore.rs index b3843381f7c968..7cd5c21f9ea5b6 100644 --- a/samples/rust/rust_semaphore.rs +++ b/samples/rust/rust_semaphore.rs @@ -80,35 +80,33 @@ impl FileOpener> for FileState { } impl FileOperations for FileState { - type Wrapper = Box; - declare_file_operations!(read, write, ioctl); - fn read(&self, _: &File, data: &mut T, offset: u64) -> Result { + fn read(this: &Self, _: &File, data: &mut T, offset: u64) -> Result { if data.is_empty() || offset > 0 { return Ok(0); } - self.consume()?; + this.consume()?; data.write_slice(&[0u8; 1])?; - self.read_count.fetch_add(1, Ordering::Relaxed); + this.read_count.fetch_add(1, Ordering::Relaxed); Ok(1) } - fn write(&self, _: &File, data: &mut T, _offset: u64) -> Result { + fn write(this: &Self, _: &File, data: &mut T, _offs: u64) -> Result { { - let mut inner = self.shared.inner.lock(); + let mut inner = this.shared.inner.lock(); inner.count = inner.count.saturating_add(data.len()); if inner.count > inner.max_seen { inner.max_seen = inner.count; } } - self.shared.changed.notify_all(); + this.shared.changed.notify_all(); Ok(data.len()) } - fn ioctl(&self, file: &File, cmd: &mut IoctlCommand) -> Result { - cmd.dispatch(self, file) + fn ioctl(this: &Self, file: &File, cmd: &mut IoctlCommand) -> Result { + cmd.dispatch::(this, file) } }