Renaming a directory that exists only in the lower layer reports success and destroys everything inside it.
handle_rename (cow/seccomp.rs:804) starts with ensure_cow_copy(&old_rel). For a directory that produces an empty directory in the upper — the copy-up is not recursive — and the following renameat_in_root(&self.upper, &old_rel, &new_rel) then moves that empty directory to the new name. The source still exists in the workdir, so mark_deleted(&old_rel) records a whiteout for it, and commit() applies the deletion. The result is a new empty directory and a deleted old one, with every file that was under it gone.
Reproducer:
let wd = workdir.path().canonicalize().unwrap();
fs::create_dir(wd.join("d")).unwrap();
fs::write(wd.join("d/inner.txt"), "PRECIOUS").unwrap();
let mut b = SeccompCowBranch::create(&wd, Some(storage.path()), 0).unwrap();
b.handle_rename(&format!("{}/d", wd.display()), &format!("{}/d2", wd.display()));
b.commit();
Observed:
rename = Ok(true)
changes = [Change { kind: Deleted, path: "d" }]
commit = Ok(())
d exists = false
d2 exists = true
d2/inner.txt = false <-- destroyed
Three things make this worse than a plain bug:
- it reports success at every step — the child gets a successful
rename(2), commit() returns Ok(()), and nothing surfaces an error;
changes() reports only Deleted "d", so neither the dry-run output nor the recovery report mentions the files that were lost;
- the whiteout is what does the destroying, so the data is gone from the workdir at commit time even though the upper never held it.
The same shape reaches handle_link (cow/seccomp.rs:872), which also calls ensure_cow_copy on a path that may be a directory.
The fix belongs in the copy-up: either make prepare_copy/ensure_cow_copy recursive for directories, or have the directory case refuse (return Ok(false) and let the kernel perform the rename on the workdir directly, which is what happens today when safe_rel rejects the path and is the behaviour that keeps the contents). The second is much cheaper and loses only the ability to stage a directory rename in the branch.
Reproduced on main at 44b1c65; found while writing COW merge tests for #148, unrelated to that PR's subject.
Note on the path form, since it cost me a false negative first time round: safe_rel uses pathdiff::diff_paths(path, workdir), so these handlers expect the absolute path as the child sees it. Passing a relative "d" makes safe_rel return None and the handler answers Ok(false) — the operation falls through to the kernel and no data is lost, which looks like the bug is absent.
Renaming a directory that exists only in the lower layer reports success and destroys everything inside it.
handle_rename(cow/seccomp.rs:804) starts withensure_cow_copy(&old_rel). For a directory that produces an empty directory in the upper — the copy-up is not recursive — and the followingrenameat_in_root(&self.upper, &old_rel, &new_rel)then moves that empty directory to the new name. The source still exists in the workdir, somark_deleted(&old_rel)records a whiteout for it, andcommit()applies the deletion. The result is a new empty directory and a deleted old one, with every file that was under it gone.Reproducer:
Observed:
Three things make this worse than a plain bug:
rename(2),commit()returnsOk(()), and nothing surfaces an error;changes()reports onlyDeleted "d", so neither the dry-run output nor the recovery report mentions the files that were lost;The same shape reaches
handle_link(cow/seccomp.rs:872), which also callsensure_cow_copyon a path that may be a directory.The fix belongs in the copy-up: either make
prepare_copy/ensure_cow_copyrecursive for directories, or have the directory case refuse (returnOk(false)and let the kernel perform the rename on the workdir directly, which is what happens today whensafe_relrejects the path and is the behaviour that keeps the contents). The second is much cheaper and loses only the ability to stage a directory rename in the branch.Reproduced on
mainat 44b1c65; found while writing COW merge tests for #148, unrelated to that PR's subject.Note on the path form, since it cost me a false negative first time round:
safe_relusespathdiff::diff_paths(path, workdir), so these handlers expect the absolute path as the child sees it. Passing a relative"d"makessafe_relreturnNoneand the handler answersOk(false)— the operation falls through to the kernel and no data is lost, which looks like the bug is absent.