From 79429edb3f7606240152e89b5b5fdecb246675ad Mon Sep 17 00:00:00 2001 From: Keiichi Watanabe Date: Mon, 27 May 2024 15:53:48 +0900 Subject: [PATCH] ext2: Support symlinks whose length is less than 60 bytes In ext2, if symlink name is shorter than 60 bytes, the data is stored in the i_block member in inode. BUG=b:342937495 TEST=cargo test TEST=start VM with pmem-ext2 Change-Id: I1bac1405625af92cf521d8da502b19a1deacad20 Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/5569437 Reviewed-by: Junichi Uekawa Commit-Queue: Keiichi Watanabe --- ext2/src/fs.rs | 47 ++++++++++++++++-- ext2/src/inode.rs | 27 ++++++++++- ext2/tests/tests.rs | 113 ++++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 177 insertions(+), 10 deletions(-) diff --git a/ext2/src/fs.rs b/ext2/src/fs.rs index fd004b52c6..bf3e0a299c 100644 --- a/ext2/src/fs.rs +++ b/ext2/src/fs.rs @@ -8,6 +8,7 @@ use std::collections::BTreeMap; use std::ffi::OsStr; use std::ffi::OsString; +use std::fs::DirEntry; use std::fs::File; use std::os::unix::ffi::OsStrExt; use std::path::Path; @@ -546,6 +547,45 @@ impl<'a> Ext2<'a> { Ok(()) } + fn add_symlink( + &mut self, + arena: &'a Arena<'a>, + parent: InodeNum, + entry: &DirEntry, + ) -> Result<()> { + let link = entry.path(); + let dst_path = std::fs::read_link(&link)?; + let dst = dst_path + .to_str() + .context("failed to convert symlink destination to str")?; + + if dst.len() >= InodeBlock::max_inline_symlink_len() { + // TODO(b/342937495): Support symlink longer than or equal to 60 bytes. + unimplemented!("long symlink is not supported yet: {:?}", dst); + } + + let inode_num = self.allocate_inode()?; + let mut block = InodeBlock::default(); + block.set_inline_symlink(dst)?; + + let inode = Inode::from_metadata( + arena, + &mut self.group_metadata, + inode_num, + &std::fs::symlink_metadata(&link)?, + dst.len() as u32, + 1, //links_count, + 0, //blocks, + block, + )?; + self.add_inode(inode_num, inode)?; + + let link_name = link.file_name().context("failed to get symlink name")?; + self.allocate_dir_entry(arena, parent, inode_num, InodeType::Symlink, link_name)?; + + Ok(()) + } + /// Walks through `src_dir` and copies directories and files to the new file system. fn copy_dirtree>(&mut self, arena: &'a Arena<'a>, src_dir: P) -> Result<()> { self.copy_dirtree_rec(arena, InodeNum(2), src_dir) @@ -557,7 +597,7 @@ impl<'a> Ext2<'a> { parent_inode: InodeNum, src_dir: P, ) -> Result<()> { - for entry in std::fs::read_dir(src_dir)? { + for entry in std::fs::read_dir(&src_dir)? { let entry = entry?; let ftype = entry.file_type()?; if ftype.is_dir() { @@ -581,10 +621,7 @@ impl<'a> Ext2<'a> { ) })?; } else if ftype.is_symlink() { - let src = entry.path(); - let dst = std::fs::read_link(&src)?; - // TODO(b/342937495): support symlink - bail!("symlink is not supported yet: {src:?} -> {dst:?}"); + self.add_symlink(arena, parent_inode, &entry)?; } else { panic!("unknown file type: {:?}", ftype); } diff --git a/ext2/src/inode.rs b/ext2/src/inode.rs index 33138f610d..95c5080d29 100644 --- a/ext2/src/inode.rs +++ b/ext2/src/inode.rs @@ -77,6 +77,8 @@ impl From for usize { } } +/// Size of the `block` field in Inode. +const INODE_BLOCK_LEN: usize = 60; /// Represents 60-byte region for block in Inode. /// This region is used for various ways depending on the file type. /// For regular files and directories, it's used for storing 32-bit indices of blocks. @@ -84,11 +86,11 @@ impl From for usize { /// This is a wrapper of `[u8; 60]` to implement `Default` manually. #[repr(C)] #[derive(Debug, Copy, Clone, FromZeroes, FromBytes, AsBytes)] -pub(crate) struct InodeBlock(pub [u8; 60]); +pub(crate) struct InodeBlock(pub [u8; INODE_BLOCK_LEN]); impl Default for InodeBlock { fn default() -> Self { - Self([0; 60]) + Self([0; INODE_BLOCK_LEN]) } } @@ -133,6 +135,27 @@ impl InodeBlock { pub fn set_double_indirect_block_table(&mut self, block_id: &BlockId) -> Result<()> { self.set_block_id(Self::DOUBLE_INDIRECT_BLOCK_TABLE_ID, block_id) } + + /// Returns the max length of symbolic links that can be stored in the inode data. + /// This length contains the trailing `\0`. + pub const fn max_inline_symlink_len() -> usize { + INODE_BLOCK_LEN + } + + /// Stores a given string as an inlined symbolic link data. + pub fn set_inline_symlink(&mut self, symlink: &str) -> Result<()> { + let bytes = symlink.as_bytes(); + if bytes.len() >= Self::max_inline_symlink_len() { + bail!( + "symlink '{symlink}' exceeds or equals tomax length: {} >= {}", + bytes.len(), + Self::max_inline_symlink_len() + ); + } + + self.0[..bytes.len()].copy_from_slice(bytes); + Ok(()) + } } /// The ext2 inode. diff --git a/ext2/tests/tests.rs b/ext2/tests/tests.rs index 4761b71a5c..72508b3369 100644 --- a/ext2/tests/tests.rs +++ b/ext2/tests/tests.rs @@ -5,13 +5,17 @@ #![cfg(target_os = "linux")] use std::collections::BTreeSet; +use std::fs; use std::fs::create_dir; +use std::fs::read_link; +use std::fs::symlink_metadata; use std::fs::File; use std::fs::OpenOptions; use std::io::BufWriter; use std::io::Seek; use std::io::SeekFrom; use std::io::Write; +use std::os::unix::fs::symlink; use std::path::Path; use std::path::PathBuf; use std::process::Command; @@ -163,14 +167,26 @@ fn assert_eq_dirs(td: &TempDir, dir: &Path, disk: &PathBuf) { for ((name1, path1), (name2, path2)) in paths1.iter().zip(paths2.iter()) { assert_eq!(name1, name2); - let m1 = std::fs::metadata(path1).unwrap(); - let m2 = std::fs::metadata(path2).unwrap(); + let m1 = symlink_metadata(path1).unwrap(); + let m2 = symlink_metadata(path2).unwrap(); assert_eq!( m1.file_type(), m2.file_type(), "file type mismatch ({name1})" ); - assert_eq!(m1.len(), m2.len(), "length mismatch ({name1})"); + + if m1.file_type().is_symlink() { + let dst1 = read_link(path1).unwrap(); + let dst2 = read_link(path2).unwrap(); + assert_eq!( + dst1, dst2, + "symlink mismatch ({name1}): {:?}->{:?} vs {:?}->{:?}", + path1, dst1, path2, dst2 + ); + } else { + assert_eq!(m1.len(), m2.len(), "length mismatch ({name1})"); + } + assert_eq!( m1.permissions(), m2.permissions(), @@ -319,3 +335,94 @@ fn test_mkfs_indirect_block() { assert_eq_dirs(&td, &dir, &disk); } + +#[test] +fn test_mkfs_symlink() { + // testdata + // ├── a.txt + // ├── self -> ./self + // ├── symlink0 -> ./a.txt + // ├── symlink1 -> ./symlink0 + // └── dir + // └── upper-a -> ../a.txt + let td = tempdir().unwrap(); + let dir = td.path().join("testdata"); + create_dir(&dir).unwrap(); + + let mut f = File::create(dir.join("a.txt")).unwrap(); + f.write_all("Hello".as_bytes()).unwrap(); + + symlink("./self", dir.join("self")).unwrap(); + + symlink("./a.txt", dir.join("symlink0")).unwrap(); + symlink("./symlink0", dir.join("symlink1")).unwrap(); + + create_dir(dir.join("dir")).unwrap(); + symlink("../a.txt", dir.join("dir/upper-a")).unwrap(); + + let disk = mkfs( + &td, + &Config { + blocks_per_group: 2048, + inodes_per_group: 4096, + }, + Some(&dir), + ); + + assert_eq_dirs(&td, &dir, &disk); +} + +#[test] +fn test_mkfs_abs_symlink() { + // testdata + // ├── a.txt + // ├── a -> /testdata/a + // ├── self -> /testdata/self + // ├── tmp -> /tmp + // └── abc -> /a/b/c + let td = tempdir().unwrap(); + let dir = td.path().join("testdata"); + + std::fs::create_dir(&dir).unwrap(); + File::create(dir.join("a.txt")).unwrap(); + symlink(dir.join("a.txt"), dir.join("a")).unwrap(); + symlink(dir.join("self"), dir.join("self")).unwrap(); + symlink("/tmp/", dir.join("tmp")).unwrap(); + symlink("/a/b/c", dir.join("abc")).unwrap(); + + let disk = mkfs( + &td, + &Config { + blocks_per_group: 2048, + inodes_per_group: 4096, + }, + Some(&dir), + ); + + assert_eq_dirs(&td, &dir, &disk); +} + +#[test] +fn test_mkfs_symlink_to_deleted() { + // testdata + // ├── (deleted) + // └── symlink_to_deleted -> (deleted) + let td = tempdir().unwrap(); + let dir = td.path().join("testdata"); + + std::fs::create_dir(&dir).unwrap(); + File::create(dir.join("deleted")).unwrap(); + symlink("./deleted", dir.join("symlink_to_deleted")).unwrap(); + fs::remove_file(dir.join("deleted")).unwrap(); + + let disk = mkfs( + &td, + &Config { + blocks_per_group: 2048, + inodes_per_group: 4096, + }, + Some(&dir), + ); + + assert_eq_dirs(&td, &dir, &disk); +}