Use `DirEntry::file_type` rather than `metadata...` in `list`
What the developer wrote
Authored by Matt Corallo
Use `DirEntry::file_type` rather than `metadata...` in `list`
In the discussions at #3799 it was noted that `DirEntry::file_type`
will often use cached information rather than making a fresh
syscall, fixing the `list` race condition where we lose files while
iterating the directory for some filesystems on some Unix
platforms.
For some reason, that fix didn't make it into the merged PR, and we
rather stuck with `DirEntry::metadata()` which *always* does a
fresh syscall and always exhibits the problematic behavior. Here we
simply swap for `DirEntry::file_type` which at least fixes the
issue for "some filesome filesystems (among them: Btrfs, ext2,
ext3, and ext4)" (per `readdir(3)`).
What changed, and why it matters
This commit fixes a subtle file-listing bug in the Lightning payment channel data storage code. When listing saved data, the program used to ask the filesystem for fresh details about each file, which on common Linux filesystems can briefly make a file appear missing if it is being changed at the same time. The change uses cached information already returned by the directory scan, avoiding that extra check and the race condition. It is a reliability fix rather than a direct exploit, but in the worst case it could cause the node to fail to load channel data it expected to find.
Treat as a low-risk reliability/robustness patch. Review whether the same metadata() pattern exists elsewhere in fs_store.rs or related persistence modules and apply the same fix if applicable. No urgent security response is indicated.
Security signals we found
Race condition in directory enumeration
TOCTOU-style metadata check
Filesystem persistence layer reliability
No input validation or crypto changes
Evidence from the diff
In lightning-persister/src/fs_store.rs, dir_entry_is_key() now calls DirEntry::file_type() instead of DirEntry::metadata(). The commit message notes that file_type() can use cached readdir results, avoiding a fresh stat syscall and eliminating a race where concurrent filesystem activity on Btrfs/ext2/ext3/ext4 could cause entries to be skipped during list(). This is a defensive fix for TOCTOU-like behavior in directory enumeration. It does not change access-control checks, cryptographic handling, or network behavior.
Changed components
lightning-persister/src/fs_store.rsdir_entry_is_key()list() / directory listing pathInspect captured patch +3 / −3
diff --git a/lightning-persister/src/fs_store.rs b/lightning-persister/src/fs_store.rs
index 73c24dc..3129748 100644
--- a/lightning-persister/src/fs_store.rs
+++ b/lightning-persister/src/fs_store.rs
@@ -560,15 +560,15 @@ fn dir_entry_is_key(dir_entry: &fs::DirEntry) -> Result<bool, lightning::io::Err
}
}
- let metadata = dir_entry.metadata()?;
+ let file_type = dir_entry.file_type()?;
// We allow the presence of directories in the empty primary namespace and just skip them.
- if metadata.is_dir() {
+ if file_type.is_dir() {
return Ok(false);
}
// If we otherwise don't find a file at the given path something went wrong.
- if !metadata.is_file() {
+ if !file_type.is_file() {
debug_assert!(
false,
"Failed to list keys at path {}: file couldn't be accessed.",
Why this scored 31/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.