From d3ab005b1fd71211d524e39beed50ee9120ad0a2 Mon Sep 17 00:00:00 2001 From: Wedson Almeida Filho Date: Tue, 27 Apr 2021 15:15:38 +0100 Subject: [PATCH] Introduce a new `io_buffer` module. It abstracts buffers involved in IO. This is in preparation for allowing drivers to implement `write_iter` and `read_iter`. No behaviour change is intended, this is a pure refactor. Signed-off-by: Wedson Almeida Filho --- drivers/android/defs.rs | 2 +- drivers/android/node.rs | 1 + drivers/android/process.rs | 1 + drivers/android/rust_binder.rs | 1 + drivers/android/thread.rs | 1 + drivers/android/transaction.rs | 5 +- rust/kernel/io_buffer.rs | 154 +++++++++++++++++++++++++++++++++ rust/kernel/lib.rs | 1 + rust/kernel/pages.rs | 9 +- rust/kernel/sysctl.rs | 11 +-- rust/kernel/user_ptr.rs | 145 +++---------------------------- samples/rust/rust_miscdev.rs | 1 + samples/rust/rust_random.rs | 1 + samples/rust/rust_semaphore.rs | 1 + 14 files changed, 191 insertions(+), 143 deletions(-) create mode 100644 rust/kernel/io_buffer.rs diff --git a/drivers/android/defs.rs b/drivers/android/defs.rs index 619bb1a234200f..54785d516a91fe 100644 --- a/drivers/android/defs.rs +++ b/drivers/android/defs.rs @@ -4,7 +4,7 @@ use core::ops::{Deref, DerefMut}; use kernel::{ bindings, bindings::*, - user_ptr::{ReadableFromBytes, WritableToBytes}, + io_buffer::{ReadableFromBytes, WritableToBytes}, }; macro_rules! pub_no_prefix { diff --git a/drivers/android/node.rs b/drivers/android/node.rs index 94c8adddbb17ad..7933212ac9fa22 100644 --- a/drivers/android/node.rs +++ b/drivers/android/node.rs @@ -6,6 +6,7 @@ use core::{ sync::atomic::{AtomicU64, Ordering}, }; use kernel::{ + io_buffer::IoBufferWriter, linked_list::{GetLinks, Links, List}, prelude::*, sync::{Guard, LockedBy, Mutex, Ref, SpinLock}, diff --git a/drivers/android/process.rs b/drivers/android/process.rs index 9d4d85646432cf..fe3f99bb5733c1 100644 --- a/drivers/android/process.rs +++ b/drivers/android/process.rs @@ -9,6 +9,7 @@ use core::{ use kernel::{ bindings, c_types, file_operations::{File, FileOpener, FileOperations, IoctlCommand, IoctlHandler, PollTable}, + io_buffer::{IoBufferReader, IoBufferWriter}, linked_list::List, pages::Pages, prelude::*, diff --git a/drivers/android/rust_binder.rs b/drivers/android/rust_binder.rs index aaef4517d48ffb..a5297651008cb5 100644 --- a/drivers/android/rust_binder.rs +++ b/drivers/android/rust_binder.rs @@ -11,6 +11,7 @@ use alloc::{boxed::Box, sync::Arc}; use core::pin::Pin; use kernel::{ cstr, + io_buffer::IoBufferWriter, linked_list::{GetLinks, GetLinksWrapped, Links}, miscdev::Registration, prelude::*, diff --git a/drivers/android/thread.rs b/drivers/android/thread.rs index f84fe9df988bc8..ef9e05304d6c91 100644 --- a/drivers/android/thread.rs +++ b/drivers/android/thread.rs @@ -5,6 +5,7 @@ use core::{alloc::AllocError, mem::size_of, pin::Pin}; use kernel::{ bindings, file_operations::{File, PollTable}, + io_buffer::{IoBufferReader, IoBufferWriter}, linked_list::{GetLinks, Links, List}, prelude::*, sync::{CondVar, Ref, SpinLock}, diff --git a/drivers/android/transaction.rs b/drivers/android/transaction.rs index 98c2c86e12f01a..e4489da37d1bdf 100644 --- a/drivers/android/transaction.rs +++ b/drivers/android/transaction.rs @@ -2,7 +2,10 @@ use alloc::sync::Arc; use core::sync::atomic::{AtomicBool, Ordering}; -use kernel::{bindings, linked_list::Links, prelude::*, sync::Ref, user_ptr::UserSlicePtrWriter}; +use kernel::{ + bindings, io_buffer::IoBufferWriter, linked_list::Links, prelude::*, sync::Ref, + user_ptr::UserSlicePtrWriter, +}; use crate::{ defs::*, diff --git a/rust/kernel/io_buffer.rs b/rust/kernel/io_buffer.rs new file mode 100644 index 00000000000000..c07d2b31ac8fa9 --- /dev/null +++ b/rust/kernel/io_buffer.rs @@ -0,0 +1,154 @@ +// SPDX-License-Identifier: GPL-2.0 + +//! Buffers used in IO. + +use crate::KernelResult; +use alloc::vec::Vec; +use core::mem::{size_of, MaybeUninit}; + +/// Represents a buffer to be read from during IO. +pub trait IoBufferReader { + /// Returns the number of bytes left to be read from the io buffer. + /// + /// Note that even reading less than this number of bytes may fail. + fn len(&self) -> usize; + + /// Returns `true` if no data is available in the io buffer. + fn is_empty(&self) -> bool { + self.len() == 0 + } + + /// Reads raw data from the io buffer into a raw kernel buffer. + /// + /// # Safety + /// + /// The output buffer must be valid. + unsafe fn read_raw(&mut self, out: *mut u8, len: usize) -> KernelResult; + + /// Reads all data remaining in the io buffer. + /// + /// Returns `EFAULT` if the address does not currently point to mapped, readable memory. + fn read_all(&mut self) -> KernelResult> { + let mut data = Vec::::new(); + data.try_reserve_exact(self.len())?; + data.resize(self.len(), 0); + + // SAFETY: The output buffer is valid as we just allocated it. + unsafe { self.read_raw(data.as_mut_ptr(), data.len())? }; + Ok(data) + } + + /// Reads a byte slice from the io buffer. + /// + /// Returns `EFAULT` if the byte slice is bigger than the remaining size of the user slice or + /// if the address does not currently point to mapped, readable memory. + fn read_slice(&mut self, data: &mut [u8]) -> KernelResult { + // SAFETY: The output buffer is valid as it's coming from a live reference. + unsafe { self.read_raw(data.as_mut_ptr(), data.len()) } + } + + /// Reads the contents of a plain old data (POD) type from the io buffer. + fn read(&mut self) -> KernelResult { + let mut out = MaybeUninit::::uninit(); + // SAFETY: The buffer is valid as it was just allocated. + unsafe { self.read_raw(out.as_mut_ptr() as _, size_of::()) }?; + // SAFETY: We just initialised the data. + Ok(unsafe { out.assume_init() }) + } +} + +/// Represents a buffer to be written to during IO. +pub trait IoBufferWriter { + /// Returns the number of bytes left to be written into the io buffer. + /// + /// Note that even writing less than this number of bytes may fail. + fn len(&self) -> usize; + + /// Returns `true` if the io buffer cannot hold any additional data. + fn is_empty(&self) -> bool { + self.len() == 0 + } + + /// Writes zeroes to the io buffer. + /// + /// Differently from the other write functions, `clear` will zero as much as it can and update + /// the writer internal state to reflect this. It will, however, return an error if it cannot + /// clear `len` bytes. + /// + /// For example, if a caller requests that 100 bytes be cleared but a segfault happens after + /// 20 bytes, then EFAULT is returned and the writer is advanced by 20 bytes. + fn clear(&mut self, len: usize) -> KernelResult; + + /// Writes a byte slice into the io buffer. + /// + /// Returns `EFAULT` if the byte slice is bigger than the remaining size of the io buffer or if + /// the address does not currently point to mapped, writable memory. + fn write_slice(&mut self, data: &[u8]) -> KernelResult { + // SAFETY: The input buffer is valid as it's coming from a live reference. + unsafe { self.write_raw(data.as_ptr(), data.len()) } + } + + /// Writes raw data to the io buffer from a raw kernel buffer. + /// + /// # Safety + /// + /// The input buffer must be valid. + unsafe fn write_raw(&mut self, data: *const u8, len: usize) -> KernelResult; + + /// Writes the contents of the given data into the io buffer. + fn write(&mut self, data: &T) -> KernelResult<()> { + // SAFETY: The input buffer is valid as it's coming from a live + // reference to a type that implements `WritableToBytes`. + unsafe { self.write_raw(data as *const T as _, size_of::()) } + } +} + +/// Specifies that a type is safely readable from byte slices. +/// +/// Not all types can be safely read from byte slices; examples from +/// include `bool` +/// that must be either `0` or `1`, and `char` that cannot be a surrogate or above `char::MAX`. +/// +/// # Safety +/// +/// Implementers must ensure that the type is made up only of types that can be safely read from +/// arbitrary byte sequences (e.g., `u32`, `u64`, etc.). +pub unsafe trait ReadableFromBytes {} + +// SAFETY: All bit patterns are acceptable values of the types below. +unsafe impl ReadableFromBytes for u8 {} +unsafe impl ReadableFromBytes for u16 {} +unsafe impl ReadableFromBytes for u32 {} +unsafe impl ReadableFromBytes for u64 {} +unsafe impl ReadableFromBytes for usize {} +unsafe impl ReadableFromBytes for i8 {} +unsafe impl ReadableFromBytes for i16 {} +unsafe impl ReadableFromBytes for i32 {} +unsafe impl ReadableFromBytes for i64 {} +unsafe impl ReadableFromBytes for isize {} + +/// Specifies that a type is safely writable to byte slices. +/// +/// This means that we don't read undefined values (which leads to UB) in preparation for writing +/// to the byte slice. It also ensures that no potentially sensitive information is leaked into the +/// byte slices. +/// +/// # Safety +/// +/// A type must not include padding bytes and must be fully initialised to safely implement +/// [`WritableToBytes`] (i.e., it doesn't contain [`MaybeUninit`] fields). A composition of +/// writable types in a structure is not necessarily writable because it may result in padding +/// bytes. +pub unsafe trait WritableToBytes {} + +// SAFETY: Initialised instances of the following types have no uninitialised portions. +unsafe impl WritableToBytes for u8 {} +unsafe impl WritableToBytes for u16 {} +unsafe impl WritableToBytes for u32 {} +unsafe impl WritableToBytes for u64 {} +unsafe impl WritableToBytes for usize {} +unsafe impl WritableToBytes for i8 {} +unsafe impl WritableToBytes for i16 {} +unsafe impl WritableToBytes for i32 {} +unsafe impl WritableToBytes for i64 {} +unsafe impl WritableToBytes for isize {} diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs index 2ad22dc1e6dece..96866504917cf4 100644 --- a/rust/kernel/lib.rs +++ b/rust/kernel/lib.rs @@ -59,6 +59,7 @@ pub mod sync; #[cfg(CONFIG_SYSCTL)] pub mod sysctl; +pub mod io_buffer; mod types; pub mod user_ptr; diff --git a/rust/kernel/pages.rs b/rust/kernel/pages.rs index 0426d411470da5..01c50a93c7ef6f 100644 --- a/rust/kernel/pages.rs +++ b/rust/kernel/pages.rs @@ -4,7 +4,10 @@ //! //! TODO: This module is a work in progress. -use crate::{bindings, c_types, user_ptr::UserSlicePtrReader, Error, KernelResult, PAGE_SIZE}; +use crate::{ + bindings, c_types, io_buffer::IoBufferReader, user_ptr::UserSlicePtrReader, Error, + KernelResult, PAGE_SIZE, +}; use core::{marker::PhantomData, ptr}; extern "C" { @@ -95,7 +98,7 @@ impl Pages { /// /// Callers must ensure that the destination buffer is valid for the given length. /// Additionally, if the raw buffer is intended to be recast, they must ensure that the data - /// can be safely cast; [`crate::user_ptr::ReadableFromBytes`] has more details about it. + /// can be safely cast; [`crate::io_buffer::ReadableFromBytes`] has more details about it. pub unsafe fn read(&self, dest: *mut u8, offset: usize, len: usize) -> KernelResult { // TODO: For now this only works on the first page. let end = offset.checked_add(len).ok_or(Error::EINVAL)?; @@ -114,7 +117,7 @@ impl Pages { /// /// Callers must ensure that the buffer is valid for the given length. Additionally, if the /// page is (or will be) mapped by userspace, they must ensure that no kernel data is leaked - /// through padding if it was cast from another type; [`crate::user_ptr::WritableToBytes`] has + /// through padding if it was cast from another type; [`crate::io_buffer::WritableToBytes`] has /// more details about it. pub unsafe fn write(&self, src: *const u8, offset: usize, len: usize) -> KernelResult { // TODO: For now this only works on the first page. diff --git a/rust/kernel/sysctl.rs b/rust/kernel/sysctl.rs index a1928a8523db80..19236c4ed57510 100644 --- a/rust/kernel/sysctl.rs +++ b/rust/kernel/sysctl.rs @@ -12,11 +12,12 @@ use core::mem; use core::ptr; use core::sync::atomic; -use crate::bindings; -use crate::c_types; -use crate::error; -use crate::types; -use crate::user_ptr::{UserSlicePtr, UserSlicePtrWriter}; +use crate::{ + bindings, c_types, error, + io_buffer::IoBufferWriter, + types, + user_ptr::{UserSlicePtr, UserSlicePtrWriter}, +}; /// Sysctl storage. pub trait SysctlStorage: Sync { diff --git a/rust/kernel/user_ptr.rs b/rust/kernel/user_ptr.rs index 041b3baa1cd3da..87985ce38b0eec 100644 --- a/rust/kernel/user_ptr.rs +++ b/rust/kernel/user_ptr.rs @@ -4,9 +4,13 @@ //! //! C header: [`include/linux/uaccess.h`](../../../../include/linux/uaccess.h) -use crate::{c_types, error::Error, KernelResult}; +use crate::{ + c_types, + error::Error, + io_buffer::{IoBufferReader, IoBufferWriter}, + KernelResult, +}; use alloc::vec::Vec; -use core::mem::{size_of, MaybeUninit}; extern "C" { fn rust_helper_copy_from_user( @@ -24,56 +28,6 @@ extern "C" { fn rust_helper_clear_user(to: *mut c_types::c_void, n: c_types::c_ulong) -> c_types::c_ulong; } -/// Specifies that a type is safely readable from byte slices. -/// -/// Not all types can be safely read from byte slices; examples from -/// include `bool` -/// that must be either `0` or `1`, and `char` that cannot be a surrogate or above `char::MAX`. -/// -/// # Safety -/// -/// Implementers must ensure that the type is made up only of types that can be safely read from -/// arbitrary byte sequences (e.g., `u32`, `u64`, etc.). -pub unsafe trait ReadableFromBytes {} - -// SAFETY: All bit patterns are acceptable values of the types below. -unsafe impl ReadableFromBytes for u8 {} -unsafe impl ReadableFromBytes for u16 {} -unsafe impl ReadableFromBytes for u32 {} -unsafe impl ReadableFromBytes for u64 {} -unsafe impl ReadableFromBytes for usize {} -unsafe impl ReadableFromBytes for i8 {} -unsafe impl ReadableFromBytes for i16 {} -unsafe impl ReadableFromBytes for i32 {} -unsafe impl ReadableFromBytes for i64 {} -unsafe impl ReadableFromBytes for isize {} - -/// Specifies that a type is safely writable to byte slices. -/// -/// This means that we don't read undefined values (which leads to UB) in preparation for writing -/// to the byte slice. It also ensures that no potentially sensitive information is leaked into the -/// byte slices. -/// -/// # Safety -/// -/// A type must not include padding bytes and must be fully initialised to safely implement -/// [`WritableToBytes`] (i.e., it doesn't contain [`MaybeUninit`] fields). A composition of -/// writable types in a structure is not necessarily writable because it may result in padding -/// bytes. -pub unsafe trait WritableToBytes {} - -// SAFETY: Initialised instances of the following types have no uninitialised portions. -unsafe impl WritableToBytes for u8 {} -unsafe impl WritableToBytes for u16 {} -unsafe impl WritableToBytes for u32 {} -unsafe impl WritableToBytes for u64 {} -unsafe impl WritableToBytes for usize {} -unsafe impl WritableToBytes for i8 {} -unsafe impl WritableToBytes for i16 {} -unsafe impl WritableToBytes for i32 {} -unsafe impl WritableToBytes for i64 {} -unsafe impl WritableToBytes for isize {} - /// A reference to an area in userspace memory, which can be either /// read-only or read-write. /// @@ -159,48 +113,20 @@ impl UserSlicePtr { /// Used to incrementally read from the user slice. pub struct UserSlicePtrReader(*mut c_types::c_void, usize); -impl UserSlicePtrReader { +impl IoBufferReader for UserSlicePtrReader { /// Returns the number of bytes left to be read from this. /// /// Note that even reading less than this number of bytes may fail. - pub fn len(&self) -> usize { + fn len(&self) -> usize { self.1 } - /// Returns `true` if `self.len()` is 0. - pub fn is_empty(&self) -> bool { - self.len() == 0 - } - - /// Reads all data remaining in the user slice. - /// - /// Returns `EFAULT` if the address does not currently point to - /// mapped, readable memory. - pub fn read_all(&mut self) -> KernelResult> { - let mut data = Vec::::new(); - data.try_reserve_exact(self.1)?; - data.resize(self.1, 0); - // SAFETY: The output buffer is valid as we just allocated it. - unsafe { self.read_raw(data.as_mut_ptr(), data.len())? }; - Ok(data) - } - - /// Reads a byte slice from the user slice. - /// - /// Returns `EFAULT` if the byte slice is bigger than the remaining size - /// of the user slice or if the address does not currently point to mapped, - /// readable memory. - pub fn read_slice(&mut self, data: &mut [u8]) -> KernelResult { - // SAFETY: The output buffer is valid as it's coming from a live reference. - unsafe { self.read_raw(data.as_mut_ptr(), data.len()) } - } - /// Reads raw data from the user slice into a raw kernel buffer. /// /// # Safety /// /// The output buffer must be valid. - pub unsafe fn read_raw(&mut self, out: *mut u8, len: usize) -> KernelResult { + unsafe fn read_raw(&mut self, out: *mut u8, len: usize) -> KernelResult { if len > self.1 || len > u32::MAX as usize { return Err(Error::EFAULT); } @@ -215,15 +141,6 @@ impl UserSlicePtrReader { self.1 -= len; Ok(()) } - - /// Reads the contents of a plain old data (POD) type from the user slice. - pub fn read(&mut self) -> KernelResult { - let mut out = MaybeUninit::::uninit(); - // SAFETY: The buffer is valid as it was just allocated. - unsafe { self.read_raw(out.as_mut_ptr() as _, size_of::()) }?; - // SAFETY: We just initialised the data. - Ok(unsafe { out.assume_init() }) - } } /// A writer for [`UserSlicePtr`]. @@ -231,28 +148,12 @@ impl UserSlicePtrReader { /// Used to incrementally write into the user slice. pub struct UserSlicePtrWriter(*mut c_types::c_void, usize); -impl UserSlicePtrWriter { - /// Returns the number of bytes left to be written from this. - /// - /// Note that even writing less than this number of bytes may fail. - pub fn len(&self) -> usize { +impl IoBufferWriter for UserSlicePtrWriter { + fn len(&self) -> usize { self.1 } - /// Returns `true` if `self.len()` is 0. - pub fn is_empty(&self) -> bool { - self.len() == 0 - } - - /// Writes zeroes to the user slice. - /// - /// Differently from the other write functions, `clear` will zero as much as it can and update - /// the writer internal state to reflect this. It will, however, return an error if it cannot - /// clear `len` bytes. - /// - /// For example, if a caller requests that 100 bytes be cleared but a segfault happens after - /// 20 bytes, then EFAULT is returned and the writer is advanced by 20 bytes. - pub fn clear(&mut self, mut len: usize) -> KernelResult { + fn clear(&mut self, mut len: usize) -> KernelResult { let mut ret = Ok(()); if len > self.1 { ret = Err(Error::EFAULT); @@ -272,21 +173,6 @@ impl UserSlicePtrWriter { ret } - /// Writes a byte slice to the user slice. - /// - /// Returns `EFAULT` if the byte slice is bigger than the remaining size - /// of the user slice or if the address does not currently point to mapped, - /// writable memory. - pub fn write_slice(&mut self, data: &[u8]) -> KernelResult { - // SAFETY: The input buffer is valid as it's coming from a live reference. - unsafe { self.write_raw(data.as_ptr(), data.len()) } - } - - /// Writes raw data to the user slice from a raw kernel buffer. - /// - /// # Safety - /// - /// The input buffer must be valid. unsafe fn write_raw(&mut self, data: *const u8, len: usize) -> KernelResult { if len > self.1 || len > u32::MAX as usize { return Err(Error::EFAULT); @@ -302,11 +188,4 @@ impl UserSlicePtrWriter { self.1 -= len; Ok(()) } - - /// Writes the contents of the given data into the user slice. - pub fn write(&mut self, data: &T) -> KernelResult<()> { - // SAFETY: The input buffer is valid as it's coming from a live - // reference to a type that implements `WritableToBytes`. - unsafe { self.write_raw(data as *const T as _, size_of::()) } - } } diff --git a/samples/rust/rust_miscdev.rs b/samples/rust/rust_miscdev.rs index 3d48f1e2fad3a2..01f5c6d0e4e553 100644 --- a/samples/rust/rust_miscdev.rs +++ b/samples/rust/rust_miscdev.rs @@ -11,6 +11,7 @@ use kernel::prelude::*; use kernel::{ cstr, file_operations::{File, FileOpener, FileOperations}, + io_buffer::{IoBufferReader, IoBufferWriter}, miscdev, sync::{CondVar, Mutex}, user_ptr::{UserSlicePtrReader, UserSlicePtrWriter}, diff --git a/samples/rust/rust_random.rs b/samples/rust/rust_random.rs index 9f17b94404661a..c4cc7d0f18027e 100644 --- a/samples/rust/rust_random.rs +++ b/samples/rust/rust_random.rs @@ -10,6 +10,7 @@ use kernel::{ file_operations::{File, FileOperations}, + io_buffer::{IoBufferReader, IoBufferWriter}, prelude::*, user_ptr::{UserSlicePtrReader, UserSlicePtrWriter}, }; diff --git a/samples/rust/rust_semaphore.rs b/samples/rust/rust_semaphore.rs index eb1dcc202e719d..556c3eadb984ec 100644 --- a/samples/rust/rust_semaphore.rs +++ b/samples/rust/rust_semaphore.rs @@ -24,6 +24,7 @@ use core::{ use kernel::{ condvar_init, cstr, declare_file_operations, file_operations::{File, FileOpener, FileOperations, IoctlCommand, IoctlHandler}, + io_buffer::{IoBufferReader, IoBufferWriter}, miscdev::Registration, mutex_init, prelude::*,