On 23/10/18 09:25AM, Wedson Almeida Filho wrote:
> From: Wedson Almeida Filho <walmeida@xxxxxxxxxxxxx>
>
> Allow Rust file systems to expose xattrs associated with inodes.
> `overlayfs` uses an xattr to indicate that a directory is opaque (i.e.,
> that lower layers should not be looked up). The planned file systems
> need to support opaque directories, so they must be able to implement
> this.
>
> Signed-off-by: Wedson Almeida Filho <walmeida@xxxxxxxxxxxxx>
> ---
> rust/bindings/bindings_helper.h | 1 +
> rust/kernel/error.rs | 2 ++
> rust/kernel/fs.rs | 43 +++++++++++++++++++++++++++++++++
> 3 files changed, 46 insertions(+)
>
> diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
> index 53a99ea512d1..fa754c5e85a2 100644
> --- a/rust/bindings/bindings_helper.h
> +++ b/rust/bindings/bindings_helper.h
> @@ -15,6 +15,7 @@
> #include <linux/refcount.h>
> #include <linux/wait.h>
> #include <linux/sched.h>
> +#include <linux/xattr.h>
>
> /* `bindgen` gets confused at certain things. */
> const size_t BINDINGS_ARCH_SLAB_MINALIGN = ARCH_SLAB_MINALIGN;
> diff --git a/rust/kernel/error.rs b/rust/kernel/error.rs
> index 484fa7c11de1..6c167583b275 100644
> --- a/rust/kernel/error.rs
> +++ b/rust/kernel/error.rs
> @@ -81,6 +81,8 @@ macro_rules! declare_err {
> declare_err!(EIOCBQUEUED, "iocb queued, will get completion event.");
> declare_err!(ERECALLCONFLICT, "Conflict with recalled state.");
> declare_err!(ENOGRACE, "NFS file lock reclaim refused.");
> + declare_err!(ENODATA, "No data available.");
> + declare_err!(EOPNOTSUPP, "Operation not supported on transport endpoint.");
> }
>
> /// Generic integer kernel error.
> diff --git a/rust/kernel/fs.rs b/rust/kernel/fs.rs
> index ee3dce87032b..adf9cbee16d2 100644
> --- a/rust/kernel/fs.rs
> +++ b/rust/kernel/fs.rs
> @@ -42,6 +42,14 @@ pub trait FileSystem {
>
> /// Reads the contents of the inode into the given folio.
> fn read_folio(inode: &INode<Self>, folio: LockedFolio<'_>) -> Result;
> +
> + /// Reads an xattr.
> + ///
> + /// Returns the number of bytes written to `outbuf`. If it is too small, returns the number of
> + /// bytes needs to hold the attribute.
> + fn read_xattr(_inode: &INode<Self>, _name: &CStr, _outbuf: &mut [u8]) -> Result<usize> {
> + Err(EOPNOTSUPP)
> + }
> }
>
> /// The types of directory entries reported by [`FileSystem::read_dir`].
> @@ -418,6 +426,7 @@ impl<T: FileSystem + ?Sized> Tables<T> {
>
> sb.0.s_magic = params.magic as _;
> sb.0.s_op = &Tables::<T>::SUPER_BLOCK;
> + sb.0.s_xattr = &Tables::<T>::XATTR_HANDLERS[0];
> sb.0.s_maxbytes = params.maxbytes;
> sb.0.s_time_gran = params.time_gran;
> sb.0.s_blocksize_bits = params.blocksize_bits;
> @@ -487,6 +496,40 @@ impl<T: FileSystem + ?Sized> Tables<T> {
> shutdown: None,
> };
>
> + const XATTR_HANDLERS: [*const bindings::xattr_handler; 2] = [&Self::XATTR_HANDLER, ptr::null()];
> +
> + const XATTR_HANDLER: bindings::xattr_handler = bindings::xattr_handler {
> + name: ptr::null(),
> + prefix: crate::c_str!("").as_char_ptr(),
> + flags: 0,
> + list: None,
> + get: Some(Self::xattr_get_callback),
> + set: None,
> + };
> +
> + unsafe extern "C" fn xattr_get_callback(
> + _handler: *const bindings::xattr_handler,
> + _dentry: *mut bindings::dentry,
> + inode_ptr: *mut bindings::inode,
> + name: *const core::ffi::c_char,
> + buffer: *mut core::ffi::c_void,
> + size: usize,
> + ) -> core::ffi::c_int {
> + from_result(|| {
> + // SAFETY: The C API guarantees that `inode_ptr` is a valid inode.
> + let inode = unsafe { &*inode_ptr.cast::<INode<T>>() };
> +
> + // SAFETY: The c API guarantees that `name` is a valid null-terminated string. It
> + // also guarantees that it's valid for the duration of the callback.
> + let name = unsafe { CStr::from_char_ptr(name) };
> +
> + // SAFETY: The C API guarantees that `buffer` is at least `size` bytes in length.
> + let buf = unsafe { core::slice::from_raw_parts_mut(buffer.cast(), size) };
I think this is not safe. from_raw_parts_mut's documentation says:
```
`data` must be non-null and aligned even for zero-length slices. One
reason for this is that enum layout optimizations may rely on references
(including slices of any length) being aligned and non-null to distinguish
them from other data. You can obtain a pointer that is usable as `data`
for zero-length slices using [`NonNull::dangling()`].
```
`vfs_getxattr_alloc` explicitly calls the `get` handler with `buffer` set
to NULL and `size` set to 0, in order to determine the required size for
the extended attributes:
```
error = handler->get(handler, dentry, inode, name, NULL, 0);
if (error < 0)
return error;
```
So `buffer` is definitely NULL in the first call to the handler.
When `buffer` is NULL, the first argument to `from_raw_parts_mut` should
be `NonNull::dangling()`.
> + let len = T::read_xattr(inode, name, buf)?;
> + Ok(len.try_into()?)
> + })
> + }
> +
> const DIR_FILE_OPERATIONS: bindings::file_operations = bindings::file_operations {
> owner: ptr::null_mut(),
> llseek: Some(bindings::generic_file_llseek),
> --
> 2.34.1
>
