Merge branch 'initfs_cleanups' into 'main'

A bunch of initfs cleanups

See merge request redox-os/base!126

Cargo.lock

@@ -163,17 +163,6 @@ version = "1.0.101"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "5f0e0fee31ef5ed1ba1316088939cea399010ed7731dba877ed44aeb407a75ea"
 
-[[package]]
-name = "archive-common"
-version = "0.1.0"
-dependencies = [
- "anyhow",
- "log",
- "pathdiff",
- "plain",
- "redox-initfs",
-]
-
 [[package]]
 name = "arrayvec"
 version = "0.7.6"
@@ -1750,10 +1739,6 @@ dependencies = [
 name = "redox-initfs"
 version = "0.2.0"
 dependencies = [
- "anyhow",
- "archive-common",
- "env_logger",
- "log",
  "plain",
 ]
 
@@ -1762,10 +1747,10 @@ name = "redox-initfs-tools"
 version = "0.2.0"
 dependencies = [
  "anyhow",
- "archive-common",
  "clap 4.5.58",
  "env_logger",
  "log",
+ "pathdiff",
  "plain",
  "redox-initfs",
  "twox-hash",

Cargo.toml

@@ -5,7 +5,6 @@ members = [
     "daemon",
     "init",
     "initfs",
-    "initfs/archive-common",
     "initfs/tools",
     "ipcd",
     "logd",

bootstrap/src/initfs.rs

@@ -8,7 +8,6 @@ use alloc::string::String;
 use hashbrown::HashMap;
 use redox_initfs::{InitFs, Inode, InodeDir, InodeKind, InodeStruct, types::Timespec};
 
-use redox_path::canonicalize_to_standard;
 use redox_rt::proc::FdGuard;
 use redox_scheme::{CallerCtx, OpenResult, RequestKind, scheme::SchemeSync};
 
@@ -185,8 +184,9 @@ impl SchemeSync for InitFsScheme {
             Self::get_inode(&self.fs, current_inode)?.kind(),
             InodeKind::Link(_)
         );
+        let o_stat_nofollow = flags & O_STAT != 0 && flags & O_NOFOLLOW != 0;
         let o_symlink = flags & O_SYMLINK != 0;
-        if is_link && !o_symlink {
+        if is_link && !o_stat_nofollow && !o_symlink {
             return Err(Error::new(EXDEV));
         }
 
@@ -237,12 +237,7 @@ impl SchemeSync for InitFsScheme {
             InodeKind::Dir(_) => Err(Error::new(EISDIR)),
             InodeKind::Link(link) => {
                 let link_data = link.data().map_err(|_| Error::new(EIO))?;
-                let path = core::str::from_utf8(link_data).map_err(|_| Error::new(ENOENT))?;
-                let cannonical =
-                    canonicalize_to_standard(Some("/"), path).ok_or_else(|| Error::new(ENOENT))?;
-                let data = cannonical.as_bytes();
-
-                let src_buf = &data[core::cmp::min(offset, data.len())..];
+                let src_buf = &link_data[core::cmp::min(offset, link_data.len())..];
 
                 let to_copy = core::cmp::min(src_buf.len(), buffer.len());
                 buffer[..to_copy].copy_from_slice(&src_buf[..to_copy]);
@@ -323,14 +318,16 @@ impl SchemeSync for InitFsScheme {
 
         let inode = Self::get_inode(&self.fs, handle.inode)?;
 
+        stat.st_ino = inode.id();
         stat.st_mode = inode.mode()
             | match inode.kind() {
                 InodeKind::Dir(_) => MODE_DIR,
                 InodeKind::File(_) => MODE_FILE,
+                InodeKind::Link(_) => MODE_SYMLINK,
                 _ => 0,
             };
-        stat.st_uid = inode.uid();
-        stat.st_gid = inode.gid();
+        stat.st_uid = 0;
+        stat.st_gid = 0;
         stat.st_size = u64::try_from(inode_len(inode)?).unwrap_or(u64::MAX);
 
         stat.st_ctime = sec.get();

initfs/.gitignore

@@ -1,4 +0,0 @@
-Cargo.lock
-# TODO: Maybe use an OUT_DIR environment variable, or mktemp, rather than
-# writing directly to the git repository root?
-/out.img

initfs/Cargo.toml

@@ -9,15 +9,3 @@ license = "MIT"
 
 [dependencies]
 plain = "0.2"
-
-[features]
-default = ["std"]
-
-std = []
-
-[dev-dependencies]
-# FIXME remove loggers
-anyhow.workspace = true
-archive-common = {path = "archive-common"}
-env_logger = "0.8"
-log.workspace = true

initfs/archive-common/Cargo.toml

@@ -1,12 +0,0 @@
-[package]
-name = "archive-common"
-version = "0.1.0"
-authors = ["4lDO2 <4lDO2@protonmail.com>", "Kamil Koczurek <koczurekk@gmail.com>"]
-edition = "2021"
-
-[dependencies]
-anyhow.workspace = true
-log.workspace = true
-pathdiff = "0.2.1"
-plain = "0.2"
-redox-initfs = {path = ".."}

initfs/src/lib.rs

@@ -2,9 +2,6 @@
 //! A super simple initfs, only meant to be loaded into RAM by the bootloader, and then directly be
 //! read.
 
-#[cfg(any(test, feature = "std"))]
-extern crate std;
-
 use core::convert::{TryFrom, TryInto};
 
 pub mod types;
@@ -28,14 +25,14 @@ impl core::fmt::Display for Error {
     }
 }
 
-#[cfg(any(test, feature = "std"))]
-impl std::error::Error for Error {}
+impl core::error::Error for Error {}
 
 type Result<T> = core::result::Result<T, Error>;
 
 #[derive(Clone, Copy)]
 pub struct InodeStruct<'initfs> {
     initfs: InitFs<'initfs>,
+    inode_id: Inode,
     inode: &'initfs InodeHeader,
 }
 
@@ -139,42 +136,36 @@ pub enum InodeKind<'initfs> {
 }
 
 impl<'initfs> InodeStruct<'initfs> {
+    pub fn id(&self) -> u64 {
+        self.inode_id.0.into()
+    }
     fn data(&self) -> Result<&'initfs [u8]> {
         let start: usize = self.inode.offset.0.get().try_into().map_err(|_| Error)?;
-
-        let length: usize = self.inode.length.get().try_into().map_err(|_| Error)?;
+        let length: usize = self.inode.length.0.get().try_into().map_err(|_| Error)?;
 
         let end = start.checked_add(length).ok_or(Error)?;
 
         self.initfs.base.get(start..end).ok_or(Error)
     }
     pub fn mode(&self) -> u16 {
-        (self.inode.type_and_mode.get() & MODE_MASK) as u16
-    }
-    pub fn uid(&self) -> u32 {
-        self.inode.uid.get()
-    }
-    pub fn gid(&self) -> u32 {
-        self.inode.gid.get()
+        match self.ty() {
+            Some(InodeType::RegularFile) => 0o444,
+            Some(InodeType::ExecutableFile) => 0o555,
+            Some(InodeType::Dir) => 0o555,
+            Some(InodeType::Link) => 0o777,
+            None => 0o000,
+        }
     }
     fn ty(&self) -> Option<InodeType> {
-        let raw = (self.inode.type_and_mode.get() & TYPE_MASK) >> TYPE_SHIFT;
-
-        Some(if raw == InodeType::Dir as u32 {
-            InodeType::Dir
-        } else if raw == InodeType::RegularFile as u32 {
-            InodeType::RegularFile
-        } else if raw == InodeType::Link as u32 {
-            InodeType::Link
-        } else {
-            return None;
-        })
+        InodeType::try_from(self.inode.type_.get()).ok()
     }
     pub fn kind(&self) -> InodeKind<'initfs> {
         let inner = *self;
         match self.ty() {
             Some(InodeType::Dir) => InodeKind::Dir(InodeDir { inner }),
-            Some(InodeType::RegularFile) => InodeKind::File(InodeFile { inner }),
+            Some(InodeType::RegularFile | InodeType::ExecutableFile) => {
+                InodeKind::File(InodeFile { inner })
+            }
             Some(InodeType::Link) => InodeKind::Link(InodeLink { inner }),
             None => InodeKind::Unknown,
         }
@@ -286,17 +277,11 @@ impl<'initfs> InitFs<'initfs> {
     pub fn inode_count(&self) -> u16 {
         self.get_header_assume_valid().inode_count.get()
     }
-    pub fn get_inode(&self, inode: Inode) -> Option<InodeStruct<'initfs>> {
-        // NOTE: Even for 16-bit architectures (obviously edge-case, but some bootloaders may
-        // perhaps use this code), we have already checked that the inode table can fit within
-        // usize, and the table byte size is always larger than the count.
-        let inode_usize = inode.0 as usize;
-
-        let inode = self.inode_table().get(inode_usize)?;
-
+    pub fn get_inode(&self, inode_id: Inode) -> Option<InodeStruct<'initfs>> {
         Some(InodeStruct {
             initfs: *self,
-            inode,
+            inode_id,
+            inode: self.inode_table().get(usize::from(inode_id.0))?,
         })
     }
 }

initfs/src/types.rs

@@ -11,14 +11,10 @@ macro_rules! primitive(
 
         impl $wrapper {
             #[inline]
-            pub const fn get(&self) -> $primitive {
+            pub const fn get(self) -> $primitive {
                 <$primitive>::from_le_bytes(self.0)
             }
             #[inline]
-            pub fn set(&mut self, primitive: $primitive) {
-                *self = Self::new(primitive);
-            }
-            #[inline]
             pub const fn new(primitive: $primitive) -> Self {
                 Self(<$primitive>::to_le_bytes(primitive))
             }
@@ -84,28 +80,39 @@ const _: () = {
 #[repr(C, packed)]
 #[derive(Clone, Copy, Debug)]
 pub struct InodeHeader {
-    pub type_and_mode: U32,
-    pub length: U32,
+    pub type_: U32,
+    pub length: Length,
     pub offset: Offset,
-    pub uid: U32,
-    pub gid: U32,
 }
 
-pub const MODE_MASK: u32 = 0xFFF;
-pub const MODE_SHIFT: u8 = 0;
-
-pub const TYPE_SHIFT: u8 = 28;
-pub const TYPE_MASK: u32 = 0xF000_0000;
-
 #[derive(Clone, Copy, Debug)]
 #[repr(u8)]
 pub enum InodeType {
     RegularFile = 0x0,
-    Dir = 0x1,
-    Link = 0x2,
+    ExecutableFile = 0x1,
+    Dir = 0x2,
+    Link = 0x3,
     // All other bit patterns are reserved... for now.
 }
 
+impl TryFrom<u32> for InodeType {
+    type Error = ();
+
+    fn try_from(value: u32) -> Result<Self, ()> {
+        Ok(if value == InodeType::RegularFile as u32 {
+            InodeType::RegularFile
+        } else if value == InodeType::ExecutableFile as u32 {
+            InodeType::ExecutableFile
+        } else if value == InodeType::Dir as u32 {
+            InodeType::Dir
+        } else if value == InodeType::Link as u32 {
+            InodeType::Link
+        } else {
+            return Err(());
+        })
+    }
+}
+
 #[repr(C, packed)]
 #[derive(Clone, Copy, Debug)]
 pub struct DirEntry {

initfs/tools/Cargo.toml

@@ -21,8 +21,8 @@ anyhow.workspace = true
 clap = {version = "4", features = ["cargo"]}
 env_logger = "0.8"
 log.workspace = true
+pathdiff = "0.2.1"
 plain = "0.2"
 twox-hash = "1.6"
 
-archive-common = {path = "../archive-common"}
 redox-initfs = {path = ".."}

initfs/tools/src/bin/archive.rs

@@ -3,7 +3,7 @@ use std::path::Path;
 use anyhow::{Context, Result};
 use clap::{Arg, Command};
 
-use archive_common::{self as archive, Args, DEFAULT_MAX_SIZE};
+use redox_initfs_tools::{self as archive, Args, DEFAULT_MAX_SIZE};
 
 fn main() -> Result<()> {
     let matches = Command::new("redox-initfs-ar")

initfs/tools/src/bin/dump.rs

@@ -3,7 +3,7 @@ use std::{ffi::OsStr, path::Path};
 use anyhow::{Context, Result};
 use clap::{Arg, Command};
 
-use redox_initfs::InitFs;
+use redox_initfs::{InitFs, InodeKind};
 
 fn main() -> Result<()> {
     let matches = Command::new("redox-initfs-dump")
@@ -25,7 +25,7 @@ fn main() -> Result<()> {
     let bytes = std::fs::read(source).context("failed to read image into memory")?;
     let initfs = InitFs::new(&bytes, None).context("failed to parse initfs header")?;
 
-    dbg!(initfs.header());
+    println!("{:#?}", initfs.header());
 
     for inode in initfs.all_inodes() {
         print!("{:?}: ", inode);
@@ -37,14 +37,6 @@ fn main() -> Result<()> {
                 continue;
             }
         };
-        print!(
-            "mode={:#0o}, uid={}, gid={}, kind=",
-            inode_struct.mode(),
-            inode_struct.uid(),
-            inode_struct.gid()
-        );
-
-        use redox_initfs::InodeKind;
 
         match inode_struct.kind() {
             InodeKind::Unknown => println!("(unknown)"),

initfs/tools/src/lib.rs

@@ -4,7 +4,7 @@ use std::io::{prelude::*, SeekFrom};
 use std::path::{Path, PathBuf};
 
 use std::os::unix::ffi::OsStrExt;
-use std::os::unix::fs::{FileExt, FileTypeExt, MetadataExt};
+use std::os::unix::fs::{FileExt, FileTypeExt, PermissionsExt};
 
 use anyhow::{anyhow, bail, Context, Result};
 
@@ -120,14 +120,14 @@ fn read_directory(state: &mut State, path: &Path, root_path: &Path) -> Result<Di
                     link_parent.canonicalize()?.join(link_path.clone())
                 };
 
-                let root_path = root_path
+                let dir_path = path
                     .canonicalize()
-                    .context("Failed to cannonicalize root path")?;
-                let path = pathdiff::diff_paths(cannonical, &root_path).ok_or_else(|| {
+                    .context("Failed to cannonicalize path")?;
+                let path = pathdiff::diff_paths(cannonical, &dir_path).ok_or_else(|| {
                     anyhow!(
-                        "Failed to diff symlink path [{}] to root path [{}]",
+                        "Failed to diff symlink path [{}] to path [{}]",
                         link_path.display(),
-                        root_path.display()
+                        dir_path.display()
                     )
                 })?;
                 EntryKind::Link(path)
@@ -238,7 +238,6 @@ fn allocate_and_write_link(state: &mut State, link: &Path) -> Result<WriteResult
 fn write_inode(
     state: &mut State,
     ty: initfs::InodeType,
-    metadata: &Metadata,
     write_result: WriteResult,
     inode: u16,
 ) -> Result<()> {
@@ -246,8 +245,6 @@ fn write_inode(
         .try_into()
         .expect("inode header length cannot fit within u32");
 
-    let type_and_mode = ((ty as u32) << initfs::TYPE_SHIFT) | (metadata.mode() & 0xFFF);
-
     // TODO: Use main buffer and write in bulk.
     let mut inode_buf = [0_u8; std::mem::size_of::<initfs::InodeHeader>()];
 
@@ -255,12 +252,9 @@ fn write_inode(
         .expect("expected inode struct to have alignment 1, and buffer size to match");
 
     *inode_hdr = initfs::InodeHeader {
-        type_and_mode: type_and_mode.into(),
-        length: write_result.size.into(),
+        type_: (ty as u32).into(),
+        length: initfs::Length(write_result.size.into()),
         offset: initfs::Offset(write_result.offset.into()),
-
-        gid: 0.into(), //metadata.gid().into(),
-        uid: 0.into(), //metadata.uid().into(),
     };
 
     log::debug!(
@@ -313,7 +307,13 @@ fn allocate_and_write_dir(
                 let write_result = allocate_and_write_file(state, file)
                     .context("failed to copy file into image")?;
 
-                (write_result, initfs::InodeType::RegularFile)
+                let type_ = if entry.metadata.permissions().mode() & 0o100 != 0 {
+                    initfs::InodeType::ExecutableFile
+                } else {
+                    initfs::InodeType::RegularFile
+                };
+
+                (write_result, type_)
             }
 
             EntryKind::Link(ref path) => {
@@ -328,7 +328,7 @@ fn allocate_and_write_dir(
             .expect("expected dir entry count not to exceed u32");
 
         *current_inode += 1;
-        write_inode(state, ty, &entry.metadata, write_result, *current_inode)?;
+        write_inode(state, ty, write_result, *current_inode)?;
 
         let (name_offset, name_len) = {
             let name_len: u16 = entry.name.len().try_into().context("file name too long")?;
@@ -377,24 +377,14 @@ fn allocate_and_write_dir(
         offset: entry_table_offset,
     })
 }
-fn allocate_contents_and_write_inodes(
-    state: &mut State,
-    dir: &Dir,
-    root_metadata: Metadata,
-) -> Result<()> {
+fn allocate_contents_and_write_inodes(state: &mut State, dir: &Dir) -> Result<()> {
     let start_inode = 0;
     let mut current_inode = start_inode;
 
     let write_result = allocate_and_write_dir(state, dir, &mut current_inode)
         .context("failed to allocate and write all directories and files")?;
 
-    write_inode(
-        state,
-        initfs::InodeType::Dir,
-        &root_metadata,
-        write_result,
-        start_inode,
-    )
+    write_inode(state, initfs::InodeType::Dir, write_result, start_inode)
 }
 
 struct OutputImageGuard<'a> {
@@ -481,9 +471,6 @@ pub fn archive(
     };
 
     let root_path = source;
-    let root_metadata = root_path
-        .metadata()
-        .context("failed to obtain metadata for root")?;
     let root = read_directory(&mut state, root_path, root_path).context("failed to read root")?;
 
     log::debug!("there are {} inodes", state.inode_count);
@@ -535,7 +522,7 @@ pub fn archive(
 
     state.inode_table_offset = inode_table_offset.0.get();
 
-    allocate_contents_and_write_inodes(&mut state, &root, root_metadata)?;
+    allocate_contents_and_write_inodes(&mut state, &root)?;
 
     let current_system_time = std::time::SystemTime::now();
 

initfs/tools/tests/archive_and_read.rs

@@ -1,6 +1,6 @@
 use std::{collections::HashMap, path::Path};
 
-use anyhow::{Context, Result, anyhow};
+use anyhow::{anyhow, Context, Result};
 use redox_initfs::{InitFs, InodeKind, InodeStruct};
 
 #[derive(Debug, Clone, PartialEq)]
@@ -74,13 +74,13 @@ fn build_tree<'a>(fs: InitFs<'a>, inode: InodeStruct<'a>) -> anyhow::Result<Node
 fn archive_and_read() -> Result<()> {
     env_logger::init();
 
-    let args = archive_common::Args {
-        destination_path: Path::new("out.img"),
+    let args = redox_initfs_tools::Args {
+        destination_path: &Path::new(env!("CARGO_TARGET_TMPDIR")).join("out.img"),
         source: Path::new("data"),
         bootstrap_code: None,
-        max_size: archive_common::DEFAULT_MAX_SIZE,
+        max_size: redox_initfs_tools::DEFAULT_MAX_SIZE,
     };
-    archive_common::archive(&args).context("failed to archive")?;
+    redox_initfs_tools::archive(&args).context("failed to archive")?;
 
     let data = std::fs::read(args.destination_path).context("failed to read new archive")?;
     let filesystem =