Rewrite fuse-overlayfs in Rust by giuseppe · Pull Request #457 · containers/fuse-overlayfs

giuseppe · 2026-03-25T21:23:07Z

Drop-in rewrite of fuse-overlayfs from C to Rust. Same CLI, same mount options, same on-disk format.

saschagrunert

Took a pass through the Rust rewrite. Left a few comments, mostly around dead code and a dangling fd in main.rs.

saschagrunert · 2026-03-26T10:35:26Z

src/main.rs

+                std::process::exit(1);
+            }
+        };
+        fd.as_raw_fd()


Bug: fd is an OwnedFd that gets dropped at the end of this if let block, closing the OS file descriptor. workdir_fd then holds a dangling raw fd.

Keep the OwnedFd alive (e.g. store it alongside workdir_fd, or pass the OwnedFd directly to OverlayFs::new).

saschagrunert · 2026-03-26T10:35:26Z

src/main.rs

+    };
+
+    // Read overflow IDs
+    let _overflow = mapping::OverflowIds::read();


Dead code: _overflow is never used. OverlayInner::new creates its own OverflowIds. This can be removed.

saschagrunert · 2026-03-26T10:35:26Z

src/main.rs

+            "noexec" => fuse_options.push(fuser::MountOption::NoExec),
+            "atime" => fuse_options.push(fuser::MountOption::Atime),
+            "noatime" => fuse_options.push(fuser::MountOption::NoAtime),
+            "diratime" => fuse_options.push(fuser::MountOption::DirSync), // closest mapping


Wrong mapping: DirSync != diratime. diratime controls directory access-time updates. This should be CUSTOM("diratime".to_string()).

saschagrunert · 2026-03-26T10:35:26Z

src/sys/openat2.rs

+            let parent_fd = safe_openat(
+                dirfd,
+                parent,
+                libc::O_DIRECTORY | libc::O_NOFOLLOW | libc::O_PATH,


O_NOFOLLOW causes ELOOP if any component in the parent path is a symlink. Since openat2 with RESOLVE_IN_ROOT already handles symlink containment, and the plain openat fallback path doesn't benefit from O_NOFOLLOW on intermediate components, drop O_NOFOLLOW and keep O_DIRECTORY | O_PATH.

saschagrunert · 2026-03-26T10:35:26Z

src/sys/openat2.rs

+        }
+    }
+
+    // Fallback to plain openat


Silent fallback from openat2 to plain openat drops the RESOLVE_IN_ROOT guarantee. Log a warning so users know the security constraint is absent.

saschagrunert · 2026-03-26T10:35:26Z

Cargo.toml

+fuser = { version = "0.17", features = ["abi-7-40"] }
+nix = { version = "0.29", features = ["fs", "dir", "mount", "user", "signal", "resource"] }
+libc = "0.2"
+libloading = "0.8"


libloading is unused in the codebase. This can be removed.

saschagrunert · 2026-03-26T10:35:26Z

src/sys/dir.rs

+
+// Safety: DirStream is only used within the big-lock mutex in OverlayFs.
+// The mutex guarantees single-threaded access.
+unsafe impl Send for DirStream {}


Comment says "never handed to another thread" but Send is exactly what enables cross-thread transfer. The real safety invariant is that DIR* is only used by one thread at a time (no concurrent readdir calls), which &mut self on next() already enforces.

saschagrunert · 2026-03-26T10:35:26Z

src/mapping.rs

+}
+
+/// Convenience: map a UID from host to container.
+pub fn get_uid(


get_uid and get_gid are never called. These can be removed.

saschagrunert · 2026-03-26T10:35:26Z

src/node.rs

+            fuse_map: FxHashMap::default(),
+            same_device: true,
+            next_fallback: 0x8000_0000_0000_0000,
+        }


With ino_t_32 (32-bit inode numbers), next_fallback starts at u32::MAX and wraps around quickly, producing values 0 and 1 that collide with FUSE_ROOT_ID. Consider starting at 2 or skipping reserved values.

saschagrunert · 2026-03-26T10:57:15Z

src/overlay.rs

+                    drop(backings);
+                    self.open_files.write().insert(fh, Arc::new(fd));
+                    self.fh_to_ino.write().insert(fh, ino);
+                    self.inode_backings.write().get_mut(&ino).unwrap().1 += 1;


Race condition: the read lock on inode_backings is dropped on line 198, then a write lock is re-acquired here. Between the two, a concurrent release() can remove this entry, causing .unwrap() to panic.

Take a single write lock for the whole block instead of the read-then-write pattern.

saschagrunert · 2026-03-26T10:57:15Z

src/copyup.rs

+        workdir_fd,
+        &wd_name,
+        libc::O_CREAT | libc::O_WRONLY,
+        mode | 0o200,


File is created with mode | 0o200 to allow writing during copy, but fchmod is never called afterward to restore the original mode. A 0o444 source file ends up 0o644 after copy-up.

Add fchmod(dfd, mode) after copy_data completes.

saschagrunert · 2026-03-26T10:57:15Z

src/sys/dir.rs

+            // SAFETY: d_name is a null-terminated C string in the dirent.
+            let name = unsafe {
+                std::ffi::CStr::from_ptr(entry.d_name.as_ptr())
+                    .to_string_lossy()


to_string_lossy() replaces non-UTF-8 bytes with U+FFFD, making files with non-UTF-8 names (which are valid on Linux) inaccessible or silently mapped to the wrong name. This is a regression from the C version, which handles raw bytes.

Consider using OsString / CString throughout instead of String.

saschagrunert · 2026-03-26T10:57:15Z

src/mapping.rs

+    }
+
+    for m in mappings {
+        if direct {


m.host + m.len can overflow u32 when the mapping covers the top of the ID range. Use id >= m.host && id - m.host < m.len instead.

Same issue on line 90 with m.to + m.len.

saschagrunert · 2026-03-26T10:57:15Z

src/overlay.rs

+        // root_id is always valid; if not, we have a fatal invariant violation
+        self.nodes
+            .get(&self.root_id)
+            .expect("root node missing from arena")


expect() will panic and crash the FUSE daemon if this invariant is ever violated. This goes against the project's own "no panic" rule in AGENTS.md. Consider returning a Result or using fuse_err!.

saschagrunert · 2026-03-26T10:57:15Z

src/main.rs

+//
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#![allow(dead_code)]


#![allow(dead_code)] at crate level suppresses warnings for all unused code, hiding real dead code (e.g. the get_uid/get_gid functions flagged in the previous review). Consider removing this and cleaning up the actual dead code.

saschagrunert · 2026-03-26T10:57:15Z

Cargo.toml

+description = "Overlay Filesystem in Userspace"
+
+[dependencies]
+fuser = { version = "0.17", features = ["abi-7-40"] }


Consider pinning dependencies to their exact patch versions (e.g. fuser = "0.17.1", nix = "0.29.1", etc.). For a filesystem, a semver-compatible but untested patch bump could introduce subtle behavioral changes or regressions. Pinning to known-good patch versions, combined with the Cargo.lock, gives more control over when updates are adopted.

saschagrunert · 2026-03-26T11:01:11Z

This is a good chance to enhance the docs for this project, too.

saschagrunert

Second round of review. Previous comments were mostly addressed; these are the remaining and new findings.

saschagrunert · 2026-03-26T13:59:39Z

Cargo.toml

+[package]
+name = "fuse-overlayfs"
+version = "2.0.0"
+edition = "2021"


Consider updating to edition = "2024". The 2024 edition enables unsafe_op_in_unsafe_fn by default, which would add an extra safety layer in the src/sys/ modules where most of the unsafe code lives. It also enables other useful lints like rust_2024_compatibility.

saschagrunert · 2026-03-26T13:59:39Z

src/sys/dir.rs

+            let name = unsafe {
+                std::ffi::CStr::from_ptr(entry.d_name.as_ptr())
+                    .to_string_lossy()
+                    .into_owned()


This to_string_lossy() silently mangles non-UTF-8 filenames, which is a regression from the C version that handled raw bytes. This is a systemic issue: RawDirEntry.name, OvlNode.name, and the children map keys in node.rs all use String. They should use OsString (or CString/Vec<u8>) to preserve arbitrary POSIX filenames.

saschagrunert · 2026-03-26T13:59:39Z

src/sys/openat2.rs

+const RESOLVE_IN_ROOT: u64 = 0x10;
+
+#[cfg(target_arch = "x86_64")]
+const SYS_OPENAT2: libc::c_long = 437;


These per-arch #[cfg] blocks all define SYS_OPENAT2 = 437 (same value on every architecture). A single const SYS_OPENAT2: libc::c_long = 437; would suffice, or better yet use libc::SYS_openat2 which the libc crate already exports. The current approach silently fails to compile on unlisted architectures (e.g. loongarch64) with no error message.

saschagrunert · 2026-03-26T13:59:39Z

src/config.rs

+    pub timeout: f64,
+    pub threaded: i32,
+    pub fsync: i32,
+    pub fast_ino_check: i32,


Several fields here use i32 where more precise types would be appropriate:

fsync, writeback, disable_xattrs, fast_ino_check, threaded are used as booleans (!= 0) but typed as i32.

squash_to_uid / squash_to_gid use -1 as a sentinel for "not set" instead of Option<u32>.

Using bool and Option<u32> respectively would make the intent clearer and prevent bugs from arithmetic on what are logically boolean/optional values.

saschagrunert · 2026-03-26T13:59:39Z

.github/workflows/rust-test.yaml

+
+    - name: Build
+      run: cargo build --release
+


There is no cargo clippy or cargo fmt -- --check step, despite AGENTS.md requiring zero warnings and formatted code. Adding these as CI steps would enforce those requirements.

saschagrunert · 2026-03-26T13:59:39Z

.github/workflows/rust-test.yaml

+
+    - name: Install cross
+      run: cargo install cross --git https://github.com/cross-rs/cross
+


cargo install cross --git https://github.com/cross-rs/cross installs from HEAD, which is not reproducible and could break without warning. Pin to a specific tag or revision, e.g. cargo install cross --git https://github.com/cross-rs/cross --tag v0.2.5.

giuseppe · 2026-03-26T15:34:16Z

@saschagrunert thanks for the review! I will ping you once I've addressed all the comments (I am pushing just to trigger the CI)

Add comprehensive end-to-end tests covering copyup, directory operations, hardlinks, readonly mode, rename (including RENAME_NOREPLACE and RENAME_EXCHANGE), special files, symlinks, and xattr handling. These tests validate the behavior of fuse-overlayfs and serve as a regression suite. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>

mknod for character/block devices requires CAP_MKNOD in the initial user namespace. Skip those test cases gracefully when mknod is not available, allowing the full test suite to pass rootless. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>

giuseppe · 2026-03-31T08:44:03Z

comments addressed!

Complete reimplementation of fuse-overlayfs in Rust as a drop-in replacement. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>

Add a release job that collects all cross-compiled binaries (x86_64, aarch64, armv7l, s390x, ppc64le, riscv64), generates SHA256SUMS, and creates a draft GitHub release when a version tag is pushed. Matches the existing C release workflow structure. Also rename armv7 artifact to armv7l for consistency with the C release. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>

giuseppe · 2026-04-07T10:16:41Z

@mheon FYI

giuseppe force-pushed the rust-rewrite branch 4 times, most recently from 84edfe7 to e64d961 Compare March 26, 2026 10:11

saschagrunert reviewed Mar 26, 2026

View reviewed changes

giuseppe force-pushed the rust-rewrite branch from e64d961 to dfd933c Compare March 26, 2026 10:46

saschagrunert reviewed Mar 26, 2026

View reviewed changes

giuseppe force-pushed the rust-rewrite branch 4 times, most recently from 2d2bb1d to 189db78 Compare March 26, 2026 13:48

saschagrunert reviewed Mar 26, 2026

View reviewed changes

giuseppe force-pushed the rust-rewrite branch from 189db78 to e387a1a Compare March 26, 2026 15:06

giuseppe force-pushed the rust-rewrite branch 6 times, most recently from e45068c to b592a54 Compare March 28, 2026 11:26

giuseppe and others added 5 commits March 28, 2026 16:28

tests: support both docker and podman as container runtime

7bebaa6

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>

tests: fix test scripts to find binary from PATH and add CI dependencies

6ba02e7

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>

AGENTS.md: new file

3df47d8

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>

giuseppe force-pushed the rust-rewrite branch 3 times, most recently from d217063 to 8db4c06 Compare March 28, 2026 16:42

giuseppe force-pushed the rust-rewrite branch from 8db4c06 to 3ab01dd Compare April 2, 2026 20:39

giuseppe and others added 3 commits April 3, 2026 11:45

fuse-overlayfs: rewrite in Rust

90d4654

Complete reimplementation of fuse-overlayfs in Rust as a drop-in replacement. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>

fuse-overlayfs: remove C implementation

a26b802

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>

giuseppe force-pushed the rust-rewrite branch from 3ab01dd to a26b802 Compare April 3, 2026 09:45

giuseppe changed the title ~~[RFC][WIP] Rewrite fuse-overlayfs in Rust~~ Rewrite fuse-overlayfs in Rust Apr 3, 2026

giuseppe mentioned this pull request Apr 10, 2026

feat: introduce Android bionic libc compatibility layer #458

Open


		- name: Install cross
		run: cargo install cross --git https://github.com/cross-rs/cross

Conversation

giuseppe commented Mar 25, 2026

Uh oh!

saschagrunert left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

saschagrunert Mar 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

saschagrunert Mar 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

saschagrunert Mar 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

saschagrunert commented Mar 26, 2026

Uh oh!

saschagrunert left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

giuseppe commented Mar 26, 2026

Uh oh!

giuseppe commented Mar 31, 2026

Uh oh!

giuseppe commented Apr 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

saschagrunert left a comment •

edited

Loading

saschagrunert Mar 26, 2026 •

edited

Loading

saschagrunert Mar 26, 2026 •

edited

Loading

saschagrunert Mar 26, 2026 •

edited

Loading