Skip to content

rust: add support for file system parameters #827

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 19, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions rust/helpers.c
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
#include <linux/skbuff.h>
#include <linux/uaccess.h>
#include <linux/uio.h>
#include <linux/fs_parser.h>

__noreturn void rust_helper_BUG(void)
{
Expand Down Expand Up @@ -645,6 +646,15 @@ void rust_helper_lockdep_unregister_key(struct lock_class_key *key)
}
EXPORT_SYMBOL_GPL(rust_helper_lockdep_unregister_key);

int rust_helper_fs_parse(struct fs_context *fc,
const struct fs_parameter_spec *desc,
struct fs_parameter *param,
struct fs_parse_result *result)
{
return fs_parse(fc, desc, param, result);
}
EXPORT_SYMBOL_GPL(rust_helper_fs_parse);

/*
* We use `bindgen`'s `--size_t-is-usize` option to bind the C `size_t` type
* as the Rust `usize` type, so we can use it in contexts where Rust
Expand Down
2 changes: 2 additions & 0 deletions rust/kernel/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -304,6 +304,8 @@ pub mod code {
declare_err!(ERESTARTSYS, "Restart the system call.");

declare_err!(ENOTSUPP, "Operation is not supported.");

declare_err!(ENOPARAM, "Parameter not supported.");
}

/// Generic integer kernel error.
Expand Down
142 changes: 139 additions & 3 deletions rust/kernel/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,13 @@
use crate::{
bindings, error::code::*, error::from_kernel_result, str::CStr, to_result,
types::PointerWrapper, AlwaysRefCounted, Result, ScopeGuard, ThisModule,
types::PointerWrapper, AlwaysRefCounted, Error, Result, ScopeGuard, ThisModule,
};
use core::{cell::UnsafeCell, marker::PhantomPinned, pin::Pin, ptr};
use macros::vtable;

pub mod param;

/// A file system context.
///
/// It is used to gather configuration to then mount or reconfigure a file system.
Expand All @@ -19,18 +21,48 @@ pub trait Context<T: Type + ?Sized> {
/// Type of the data associated with the context.
type Data: PointerWrapper + Send + Sync + 'static;

/// The typed file system parameters.
///
/// Users are encouraged to define it using the [`crate::define_fs_params`] macro.
const PARAMS: param::SpecTable<'static, Self::Data> = param::SpecTable::empty();

/// Creates a new context.
fn try_new() -> Result<Self::Data>;

/// Parses a parameter that wasn't specified in [`Self::PARAMS`].
fn parse_unknown_param(
_data: &mut Self::Data,
_name: &CStr,
_value: param::Value<'_>,
) -> Result {
Err(ENOPARAM)
}

/// Parses the whole parameter block, potentially skipping regular handling for parts of it.
///
/// The return value is the portion of the input buffer for which the regular handling
/// (involving [`Self::PARAMS`] and [`Self::parse_unknown_param`]) will still be carried out.
/// If it's `None`, the regular handling is not performed at all.
fn parse_monolithic<'a>(
_data: &mut Self::Data,
_buf: Option<&'a mut [u8]>,
) -> Result<Option<&'a mut [u8]>> {
Ok(None)
}
}

struct Tables<T: Type + ?Sized>(T);
impl<T: Type + ?Sized> Tables<T> {
const CONTEXT: bindings::fs_context_operations = bindings::fs_context_operations {
free: Some(Self::free_callback),
parse_param: None,
parse_param: Some(Self::parse_param_callback),
get_tree: Some(Self::get_tree_callback),
reconfigure: Some(Self::reconfigure_callback),
parse_monolithic: None,
parse_monolithic: if <T::Context as Context<T>>::HAS_PARSE_MONOLITHIC {
Some(Self::parse_monolithic_callback)
} else {
None
},
dup: None,
};

Expand All @@ -44,6 +76,55 @@ impl<T: Type + ?Sized> Tables<T> {
}
}

unsafe extern "C" fn parse_param_callback(

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't all 'unsafe' comments require a '//SAFETY' comment ( #351 ) ?
Although the following unsafe code it is indeed commented.

Copy link
Member

@ojeda ojeda Jul 17, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that this is an unsafe block, but an unsafe fn. For the former, you indeed need to describe why the operations inside the block are sound with a // SAFETY: comment. For the latter, we use /// # Safety documentation sections to describe the safety preconditions, if public.

It may be a good idea to start requiring documentation for private unsafe functions later on too, though.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As Miguel explained, we normally require /// # Safety blocks for these, which is slightly different: we're telling callers what they need to do to use it safely. I do write these sections even for private functions (see for example unnregister_keys in this very same file), but this case is a callback from C: there are no Rust callers, so no one is going to write a // SAFETY block based on this function.

None of the C callbacks currently have /// # Safety sections because of this.

fc: *mut bindings::fs_context,
param: *mut bindings::fs_parameter,
) -> core::ffi::c_int {
from_kernel_result! {
// SAFETY: The callback contract guarantees that `fc` is valid.
let ptr = unsafe { (*fc).fs_private };

// SAFETY: The value of `ptr` (coming from `fs_private` was initialised in
// `init_fs_context_callback` to the result of an `into_pointer` call. Since the
// context is valid, `from_pointer` wasn't called yet, so `ptr` is valid. Additionally,
// the callback contract guarantees that callbacks are serialised, so it is ok to
// mutably reference it.
let mut data =
unsafe { <<T::Context as Context<T>>::Data as PointerWrapper>::borrow_mut(ptr) };
let mut result = bindings::fs_parse_result::default();
// SAFETY: All parameters are valid at least for the duration of the call.
let opt =
unsafe { bindings::fs_parse(fc, T::Context::PARAMS.first, param, &mut result) };

// SAFETY: The callback contract guarantees that `param` is valid for the duration of
// the callback.
let param = unsafe { &*param };
if opt >= 0 {
let opt = opt as usize;
if opt >= T::Context::PARAMS.handlers.len() {
return Err(EINVAL);
}
T::Context::PARAMS.handlers[opt].handle_param(&mut data, param, &result)?;
return Ok(0);
}

if opt != ENOPARAM.to_kernel_errno() {
return Err(Error::from_kernel_errno(opt));
}

if !T::Context::HAS_PARSE_UNKNOWN_PARAM {
return Err(ENOPARAM);
}

let val = param::Value::from_fs_parameter(param);
// SAFETY: The callback contract guarantees the parameter key to be valid and last at
// least the duration of the callback.
T::Context::parse_unknown_param(
&mut data, unsafe { CStr::from_char_ptr(param.key) }, val)?;
Ok(0)
}
}

unsafe extern "C" fn fill_super_callback(
sb_ptr: *mut bindings::super_block,
_fc: *mut bindings::fs_context,
Expand All @@ -64,6 +145,8 @@ impl<T: Type + ?Sized> Tables<T> {
sb.s_time_gran = 1;

// Create and initialise the root inode.

// SAFETY: `sb` was just created initialised, so it is safe pass it to `new_inode`.
let inode = unsafe { bindings::new_inode(sb) };
if inode.is_null() {
return Err(ENOMEM);
Expand Down Expand Up @@ -117,6 +200,40 @@ impl<T: Type + ?Sized> Tables<T> {
EINVAL.to_kernel_errno()
}

unsafe extern "C" fn parse_monolithic_callback(
fc: *mut bindings::fs_context,
buf: *mut core::ffi::c_void,
) -> core::ffi::c_int {
from_kernel_result! {
// SAFETY: The callback contract guarantees that `fc` is valid.
let ptr = unsafe { (*fc).fs_private };

// SAFETY: The value of `ptr` (coming from `fs_private` was initialised in
// `init_fs_context_callback` to the result of an `into_pointer` call. Since the
// context is valid, `from_pointer` wasn't called yet, so `ptr` is valid. Additionally,
// the callback contract guarantees that callbacks are serialised, so it is ok to
// mutably reference it.
let mut data =
unsafe { <<T::Context as Context<T>>::Data as PointerWrapper>::borrow_mut(ptr) };
let page = if buf.is_null() {
None
} else {
// SAFETY: This callback is called to handle the `mount` syscall, which takes a
// page-sized buffer as data.
Some(unsafe { &mut *ptr::slice_from_raw_parts_mut(buf.cast(), crate::PAGE_SIZE) })
};
let regular = T::Context::parse_monolithic(&mut data, page)?;
if let Some(buf) = regular {
// SAFETY: Both `fc` and `buf` are guaranteed to be valid; the former because the
// callback is still ongoing and the latter because its lifefime is tied to that of
// `page`, which is also valid for the duration of the callback.
to_result(
unsafe { bindings::generic_parse_monolithic(fc, buf.as_mut_ptr().cast()) })?;
}
Ok(0)
}
}

const SUPER_BLOCK: bindings::super_operations = bindings::super_operations {
alloc_inode: None,
destroy_inode: None,
Expand Down Expand Up @@ -240,6 +357,7 @@ impl Registration {
fs.owner = module.0;
fs.name = T::NAME.as_char_ptr();
fs.fs_flags = T::FLAGS;
fs.parameters = T::Context::PARAMS.first;
fs.init_fs_context = Some(Self::init_fs_context_callback::<T>);
fs.kill_sb = Some(Self::kill_sb_callback::<T>);

Expand Down Expand Up @@ -372,3 +490,21 @@ unsafe impl AlwaysRefCounted for DEntry {
unsafe { bindings::dput(obj.cast().as_ptr()) }
}
}

/// Wraps the kernel's `struct filename`.
#[repr(transparent)]
pub struct Filename(pub(crate) UnsafeCell<bindings::filename>);

impl Filename {
/// Creates a reference to a [`Filename`] from a valid pointer.
///
/// # Safety
///
/// The caller must ensure that `ptr` is valid and remains valid for the lifetime of the
/// returned [`Filename`] instance.
pub(crate) unsafe fn from_ptr<'a>(ptr: *const bindings::filename) -> &'a Filename {
// SAFETY: The safety requirements guarantee the validity of the dereference, while the
// `Filename` type being transparent makes the cast ok.
unsafe { &*ptr.cast() }
}
}
Loading