2023-07-26 17:14:01

by Ariel Miculas

[permalink] [raw]
Subject: [RFC PATCH v2 03/10] rust: kernel: add an abstraction over vfsmount to allow cloning a new private mount

Signed-off-by: Ariel Miculas <[email protected]>
---
rust/bindings/bindings_helper.h | 1 +
rust/kernel/lib.rs | 1 +
rust/kernel/mount.rs | 71 +++++++++++++++++++++++++++++++++
3 files changed, 73 insertions(+)
create mode 100644 rust/kernel/mount.rs

diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index e5fc3b1d408d..205ef50dfd4c 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -18,6 +18,7 @@
#include <linux/uio.h>
#include <linux/uaccess.h>
#include <linux/delay.h>
+#include <linux/namei.h>

/* `bindgen` gets confused at certain things. */
const gfp_t BINDINGS_GFP_KERNEL = GFP_KERNEL;
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 08f67833afcf..7dc07bdb5d55 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -46,6 +46,7 @@
pub mod ioctl;
pub mod iov_iter;
pub mod mm;
+pub mod mount;
pub mod pages;
pub mod prelude;
pub mod print;
diff --git a/rust/kernel/mount.rs b/rust/kernel/mount.rs
new file mode 100644
index 000000000000..d08830b27571
--- /dev/null
+++ b/rust/kernel/mount.rs
@@ -0,0 +1,71 @@
+//! Mount interface
+//!
+//! C headers: [`include/linux/mount.h`](../../../../include/linux/mount.h)
+
+use kernel::bindings;
+use kernel::error::from_err_ptr;
+use kernel::pr_err;
+use kernel::prelude::*;
+use kernel::str::CStr;
+use kernel::types::Opaque;
+
+/// Wraps the kernel's `struct path`.
+#[repr(transparent)]
+pub struct Path(pub(crate) Opaque<bindings::path>);
+
+/// Wraps the kernel's `struct vfsmount`.
+#[repr(transparent)]
+#[derive(Debug)]
+pub struct Vfsmount {
+ vfsmount: *mut bindings::vfsmount,
+}
+
+// SAFETY: No one besides us has the raw pointer, so we can safely transfer Vfsmount to another thread
+unsafe impl Send for Vfsmount {}
+// SAFETY: It's OK to access `Vfsmount` through references from other threads because we're not
+// accessing any properties from the underlying raw pointer
+unsafe impl Sync for Vfsmount {}
+
+impl Vfsmount {
+ /// Create a new private mount clone based on a path name
+ pub fn new_private_mount(path_name: &CStr) -> Result<Self> {
+ let path: Path = Path(Opaque::uninit());
+ // SAFETY: path_name is a &CStr, so it's a valid string pointer; path is an uninitialized
+ // struct stored on the stack and it's ok because kern_path expects an out parameter
+ let err = unsafe {
+ bindings::kern_path(
+ path_name.as_ptr() as *const i8,
+ bindings::LOOKUP_FOLLOW | bindings::LOOKUP_DIRECTORY,
+ path.0.get(),
+ )
+ };
+ if err != 0 {
+ pr_err!("failed to resolve '{}': {}\n", path_name, err);
+ return Err(EINVAL);
+ }
+
+ // SAFETY: path is a struct stored on the stack and it is initialized because the call to
+ // kern_path succeeded
+ let vfsmount = unsafe { from_err_ptr(bindings::clone_private_mount(path.0.get()))? };
+
+ // Don't inherit atime flags
+ // SAFETY: we called from_err_ptr so it's safe to dereference this pointer
+ unsafe {
+ (*vfsmount).mnt_flags &=
+ !(bindings::MNT_NOATIME | bindings::MNT_NODIRATIME | bindings::MNT_RELATIME) as i32;
+ }
+ Ok(Self { vfsmount })
+ }
+
+ /// Returns a raw pointer to vfsmount
+ pub fn get(&self) -> *mut bindings::vfsmount {
+ self.vfsmount
+ }
+}
+
+impl Drop for Vfsmount {
+ fn drop(&mut self) {
+ // SAFETY new_private_mount makes sure to return a valid pointer
+ unsafe { bindings::kern_unmount(self.vfsmount) };
+ }
+}
--
2.41.0



2023-07-27 00:04:06

by Trevor Gross

[permalink] [raw]
Subject: Re: [RFC PATCH v2 03/10] rust: kernel: add an abstraction over vfsmount to allow cloning a new private mount

On Wed, Jul 26, 2023 at 12:54 PM Ariel Miculas <[email protected]> wrote:
> [...]
> +++ b/rust/kernel/mount.rs
> @@ -0,0 +1,71 @@
> [...]
> +impl Vfsmount {
> + /// Create a new private mount clone based on a path name
> + pub fn new_private_mount(path_name: &CStr) -> Result<Self> {
> + let path: Path = Path(Opaque::uninit());
> + // SAFETY: path_name is a &CStr, so it's a valid string pointer; path is an uninitialized
> + // struct stored on the stack and it's ok because kern_path expects an out parameter
> + let err = unsafe {
> + bindings::kern_path(
> + path_name.as_ptr() as *const i8,
> + bindings::LOOKUP_FOLLOW | bindings::LOOKUP_DIRECTORY,
> + path.0.get(),
> + )
> + };

Nit: I believe that `.cast::<i8>()` is preferred over `as *const T`
because it makes it harder to
accidentally change constness or indirection level. This isn't always
followed in kernel, but
good practice.

> + if err != 0 {
> + pr_err!("failed to resolve '{}': {}\n", path_name, err);
> + return Err(EINVAL);
> + }
> +
> + // SAFETY: path is a struct stored on the stack and it is initialized because the call to
> + // kern_path succeeded
> + let vfsmount = unsafe { from_err_ptr(bindings::clone_private_mount(path.0.get()))? };
> +
> + // Don't inherit atime flags
> + // SAFETY: we called from_err_ptr so it's safe to dereference this pointer
> + unsafe {
> + (*vfsmount).mnt_flags &=
> + !(bindings::MNT_NOATIME | bindings::MNT_NODIRATIME | bindings::MNT_RELATIME) as i32;
> + }

Nit: it looks a bit cleaner to OR the flags outside of the unsafe
block, which is also nice because then
the unsafe block only contains the unsafe ops (could do this above
too). Also - I think maybe style
might encourage `// CAST:` where `as` is used?

let new_flags =
!(bindings::MNT_NOATIME | bindings::MNT_NODIRATIME |
bindings::MNT_RELATIME) as i32;

// SAFETY: we called from_err_ptr so it's safe to dereference this pointer
unsafe { (*vfsmount).mnt_flags &= new_flags }

> + Ok(Self { vfsmount })
> + }
> +
> + /// Returns a raw pointer to vfsmount
> + pub fn get(&self) -> *mut bindings::vfsmount {
> + self.vfsmount
> + }
> +}
> +
> +impl Drop for Vfsmount {
> + fn drop(&mut self) {
> + // SAFETY new_private_mount makes sure to return a valid pointer
> + unsafe { bindings::kern_unmount(self.vfsmount) };
> + }
> +}
> --
> 2.41.0
>
>