From 52ffab963a4cf5592691ff2f65fbce50376e0bea Mon Sep 17 00:00:00 2001 From: Alice Ryhl Date: Fri, 13 Sep 2024 13:33:45 +0000 Subject: [PATCH] ANDROID: ashmem_rust: add mmap ioctl The new `shmem.rs` file is a Rust abstraction around shmem files. It's important that we modify the fops to prevent users from calling mmap again, as they could otherwise modify the protection bits on the memory region in a way that bypasses the ashmem protection mask. Bug: 370906207 Change-Id: I0d2f4b56ac005d131a6ef0f9c7b346549effd1d7 Signed-off-by: Alice Ryhl --- drivers/staging/android/ashmem_rust.rs | 96 ++++++++++++- drivers/staging/android/shmem.rs | 178 +++++++++++++++++++++++++ rust/bindings/bindings_helper.h | 5 + rust/helpers/helpers.c | 1 + rust/helpers/mman.c | 9 ++ rust/kernel/sync.rs | 3 +- 6 files changed, 289 insertions(+), 3 deletions(-) create mode 100644 drivers/staging/android/shmem.rs create mode 100644 rust/helpers/mman.c diff --git a/drivers/staging/android/ashmem_rust.rs b/drivers/staging/android/ashmem_rust.rs index 7efa00b5d36c..d25e2a33301d 100644 --- a/drivers/staging/android/ashmem_rust.rs +++ b/drivers/staging/android/ashmem_rust.rs @@ -17,6 +17,8 @@ use kernel::{ fs::File, ioctl::_IOC_SIZE, miscdevice::{MiscDevice, MiscDeviceOptions, MiscDeviceRegistration}, + mm::virt::{flags as vma_flags, VmAreaNew}, + page::page_align, prelude::*, sync::{new_mutex, Mutex}, task::Task, @@ -24,12 +26,18 @@ use kernel::{ }; const ASHMEM_NAME_LEN: usize = bindings::ASHMEM_NAME_LEN as usize; +const ASHMEM_FULL_NAME_LEN: usize = bindings::ASHMEM_FULL_NAME_LEN as usize; +const ASHMEM_NAME_PREFIX_LEN: usize = bindings::ASHMEM_NAME_PREFIX_LEN as usize; +const ASHMEM_NAME_PREFIX: [u8; ASHMEM_NAME_PREFIX_LEN] = *b"dev/ashmem/"; const PROT_READ: usize = bindings::PROT_READ as usize; const PROT_EXEC: usize = bindings::PROT_EXEC as usize; const PROT_WRITE: usize = bindings::PROT_WRITE as usize; const PROT_MASK: usize = PROT_EXEC | PROT_READ | PROT_WRITE; +mod shmem; +use shmem::ShmemFile; + /// Does PROT_READ imply PROT_EXEC for this task? fn read_implies_exec(task: &Task) -> bool { // SAFETY: Always safe to read. @@ -51,6 +59,9 @@ struct AshmemModule { impl kernel::Module for AshmemModule { fn init(_module: &'static kernel::ThisModule) -> Result { + // SAFETY: Called once since this is the module initializer. + unsafe { shmem::SHMEM_FOPS_ONCE.init() }; + pr_info!("Using Rust implementation."); Ok(Self { @@ -76,6 +87,7 @@ struct AshmemInner { prot_mask: usize, /// If set, then this holds the ashmem name without the dev/ashmem/ prefix. No zero terminator. name: Option>, + file: Option, } #[vtable] @@ -90,6 +102,7 @@ impl MiscDevice for Ashmem { size: 0, prot_mask: PROT_MASK, name: None, + file: None, }), } }, @@ -97,6 +110,51 @@ impl MiscDevice for Ashmem { ) } + fn mmap(me: Pin<&Ashmem>, _file: &File, vma: &VmAreaNew) -> Result<()> { + let asma = &mut *me.inner.lock(); + + // User needs to SET_SIZE before mapping. + if asma.size == 0 { + return Err(EINVAL); + } + + // Requested mapping size larger than object size. + if vma.end() - vma.start() > page_align(asma.size) { + return Err(EINVAL); + } + + if asma.prot_mask & PROT_WRITE == 0 { + vma.try_clear_maywrite().map_err(|_| EPERM)?; + } + if asma.prot_mask & PROT_EXEC == 0 { + vma.try_clear_mayexec().map_err(|_| EPERM)?; + } + if asma.prot_mask & PROT_READ == 0 { + vma.try_clear_mayread().map_err(|_| EPERM)?; + } + + let file = match asma.file.as_ref() { + Some(file) => file, + None => { + let mut name_buffer = [0u8; ASHMEM_FULL_NAME_LEN]; + let name = asma.full_name(&mut name_buffer); + asma.file + .insert(ShmemFile::new(name, asma.size, vma.flags())?) + } + }; + + if vma.flags() & vma_flags::SHARED != 0 { + // We're really using this just to set vm_ops to `shmem_anon_vm_ops`. Anything else it + // does is undone by the call to `set_file` below. + shmem::zero_setup(vma)?; + } else { + shmem::vma_set_anonymous(vma); + } + + shmem::set_file(vma, file.file()); + Ok(()) + } + fn ioctl(me: Pin<&Ashmem>, _file: &File, cmd: u32, arg: usize) -> Result { let size = _IOC_SIZE(cmd); match cmd { @@ -138,7 +196,9 @@ impl Ashmem { v.extend_from_slice(&local_name[..len], GFP_KERNEL)?; let mut asma = self.inner.lock(); - // TODO: fail if `mmap` is already called + if asma.file.is_some() { + return Err(EINVAL); + } asma.name = Some(v); Ok(0) } @@ -164,7 +224,9 @@ impl Ashmem { fn set_size(&self, size: usize) -> Result { let mut asma = self.inner.lock(); - // TODO: fail if `mmap` is already called + if asma.file.is_some() { + return Err(EINVAL); + } asma.size = size; Ok(0) } @@ -193,3 +255,33 @@ impl Ashmem { Ok(self.inner.lock().prot_mask as isize) } } + +impl AshmemInner { + /// Get the full name. + /// + /// If the name is `Some(name)`, then this returns `dev/ashmem/name\0`. + /// + /// If the name is `None`, then this returns `dev/ashmem\0`. + fn full_name<'name>(&self, name: &'name mut [u8; ASHMEM_FULL_NAME_LEN]) -> &'name CStr { + name[..ASHMEM_NAME_PREFIX_LEN].copy_from_slice(&ASHMEM_NAME_PREFIX); + if let Some(set_name) = self.name.as_deref() { + name[ASHMEM_NAME_PREFIX_LEN..][..set_name.len()].copy_from_slice(set_name); + } else { + // Remove last slash if no name set. + name[ASHMEM_NAME_PREFIX_LEN - 1] = 0; + } + name[ASHMEM_FULL_NAME_LEN - 1] = 0; + + // This unwrap only fails if there's no nul-byte, but we just added one at the end above. + let len_with_nul = name + .iter() + .position(|&c| c == 0) + .map(|len| len + 1) + .unwrap(); + + // This unwrap fails if the last byte is not a nul-byte, or if there are any nul-bytes + // before the last byte. Neither of those are possible here since `len_with_nul` is the + // index of the first nul-byte in `name`. + CStr::from_bytes_with_nul(&name[..len_with_nul]).unwrap() + } +} diff --git a/drivers/staging/android/shmem.rs b/drivers/staging/android/shmem.rs new file mode 100644 index 000000000000..1fe15e390093 --- /dev/null +++ b/drivers/staging/android/shmem.rs @@ -0,0 +1,178 @@ +// SPDX-License-Identifier: GPL-2.0 + +// Copyright (C) 2024 Google LLC. + +//! Safe rust abstraction around a shmem file for use by ashmem. + +use kernel::{ + bindings, + error::{from_err_ptr, to_result, Result}, + fs::file::File, + mm::virt::{vm_flags_t, VmAreaNew}, + prelude::*, + str::CStr, + types::ARef, +}; + +use core::{ + cell::UnsafeCell, + ffi::{c_int, c_ulong}, + ptr::{addr_of_mut, NonNull}, +}; + +pub(crate) fn vma_set_anonymous(vma: &VmAreaNew) { + // SAFETY: The `VmAreaNew` type is only used when the vma is being set up, so this operation is + // safe. + unsafe { (*vma.as_ptr()).vm_ops = core::ptr::null_mut() }; +} + +/// Wrapper around a file that is known to be a shmem file. +#[derive(Clone)] +pub(crate) struct ShmemFile { + inner: ARef, +} + +impl ShmemFile { + /// Create a shmem file for use by ashmem. + /// + /// This sets up the file with the exact configuration that ashmem needs. + pub(crate) fn new(name: &CStr, size: usize, flags: vm_flags_t) -> Result { + // SAFETY: The name is a nul-terminated string. + let vmfile = from_err_ptr(unsafe { + bindings::shmem_file_setup(name.as_char_ptr(), size as _, flags) + })?; + + // SAFETY: The call to `shmem_file_setup` was successful, so `vmfile` is a valid pointer to + // a file and we can transfer ownership of the refcount it created to an `ARef`. + let vmfile = unsafe { ARef::::from_raw(NonNull::new_unchecked(vmfile.cast())) }; + + // The C driver sets the FMODE_LSEEK bit in `f_mode` here. However, that is not necessary + // anymore. It was added to the C driver in commit 97fbfef6bd59 ("staging: android: ashmem: + // lseek failed due to no FMODE_LSEEK.") since they started using the VFS implementation of + // lseek rather than a custom hook, and the VFS version actually checks the permissions. + // + // However, commit e7478158e137 ("fs: clear or set FMODE_LSEEK based on llseek function") + // has since made it so that if lseek is implemented, then FMODE_LSEEK will be set on + // pseudo-files by default. Since llseek is implemented on shmem files, we no longer need + // to set FMODE_LSEEK. + + set_inode_lockdep_class(&vmfile); + + // SAFETY: We just created the file and have not yet published it, so nobody else is + // looking at this field yet. + unsafe { (*vmfile.as_ptr()).f_op = get_shmem_fops((*vmfile.as_ptr()).f_op) }; + + Ok(Self { inner: vmfile }) + } + + pub(crate) fn file(&self) -> &File { + &self.inner + } +} + +/// Fix the lockdep class of the shmem inode. +/// +/// A separate lockdep class for the backing shmem inodes to resolve the lockdep warning about the +/// race between kswapd taking fs_reclaim before inode_lock and write syscall taking inode_lock and +/// then fs_reclaim. Note that such race is impossible because ashmem does not support write +/// syscalls operating on the backing shmem. +fn set_inode_lockdep_class(vmfile: &File) { + // SAFETY: This sets the lockdep class correctly. + unsafe { + let inode = (*vmfile.as_ptr()).f_inode; + let lock = addr_of_mut!((*inode).i_rwsem); + bindings::lockdep_set_class_rwsem( + lock, + kernel::static_lock_class!().as_ptr(), + kernel::c_str!("backing_shmem_inode_class").as_char_ptr(), + ) + } +} + +pub(crate) fn zero_setup(vma: &VmAreaNew) -> Result<()> { + // SAFETY: The `VmAreaNew` type is only used when the vma is being set up, so we can set up the + // vma. + to_result(unsafe { bindings::shmem_zero_setup(vma.as_ptr()) }) +} + +pub(crate) fn set_file(vma: &VmAreaNew, file: &File) { + let file = ARef::from(file); + // SAFETY: We're setting up the vma, so we can read the file pointer. + let old_file = unsafe { (*vma.as_ptr()).vm_file }; + + // INVARIANT: This transfers ownership of the refcount we just created to the vma. + // + // SAFETY: We're setting up the vma, so we can write to the file pointer. + unsafe { (*vma.as_ptr()).vm_file = ARef::into_raw(file).as_ptr().cast() }; + + if let Some(old_file) = NonNull::new(old_file) { + // SAFETY: We took ownership of the file refcount from the vma, so we can drop it. + drop(unsafe { ARef::::from_raw(old_file.cast()) }); + } +} + +// Used to synchronize the initialization of `VMFILE_FOPS`. +// +// INVARIANT: Once `SHMEM_FOPS_ONCE` becomes true, `VMFILE_FOPS` is permanently immutable. +kernel::sync::global_lock! { + // SAFETY: We call `init` as the very first thing in the initialization of this module, so + // there are no calls to `lock` before `init` is called. + pub(super) unsafe(uninit) static SHMEM_FOPS_ONCE: Mutex = false; +} + +/// # Safety +/// +/// Must only be used with the fops of a shmem file. +unsafe fn get_shmem_fops( + shmem_fops: *const bindings::file_operations, +) -> &'static bindings::file_operations { + struct FopsHelper { + inner: UnsafeCell, + } + unsafe impl Sync for FopsHelper {} + + static VMFILE_FOPS: FopsHelper = FopsHelper { + // SAFETY: All zeros is valid for `struct file_operations`. + inner: UnsafeCell::new(unsafe { core::mem::zeroed() }), + }; + + let fops_ptr = VMFILE_FOPS.inner.get(); + + let mut once_guard = SHMEM_FOPS_ONCE.lock(); + if !*once_guard { + // SAFETY: This points at the file operations of an existing file, so the contents must be + // immutable. + let mut new_fops = unsafe { *shmem_fops }; + new_fops.mmap = Some(ashmem_vmfile_mmap); + new_fops.get_unmapped_area = Some(ashmem_vmfile_get_unmapped_area); + // SAFETY: We hold the `SHMEM_FOPS_ONCE` guard, so there are no other writers. The value of + // `SHMEM_FOPS_ONCE` is false, so there are no readers either. + unsafe { *fops_ptr = new_fops }; + *once_guard = true; + } + drop(once_guard); + + // SAFETY: The value of `SHMEM_FOPS_ONCE` is true, so `VMFILE_FOPS` is never going to change + // again. + unsafe { &*fops_ptr } +} + +extern "C" fn ashmem_vmfile_mmap( + _file: *mut bindings::file, + _vma: *mut bindings::vm_area_struct, +) -> c_int { + EPERM.to_errno() +} + +unsafe extern "C" fn ashmem_vmfile_get_unmapped_area( + file: *mut bindings::file, + addr: c_ulong, + len: c_ulong, + pgoff: c_ulong, + flags: c_ulong, +) -> c_ulong { + // SAFETY: The `mm` of current does not change, so it is safe to access. + let mm = unsafe { (*bindings::get_current()).mm }; + // SAFETY: This calls the right get_unmapped_area for a shmem. + unsafe { bindings::mm_get_unmapped_area(mm, file, addr, len, pgoff, flags) } +} diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h index e0084b22fa26..180bafe4de93 100644 --- a/rust/bindings/bindings_helper.h +++ b/rust/bindings/bindings_helper.h @@ -29,12 +29,15 @@ #include #include #include +#include #include #include #include #include #include #include +#include +#include /* `bindgen` gets confused at certain things. */ const size_t RUST_CONST_HELPER_ARCH_SLAB_MINALIGN = ARCH_SLAB_MINALIGN; @@ -49,4 +52,6 @@ const blk_features_t RUST_CONST_HELPER_BLK_FEAT_ROTATIONAL = BLK_FEAT_ROTATIONAL #ifdef CONFIG_ASHMEM_RUST #include "../../drivers/staging/android/ashmem.h" +const size_t RUST_CONST_HELPER_ASHMEM_NAME_PREFIX_LEN = ASHMEM_NAME_PREFIX_LEN; +const size_t RUST_CONST_HELPER_ASHMEM_FULL_NAME_LEN = ASHMEM_FULL_NAME_LEN; #endif diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c index c4e5c7d78cf7..2d03734b5b5c 100644 --- a/rust/helpers/helpers.c +++ b/rust/helpers/helpers.c @@ -18,6 +18,7 @@ #include "jump_label.c" #include "kunit.c" #include "mm.c" +#include "mman.c" #include "mutex.c" #include "page.c" #include "rbtree.c" diff --git a/rust/helpers/mman.c b/rust/helpers/mman.c new file mode 100644 index 000000000000..1ebcf420e46c --- /dev/null +++ b/rust/helpers/mman.c @@ -0,0 +1,9 @@ +// SPDX-License-Identifier: GPL-2.0 + +#include + +void rust_helper_lockdep_set_class_rwsem(struct rw_semaphore *lock, struct lock_class_key *key, + const char *name) +{ + lockdep_set_class_and_name(lock, key, name); +} diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs index 1eab7ebf25fd..e7817849895b 100644 --- a/rust/kernel/sync.rs +++ b/rust/kernel/sync.rs @@ -34,7 +34,8 @@ impl LockClassKey { Self(Opaque::uninit()) } - pub(crate) fn as_ptr(&self) -> *mut bindings::lock_class_key { + /// Returns a raw pointer to the lock class key. + pub fn as_ptr(&self) -> *mut bindings::lock_class_key { self.0.get() } }