Received-SPF: pass (google.com: domain of linux-kernel+bounces-178739-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
From: Wedson Almeida Filho <wedsonaf@gmail.com>
To: Alexander Viro <viro@zeniv.linux.org.uk>,
	Christian Brauner <brauner@kernel.org>,
	Matthew Wilcox <willy@infradead.org>,
	Dave Chinner <david@fromorbit.com>
Cc: Kent Overstreet <kent.overstreet@gmail.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	linux-fsdevel@vger.kernel.org,
	rust-for-linux@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	Wedson Almeida Filho <walmeida@microsoft.com>
Subject: [RFC PATCH v2 26/30] rust: fs: allow populating i_lnk
Date: Tue, 14 May 2024 10:17:07 -0300
Message-Id: <20240514131711.379322-27-wedsonaf@gmail.com>
In-Reply-To: <20240514131711.379322-1-wedsonaf@gmail.com>
References: <20240514131711.379322-1-wedsonaf@gmail.com>
Precedence: bulk
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit

From: Wedson Almeida Filho <walmeida@microsoft.com>

Allow Rust file systems to store a string that represents the
destination of a symbolic link inode in the inode itself.

Signed-off-by: Wedson Almeida Filho <walmeida@microsoft.com>
---
 rust/kernel/fs/file.rs    |  6 ++---
 rust/kernel/fs/inode.rs   | 32 +++++++++++++++++++++----
 samples/rust/rust_rofs.rs | 50 ++++++++++++---------------------------
 3 files changed, 45 insertions(+), 43 deletions(-)

diff --git a/rust/kernel/fs/file.rs b/rust/kernel/fs/file.rs
index a819724b75f8..9db70eff1169 100644
--- a/rust/kernel/fs/file.rs
+++ b/rust/kernel/fs/file.rs
@@ -518,15 +518,15 @@ pub enum DirEntryType {
     Wht = bindings::DT_WHT,
 }
 
-impl From<inode::Type> for DirEntryType {
-    fn from(value: inode::Type) -> Self {
+impl From<&inode::Type> for DirEntryType {
+    fn from(value: &inode::Type) -> Self {
         match value {
             inode::Type::Fifo => DirEntryType::Fifo,
             inode::Type::Chr(_, _) => DirEntryType::Chr,
             inode::Type::Dir => DirEntryType::Dir,
             inode::Type::Blk(_, _) => DirEntryType::Blk,
             inode::Type::Reg => DirEntryType::Reg,
-            inode::Type::Lnk => DirEntryType::Lnk,
+            inode::Type::Lnk(_) => DirEntryType::Lnk,
             inode::Type::Sock => DirEntryType::Sock,
         }
     }
diff --git a/rust/kernel/fs/inode.rs b/rust/kernel/fs/inode.rs
index 5230ff2fe0dd..b2b7d000080e 100644
--- a/rust/kernel/fs/inode.rs
+++ b/rust/kernel/fs/inode.rs
@@ -7,8 +7,8 @@
 //! C headers: [`include/linux/fs.h`](srctree/include/linux/fs.h)
 
 use super::{
-    address_space, dentry, dentry::DEntry, file, sb::SuperBlock, FileSystem, Offset, PageOffset,
-    UnspecifiedFS,
+    address_space, dentry, dentry::DEntry, file, mode, sb::SuperBlock, FileSystem, Offset,
+    PageOffset, UnspecifiedFS,
 };
 use crate::error::{code::*, from_err_ptr, Result};
 use crate::types::{ARef, AlwaysRefCounted, Either, ForeignOwnable, Lockable, Locked, Opaque};
@@ -255,6 +255,17 @@ pub(crate) fn new_cache() -> Result<Option<MemCache>> {
         let ptr = container_of!(inode, WithData<T::INodeData>, inode).cast_mut();
 
         if !is_bad {
+            // SAFETY: The API contract guarantees that `inode` is valid.
+            if unsafe { (*inode).i_mode & mode::S_IFMT == mode::S_IFLNK } {
+                // SAFETY: We just checked that the inode is a link.
+                let lnk = unsafe { (*inode).__bindgen_anon_4.i_link };
+                if !lnk.is_null() {
+                    // SAFETY: This value is on link inode are only populated from with the result
+                    // of `CString::into_foreign`.
+                    unsafe { CString::from_foreign(lnk.cast::<core::ffi::c_void>()) };
+                }
+            }
+
             // SAFETY: The code either initialises the data or marks the inode as bad. Since the
             // inode is not bad, the data is initialised, and thus safe to drop.
             unsafe { ptr::drop_in_place((*ptr).data.as_mut_ptr()) };
@@ -381,7 +392,7 @@ pub fn init(self, params: Params<T::INodeData>) -> Result<ARef<INode<T>>> {
                 unsafe { bindings::mapping_set_large_folios(inode.i_mapping) };
                 bindings::S_IFREG
             }
-            Type::Lnk => {
+            Type::Lnk(str) => {
                 // If we are using `page_get_link`, we need to prevent the use of high mem.
                 if !inode.i_op.is_null() {
                     // SAFETY: We just checked that `i_op` is non-null, and we always just set it
@@ -393,6 +404,9 @@ pub fn init(self, params: Params<T::INodeData>) -> Result<ARef<INode<T>>> {
                         unsafe { bindings::inode_nohighmem(inode) };
                     }
                 }
+                if let Some(s) = str {
+                    inode.__bindgen_anon_4.i_link = s.into_foreign().cast::<i8>().cast_mut();
+                }
                 bindings::S_IFLNK
             }
             Type::Fifo => {
@@ -485,7 +499,6 @@ fn drop(&mut self) {
 }
 
 /// The type of an inode.
-#[derive(Copy, Clone)]
 pub enum Type {
     /// Named pipe (first-in, first-out) type.
     Fifo,
@@ -503,7 +516,7 @@ pub enum Type {
     Reg,
 
     /// Symbolic link type.
-    Lnk,
+    Lnk(Option<CString>),
 
     /// Named unix-domain socket type.
     Sock,
@@ -565,6 +578,15 @@ pub fn page_symlink_inode() -> Self {
         )
     }
 
+    /// Returns inode operations for symbolic links that are stored in the `i_lnk` field.
+    pub fn simple_symlink_inode() -> Self {
+        // SAFETY: This is a constant in C, it never changes.
+        Self(
+            unsafe { &bindings::simple_symlink_inode_operations },
+            PhantomData,
+        )
+    }
+
     /// Creates the inode operations from a type that implements the [`Operations`] trait.
     pub const fn new<U: Operations<FileSystem = T> + ?Sized>() -> Self {
         struct Table<T: Operations + ?Sized>(PhantomData<T>);
diff --git a/samples/rust/rust_rofs.rs b/samples/rust/rust_rofs.rs
index 5b6c3f50adf4..5d0d1936459d 100644
--- a/samples/rust/rust_rofs.rs
+++ b/samples/rust/rust_rofs.rs
@@ -7,7 +7,7 @@
 };
 use kernel::prelude::*;
 use kernel::types::{ARef, Either, Locked};
-use kernel::{c_str, folio::Folio, folio::PageCache, fs, str::CString, time::UNIX_EPOCH, user};
+use kernel::{c_str, folio::Folio, folio::PageCache, fs, time::UNIX_EPOCH, user};
 
 kernel::module_fs! {
     type: RoFs,
@@ -46,7 +46,7 @@ struct Entry {
     Entry {
         name: b"link.txt",
         ino: 3,
-        etype: inode::Type::Lnk,
+        etype: inode::Type::Lnk(None),
         contents: b"./test.txt",
     },
 ];
@@ -54,7 +54,6 @@ struct Entry {
 const DIR_FOPS: file::Ops<RoFs> = file::Ops::new::<RoFs>();
 const DIR_IOPS: inode::Ops<RoFs> = inode::Ops::new::<RoFs>();
 const FILE_AOPS: address_space::Ops<RoFs> = address_space::Ops::new::<RoFs>();
-const LNK_IOPS: inode::Ops<RoFs> = inode::Ops::new::<Link>();
 
 struct RoFs;
 
@@ -65,25 +64,30 @@ fn iget(sb: &sb::SuperBlock<Self>, e: &'static Entry) -> Result<ARef<INode<Self>
             Either::Right(new) => new,
         };
 
-        let (mode, nlink, size) = match e.etype {
+        let (mode, nlink, size, typ) = match e.etype {
             inode::Type::Dir => {
                 new.set_iops(DIR_IOPS).set_fops(DIR_FOPS);
-                (0o555, 2, ENTRIES.len().try_into()?)
+                (0o555, 2, ENTRIES.len().try_into()?, inode::Type::Dir)
             }
             inode::Type::Reg => {
                 new.set_fops(file::Ops::generic_ro_file())
                     .set_aops(FILE_AOPS);
-                (0o444, 1, e.contents.len().try_into()?)
+                (0o444, 1, e.contents.len().try_into()?, inode::Type::Reg)
             }
-            inode::Type::Lnk => {
-                new.set_iops(LNK_IOPS);
-                (0o444, 1, e.contents.len().try_into()?)
+            inode::Type::Lnk(_) => {
+                new.set_iops(inode::Ops::simple_symlink_inode());
+                (
+                    0o444,
+                    1,
+                    e.contents.len().try_into()?,
+                    inode::Type::Lnk(Some(e.contents.try_into()?)),
+                )
             }
             _ => return Err(ENOENT),
         };
 
         new.init(inode::Params {
-            typ: e.etype,
+            typ,
             mode,
             size,
             blocks: (u64::try_from(size)? + 511) / 512,
@@ -138,30 +142,6 @@ fn lookup(
     }
 }
 
-struct Link;
-#[vtable]
-impl inode::Operations for Link {
-    type FileSystem = RoFs;
-
-    fn get_link<'a>(
-        dentry: Option<&DEntry<RoFs>>,
-        inode: &'a INode<RoFs>,
-    ) -> Result<Either<CString, &'a CStr>> {
-        if dentry.is_none() {
-            return Err(ECHILD);
-        }
-
-        let name_buf = inode.data().contents;
-        let mut name = Box::new_slice(
-            name_buf.len().checked_add(1).ok_or(ENOMEM)?,
-            b'\0',
-            GFP_NOFS,
-        )?;
-        name[..name_buf.len()].copy_from_slice(name_buf);
-        Ok(Either::Left(name.try_into()?))
-    }
-}
-
 #[vtable]
 impl address_space::Operations for RoFs {
     type FileSystem = Self;
@@ -212,7 +192,7 @@ fn read_dir(
         }
 
         for e in ENTRIES.iter().skip(pos.try_into()?) {
-            if !emitter.emit(1, e.name, e.ino, e.etype.into()) {
+            if !emitter.emit(1, e.name, e.ino, (&e.etype).into()) {
                 break;
             }
         }
-- 
2.34.1