fix(filesystem): resolve critical data loss, POSIX non-compliance, and deadlock vulnerabilities

- Standardize POSIX unlink (EISDIR) and rmdir (ENOTDIR/ENOTEMPTY) checks
- Defer local directory eviction in mode switch until FUSE mount verification succeeds
- Enforce fsync before atomic renames in ContentCache store operations
- Add directory ancestor loop check in rename (EINVAL)
- Implement punch_hole interval tracking in fallocate
- Purge un-created local node write revisions on unlink
- Fix control socket permission race with octal umask 0o77
- Add unit tests for state, DB, and FUSE POSIX logic (208/208 tests passing)
This commit is contained in:
nvrl committed 2026-07-22 00:23:55 +02:00
1 parent 9d55ffb9f6
commit fe75998cea
8 files changed
+434 -28

No files matched your search

+19 -3
View File
@@ -153,10 +153,12 @@ pub struct StagedWrite {
/// The identity of a remote revision, as far as we can observe it: a file whose
/// size and mtime both still match the ones a queued change was made against has
/// not been rewritten by anyone else in the meantime.
#[derive(Serialize, Deserialize, Debug, Clone, Copy, PartialEq, Eq)]
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq)]
pub struct Baseline {
pub mtime: i64,
pub size: u64,
#[serde(default)]
pub hash: Option<String>,
}
/// Content cache rooted at a directory, with a sibling pin-registry file.
@@ -431,7 +433,12 @@ impl ContentCache {
// 64-char hex key with no `.`, so this appends rather than replaces.
// `store_thumbnail` cannot use the same idiom — see the note there.
let tmp = blob.with_extension("tmp");
std::fs::write(&tmp, bytes)?;
{
let mut f = std::fs::File::create(&tmp)?;
use std::io::Write;
f.write_all(bytes)?;
f.sync_all()?;
}
std::fs::rename(&tmp, &blob)?;
let meta = serde_json::to_vec(&Meta { mtime, size })?;
std::fs::write(self.meta_path(uid), meta)?;
@@ -464,6 +471,9 @@ impl ContentCache {
if std::fs::hard_link(src, &tmp).is_err() {
std::fs::copy(src, &tmp)?;
}
if let Ok(f) = std::fs::File::open(&tmp) {
let _ = f.sync_all();
}
let bytes = std::fs::metadata(&tmp)?.len();
std::fs::rename(&tmp, &blob)?;
let meta = serde_json::to_vec(&Meta { mtime, size })?;
@@ -526,7 +536,12 @@ impl ContentCache {
let tmp = self
.block_dir
.join(format!("{}.b{idx}.tmp", Self::key(uid)));
std::fs::write(&tmp, bytes)?;
{
let mut f = std::fs::File::create(&tmp)?;
use std::io::Write;
f.write_all(bytes)?;
f.sync_all()?;
}
std::fs::rename(&tmp, &blob)?;
std::fs::write(
self.block_meta(uid, idx),
@@ -1759,6 +1774,7 @@ mod tests {
based_on: Some(Baseline {
mtime: 100,
size: 40,
hash: None,
}),
};
let staged = c.stage_write(&meta, &scratch).unwrap();
+12
View File
@@ -118,6 +118,18 @@ impl Db {
Ok(())
}
/// Check if a folder node has any non-trashed children in the database.
pub fn has_children(&self, parent_uid: &NodeUid) -> Result<bool> {
let uid_str = parent_uid.to_string();
let conn = self.conn.lock();
let count: i64 = conn.query_row(
"SELECT COUNT(*) FROM nodes WHERE parent_uid = ?1 AND trashed = 0",
params![uid_str],
|row| row.get(0),
)?;
Ok(count > 0)
}
/// Full-text search over node names, newest schema's trigram index giving
/// substring (not just prefix) matches. Returns up to `limit` non-trashed
/// hits, each with its mountpoint-relative path resolved. Queries shorter
+11
View File
@@ -230,6 +230,17 @@ impl Db {
Ok(n > 0)
}
/// Check if a queued create or mkdir op exists for `uid`.
pub fn has_create_op(&self, uid: &str) -> Result<bool> {
let conn = self.conn.lock();
let count: i64 = conn.query_row(
"SELECT COUNT(*) FROM pending_op WHERE uid = ?1 AND kind IN (?2, ?3)",
params![uid, OP_CREATE, OP_MKDIR],
|row| row.get(0),
)?;
Ok(count > 0)
}
/// Drop every op targeting a node **or anything queued beneath it**,
/// returning the staged blobs they held so the caller can delete them.
///
+88
View File
@@ -1522,3 +1522,91 @@ fn db_contention_under_fuse_worker_load() {
serial / total as u32
);
}
#[test]
fn test_db_has_children_and_trashed_filtering() {
use proton_drive_rs::proton_sdk::ids::{LinkId, VolumeId};
let db = Db::open_in_memory().unwrap();
let parent_uid = NodeUid::new(VolumeId::from("vol"), LinkId::from("parent"));
let child_uid = NodeUid::new(VolumeId::from("vol"), LinkId::from("child"));
assert!(
!db.has_children(&parent_uid).unwrap(),
"no children initially"
);
let parent_node = Node {
uid: parent_uid.clone(),
parent_uid: None,
name: "parent".into(),
kind: NodeKind::Folder,
creation_time: 100,
modification_time: 100,
trashed: false,
is_shared: false,
is_shared_publicly: false,
signature_email: None,
verification: Default::default(),
};
db.upsert_node(&parent_node).unwrap();
let mut child_node = Node {
uid: child_uid.clone(),
parent_uid: Some(parent_uid.clone()),
name: "child.txt".into(),
kind: NodeKind::File {
media_type: "text/plain".into(),
total_size_on_storage: 10,
active_revision_state: None,
claimed_size: Some(10),
claimed_modification_time: None,
},
creation_time: 100,
modification_time: 100,
trashed: false,
is_shared: false,
is_shared_publicly: false,
signature_email: None,
verification: Default::default(),
};
db.upsert_node(&child_node).unwrap();
assert!(db.has_children(&parent_uid).unwrap(), "child node present");
// Trashing child removes it from active children
child_node.trashed = true;
db.upsert_node(&child_node).unwrap();
assert!(
!db.has_children(&parent_uid).unwrap(),
"trashed child ignored"
);
}
#[test]
fn test_db_has_create_op() {
let db = Db::open_in_memory().unwrap();
let local_uid = "local~testnode12345678901234567890";
assert!(!db.has_create_op(local_uid).unwrap(), "no op initially");
let op = ops::PendingOp {
id: 0,
kind: ops::OP_CREATE.to_string(),
uid: local_uid.to_string(),
parent_uid: Some("vol~parent".to_string()),
name: Some("test.txt".to_string()),
blob_path: None,
meta_json: None,
created_at: 1000,
attempts: 0,
last_error: None,
next_attempt_at: 0,
};
db.enqueue_op(&op).unwrap();
assert!(db.has_create_op(local_uid).unwrap(), "create op present");
db.delete_ops_for_uid(local_uid).unwrap();
assert!(!db.has_create_op(local_uid).unwrap(), "op deleted");
}
+15 -15
View File
@@ -683,26 +683,26 @@ impl Core {
// fails, since eviction may already have deleted part of the tree.
// Recovery is the user's explicit switch back to `mirror`, whose arm
// clears the baseline and re-downloads.
self.db
.sync_folder_set_mode(id, "ondemand")
.map_err(|e| SwitchBlocked::Failed(format!("db: {e:?}")))?;
// Reclaim the disk: empty the local dir (keep it as the mountpoint).
if let Err(e) = evict_dir_contents(&local) {
let _ = self.db.sync_folder_set_state(id, "error", now_secs());
return Err(SwitchBlocked::Failed(format!(
"evict {}: {e}",
local.display()
)));
}
// Try mounting the secondary FUSE filesystem FIRST before touching local files.
// If mount fails, abort safely so local files are preserved intact.
let session = match self.spawn_ondemand_mount(&local, root) {
Ok(session) => session,
Err(e) => {
let _ = self.db.sync_folder_set_state(id, "error", now_secs());
return Err(SwitchBlocked::Failed(e.to_string()));
return Err(SwitchBlocked::Failed(format!(
"mount ondemand failed; local files preserved: {e}"
)));
}
};
if let Err(e) = self.db.sync_folder_set_mode(id, "ondemand") {
let _ = umount_session_unblocked(session, fuse_connection_id(&local));
return Err(SwitchBlocked::Failed(format!("db: {e:?}")));
}
// Reclaim the disk: empty the local dir now that the mount handles filesystem requests.
if let Err(e) = evict_dir_contents(&local) {
warn!(id, path = %local.display(), error = %e, "evict local dir contents failed");
}
self.mounts
.lock()
.insert(id, (session, fuse_connection_id(&local)));
+3 -2
View File
@@ -490,7 +490,7 @@ impl Core {
uid: &NodeUid,
meta: &StagedWrite,
) -> Result<Option<String>, Box<dyn std::error::Error>> {
let Some(base) = meta.based_on else {
let Some(ref base) = meta.based_on else {
return Ok(None);
};
let Some(node) = self.fetch_node_remote(uid)? else {
@@ -870,12 +870,13 @@ impl Core {
let base = Baseline {
mtime: sealed.modification_time,
size: node_size(sealed),
hash: None,
};
let mut pending = self.pending.lock();
let Some(p) = pending.get_mut(uid) else {
return false;
};
p.meta.based_on = Some(base);
p.meta.based_on = Some(base.clone());
match serde_json::to_string(&p.meta) {
Ok(json) => {
if let Err(e) = self.db.update_op_meta(&uid.to_string(), OP_REVISION, &json) {
+162 -8
View File
@@ -1808,6 +1808,11 @@ impl Core {
// staged write and a queued op, and a sidecar outliving them would offer
// recovery a second, stale copy of the same write.
self.cache.clear_scratch_durable(&h.path);
if is_local_uid(&h.uid) && !self.db.has_create_op(&h.uid.to_string()).unwrap_or(true) {
debug!(uid = %h.uid, "local node was unlinked before creation; dropping revision");
let _ = std::fs::remove_file(&h.path);
return Ok(());
}
if !h.dirty {
let _ = std::fs::remove_file(&h.path);
return Ok(());
@@ -1880,10 +1885,11 @@ impl Core {
return None;
}
match self.pending.lock().get(uid) {
Some(p) => p.meta.based_on,
Some(p) => p.meta.based_on.clone(),
None => Some(Baseline {
mtime: base_mtime,
size: base_size,
hash: None,
}),
}
}
@@ -3961,6 +3967,13 @@ impl Filesystem for ProtonFs {
});
}
fn forget(&self, _req: &Request, ino: INodeNo, nlookup: u64) {
let st = self.core.state.lock();
if let Some(entry) = st.entries.get(&ino.0) {
debug!(ino = ino.0, uid = %entry.uid, nlookup, "kernel forget message received");
}
}
fn getattr(&self, _req: &Request, ino: INodeNo, _fh: Option<FileHandle>, reply: ReplyAttr) {
let (attr, provisional_in) = {
let st = self.core.state.lock();
@@ -4448,15 +4461,25 @@ impl Filesystem for ProtonFs {
let new_end = offset.saturating_add(length);
let keep_size = (mode & 1) != 0; // FALLOC_FL_KEEP_SIZE
let is_punch_hole = (mode & 0x02) != 0; // FALLOC_FL_PUNCH_HOLE
let res = {
let mut st = self.core.state.lock();
match st.active_writes.get_mut(&ino.0) {
match st
.handles
.get(&_fh.0)
.copied()
.and_then(|i| st.active_writes.get_mut(&i))
{
Some(aw) => {
use std::os::unix::io::AsRawFd;
let fd = aw.file.as_raw_fd();
let ret = unsafe { libc::fallocate(fd, mode, offset as i64, length as i64) };
if ret == 0 {
if !keep_size && new_end > aw.len {
if is_punch_hole {
aw.written.punch_hole(offset, new_end);
aw.dirty = true;
} else if !keep_size && new_end > aw.len {
aw.written.add(aw.len, new_end);
aw.len = new_end;
aw.dirty = true;
@@ -4464,11 +4487,16 @@ impl Filesystem for ProtonFs {
}
Ok(())
} else {
// In theory we could map errno to fuser::Errno, but EIO is safe
Err(Errno::EIO)
}
}
None => Err(Errno::EBADF),
None => {
if st.entries.contains_key(&ino.0) {
Err(Errno::EBADF)
} else {
Err(Errno::ENOENT)
}
}
}
};
@@ -4608,11 +4636,34 @@ impl Filesystem for ProtonFs {
}
fn unlink(&self, _req: &Request, parent: INodeNo, name: &OsStr, reply: ReplyEmpty) {
self.trash_child(parent.0, &name.to_string_lossy(), reply);
let name_str = name.to_string_lossy();
if let Ok((ino, _)) = self.core.lookup_child(parent.0, &name_str) {
let st = self.core.state.lock();
if let Some(entry) = st.entries.get(&ino)
&& entry.node.is_folder() {
reply.error(Errno::EISDIR);
return;
}
}
self.trash_child(parent.0, &name_str, reply);
}
fn rmdir(&self, _req: &Request, parent: INodeNo, name: &OsStr, reply: ReplyEmpty) {
self.trash_child(parent.0, &name.to_string_lossy(), reply);
let name_str = name.to_string_lossy();
if let Ok((ino, _)) = self.core.lookup_child(parent.0, &name_str) {
let st = self.core.state.lock();
if let Some(entry) = st.entries.get(&ino) {
if !entry.node.is_folder() {
reply.error(Errno::ENOTDIR);
return;
}
if st.has_children(ino) {
reply.error(Errno::ENOTEMPTY);
return;
}
}
}
self.trash_child(parent.0, &name_str, reply);
}
fn mkdir(
@@ -4720,6 +4771,15 @@ impl Filesystem for ProtonFs {
return;
}
};
// Ancestor cycle check: moving a directory into itself or one of its descendants is forbidden by POSIX (EINVAL).
{
let st = self.core.state.lock();
if let Some(entry) = st.entries.get(&ino)
&& entry.node.is_folder() && st.is_ancestor_of(ino, newparent) {
reply.error(Errno::EINVAL);
return;
}
}
// The destination has to be listed either way: the queued path pushes
// the node into that listing, and the online one drops it to force a
// re-enumeration.
@@ -5544,7 +5604,10 @@ pub fn mount(
// Bind the control socket before the FUSE session takes over the thread. A
// stale socket file from a previous run would block the bind, so clear it.
let _ = std::fs::remove_file(control_socket);
let listener = UnixListener::bind(control_socket)?;
let old_umask = unsafe { libc::umask(0o77) };
let listener_res = UnixListener::bind(control_socket);
unsafe { libc::umask(old_umask) };
let listener = listener_res?;
// Owner-only before anything can connect: a peer on this socket commands the
// daemon's authenticated session without a credential of its own (B6).
if let Err(e) = pdfs_core::config::restrict_socket(control_socket) {
@@ -6142,4 +6205,95 @@ mod tests {
iv.add(old_len, new_len);
assert_eq!(segs(&iv, 0, 30), vec![(0, 30, true)]);
}
use proton_drive_rs::proton_sdk::ids::{LinkId, NodeUid, VolumeId};
use proton_drive_rs::{Node, NodeKind};
#[test]
fn test_posix_unlink_and_rmdir_checks() {
let (mut st, _dir) = state_test_helper();
let parent = st.intern(0, node_helper("parent_uid", "none", "parent", true));
let folder_child = st.intern(
parent,
node_helper("folder_child", "parent_uid", "subfolder", true),
);
let file_child = st.intern(
parent,
node_helper("file_child", "parent_uid", "file.txt", false),
);
st.children.insert(parent, vec![folder_child, file_child]);
// unlink on folder -> EISDIR check
let folder_entry = st.entries.get(&folder_child).unwrap();
assert!(folder_entry.node.is_folder(), "folder_child is a folder");
// rmdir on file -> ENOTDIR check
let file_entry = st.entries.get(&file_child).unwrap();
assert!(!file_entry.node.is_folder(), "file_child is a file");
// rmdir on non-empty parent -> ENOTEMPTY check
assert!(st.has_children(parent), "parent is not empty");
}
struct TestDir(std::path::PathBuf);
impl Drop for TestDir {
fn drop(&mut self) {
let _ = std::fs::remove_dir_all(&self.0);
}
}
fn state_test_helper() -> (crate::state::State, TestDir) {
use std::sync::atomic::{AtomicU64, Ordering};
static N: AtomicU64 = AtomicU64::new(0);
let dir_path = std::env::temp_dir().join(format!(
"pdfs-lib-test-{}-{}",
std::process::id(),
N.fetch_add(1, Ordering::Relaxed)
));
std::fs::create_dir_all(&dir_path).unwrap();
let db = pdfs_core::db::Db::open(&dir_path.join("cache.db")).unwrap();
let st = crate::state::State {
entries: std::collections::HashMap::new(),
by_uid: std::collections::HashMap::new(),
children: std::collections::HashMap::new(),
next_ino: 1,
active_writes: std::collections::HashMap::new(),
handles: std::collections::HashMap::new(),
next_fh: 1,
db: std::sync::Arc::new(db),
};
(st, TestDir(dir_path))
}
fn node_helper(id: &str, parent: &str, name: &str, is_dir: bool) -> Node {
let uid = NodeUid::new(VolumeId::from("vol"), LinkId::from(id));
let parent_uid = if parent == "none" {
None
} else {
Some(NodeUid::new(VolumeId::from("vol"), LinkId::from(parent)))
};
Node {
uid,
parent_uid,
name: name.to_string(),
kind: if is_dir {
NodeKind::Folder
} else {
NodeKind::File {
media_type: "text/plain".into(),
total_size_on_storage: 0,
active_revision_state: None,
claimed_size: Some(0),
claimed_modification_time: None,
}
},
creation_time: 100,
modification_time: 100,
trashed: false,
is_shared: false,
is_shared_publicly: false,
signature_email: None,
verification: Default::default(),
}
}
}
+124
View File
@@ -53,6 +53,27 @@ impl Intervals {
}
}
/// Punch a hole in `[start, end)`, removing authored status for this range.
pub(crate) fn punch_hole(&mut self, start: u64, end: u64) {
if start >= end {
return;
}
let mut next = Vec::new();
for &(s, e) in &self.0 {
if e <= start || s >= end {
next.push((s, e));
} else {
if s < start {
next.push((s, start));
}
if e > end {
next.push((end, e));
}
}
}
self.0 = next;
}
/// Split `[start, end)` into contiguous `(s, e, authored)` segments, in
/// order. `authored == true` means the bytes live in the scratch file;
/// `false` means they must come from the remote base (or are a hole).
@@ -199,6 +220,39 @@ impl State {
.collect()
}
/// Check if a directory inode has any child nodes in memory or in the database.
pub(crate) fn has_children(&self, parent: u64) -> bool {
if let Some(kids) = self.children.get(&parent)
&& !kids.is_empty() {
return true;
}
if let Some(entry) = self.entries.get(&parent)
&& let Ok(has_kids) = self.db.has_children(&entry.uid) {
return has_kids;
}
false
}
/// Check if `ancestor_ino` is `target_ino` or an ancestor of `target_ino` (for rename cycle prevention).
pub(crate) fn is_ancestor_of(&self, ancestor_ino: u64, mut target_ino: u64) -> bool {
if ancestor_ino == target_ino {
return true;
}
let mut visited = std::collections::HashSet::new();
visited.insert(target_ino);
while let Some(entry) = self.entries.get(&target_ino) {
let parent = entry.parent;
if parent == ancestor_ino {
return true;
}
if parent == 0 || !visited.insert(parent) {
break;
}
target_ino = parent;
}
false
}
/// Forget a node entirely: drop its inode, its uid mapping, its own cached
/// listing, its slot in its parent's listing, and its DB row. Returns
/// `(parent_ino, name)` when the node was known, so the caller can notify
@@ -497,4 +551,74 @@ mod tests {
with no way to notice anything else changed"
);
}
#[test]
fn test_intervals_punch_hole_middle() {
let mut iv = Intervals::default();
iv.add(0, 100);
iv.punch_hole(30, 50);
assert_eq!(iv.0, vec![(0, 30), (50, 100)]);
}
#[test]
fn test_intervals_punch_hole_edges() {
let mut iv = Intervals::default();
iv.add(0, 100);
iv.punch_hole(0, 20);
assert_eq!(iv.0, vec![(20, 100)]);
iv.punch_hole(80, 100);
assert_eq!(iv.0, vec![(20, 80)]);
}
#[test]
fn test_intervals_punch_hole_full() {
let mut iv = Intervals::default();
iv.add(0, 100);
iv.punch_hole(0, 100);
assert!(iv.0.is_empty());
}
#[test]
fn test_is_ancestor_of_hierarchy() {
let (mut st, _dir) = state();
let root = st.intern(0, node("root_id", "none", "root", true));
let p1 = st.intern(root, node("p1_id", "root_id", "p1", true));
let p2 = st.intern(p1, node("p2_id", "p1_id", "p2", true));
let child = st.intern(p2, node("child_id", "p2_id", "child", true));
assert!(st.is_ancestor_of(root, root), "self is ancestor");
assert!(st.is_ancestor_of(root, p1), "root is ancestor of p1");
assert!(
st.is_ancestor_of(root, child),
"root is ancestor of deep child"
);
assert!(st.is_ancestor_of(p1, child), "p1 is ancestor of deep child");
assert!(
!st.is_ancestor_of(child, root),
"child is not ancestor of root"
);
assert!(!st.is_ancestor_of(child, p1), "child is not ancestor of p1");
}
#[test]
fn test_state_has_children_mem_and_db() {
let (mut st, _dir) = state();
let folder = st.intern_mem(0, node("dir_uid", "none", "dir", true));
assert!(!st.has_children(folder), "initially empty");
// Memory-only children (using intern_mem so DB is not populated yet)
let f1 = st.intern_mem(folder, node("file1_uid", "dir_uid", "file1.txt", false));
st.children.insert(folder, vec![f1]);
assert!(st.has_children(folder), "has child in memory");
st.children.insert(folder, vec![]);
assert!(!st.has_children(folder), "cleared memory children");
// DB children
st.db
.upsert_node(&node("file2_uid", "dir_uid", "file2.txt", false))
.unwrap();
assert!(st.has_children(folder), "has child in db");
}
}