Skip to content

Commit acb88ef

Browse files
Kranzesjiangliu
authored andcommitted
overlayfs: count readdirplus lookups only for linked entries
do_readdir() took a lookup of every READDIRPLUS entry before it called the reply callback, "." and ".." included. The kernel skips the dots without linking them, and an entry the callback rejects never reaches the kernel, so neither gets a FORGET. Every listing from the start leaked a lookup of the directory and its parent, and every reply that filled its buffer leaked one more. Count a lookup once the callback has accepted an entry, and not for the dots. Also return the entries already in the buffer when stat fails on a later one, as for a callback error, because an error reply drops them along with their lookups. Signed-off-by: Ilan Joselevich <personal@ilanjoselevich.com>
1 parent 62b169f commit acb88ef

2 files changed

Lines changed: 121 additions & 2 deletions

File tree

‎crates/fuse-backend-overlayfs/Cargo.toml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,9 @@ log = "0.4.6"
2424
radix_trie = "0.2.1"
2525
vmm-sys-util = { workspace = true }
2626

27+
[dev-dependencies]
28+
fuse-backend-passthrough = { path = "../fuse-backend-passthrough", version = "0.14.0" }
29+
2730
[features]
2831
default = []
2932
async-io = ["fuse-backend-core/async-io"]

‎crates/fuse-backend-overlayfs/src/lib.rs‎

Lines changed: 118 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1446,8 +1446,14 @@ impl OverlayFs {
14461446

14471447
for (index, (name, child)) in (0_u64..).zip(childrens) {
14481448
if index >= offset {
1449+
// An error drops the reply, so once entries are in the
1450+
// buffer, return them instead.
1451+
let st = match child.stat64(ctx) {
1452+
Ok(st) => st,
1453+
Err(_) if len > 0 => return Ok(()),
1454+
Err(e) => return Err(e),
1455+
};
14491456
// make struct DireEntry and Entry
1450-
let st = child.stat64(ctx)?;
14511457
let dir_entry = DirEntry {
14521458
ino: st.st_ino,
14531459
offset: index + 1,
@@ -1456,7 +1462,6 @@ impl OverlayFs {
14561462
};
14571463

14581464
let entry = if is_readdirplus {
1459-
child.lookups.fetch_add(1, Ordering::Relaxed);
14601465
Some(Entry {
14611466
inode: child.inode,
14621467
generation: 0,
@@ -1471,6 +1476,13 @@ impl OverlayFs {
14711476
match add_entry(dir_entry, entry) {
14721477
Ok(0) => break,
14731478
Ok(l) => {
1479+
// The kernel links each READDIRPLUS entry in the
1480+
// reply and forgets it later, except "." and "..",
1481+
// which it skips. The reply goes out after this
1482+
// returns, so counting here can't race the forget.
1483+
if is_readdirplus && name != CURRENT_DIR && name != PARENT_DIR {
1484+
child.lookups.fetch_add(1, Ordering::Relaxed);
1485+
}
14741486
len += l;
14751487
if len as u32 >= size {
14761488
// no more space, stop here
@@ -2806,6 +2818,8 @@ impl BackendFileSystem for OverlayFs {
28062818
#[cfg(test)]
28072819
mod tests {
28082820
use super::*;
2821+
use fuse_backend_core::api::filesystem::FileSystem;
2822+
use vmm_sys_util::tempdir::TempDir;
28092823

28102824
fn stat_with(mode: u32, nlink: u64) -> stat64 {
28112825
// Safe: an all-zero stat64 is a valid stat of a hidden file.
@@ -2830,4 +2844,106 @@ mod tests {
28302844
// Symlinks can be hard linked too.
28312845
assert!(is_multi_link_file(stat_with(libc::S_IFLNK | 0o777, 2)));
28322846
}
2847+
2848+
/// An overlay of an empty upper layer over a lower layer with a
2849+
/// directory "d" of four files.
2850+
fn readdirplus_fs() -> (OverlayFs, TempDir, TempDir) {
2851+
use fuse_backend_passthrough::{Config as PassthroughConfig, PassthroughFs};
2852+
2853+
let lower = TempDir::new().unwrap();
2854+
let upper = TempDir::new().unwrap();
2855+
let dir = lower.as_path().join("d");
2856+
std::fs::create_dir(&dir).unwrap();
2857+
for i in 0..4 {
2858+
std::fs::File::create(dir.join(format!("f{}", i))).unwrap();
2859+
}
2860+
let layer = |root: &TempDir| -> Arc<BoxedLayer> {
2861+
let fs = PassthroughFs::<()>::new(PassthroughConfig {
2862+
root_dir: root.as_path().to_str().unwrap().to_string(),
2863+
xattr: true,
2864+
..Default::default()
2865+
})
2866+
.unwrap();
2867+
fs.import().unwrap();
2868+
Arc::new(Box::new(fs))
2869+
};
2870+
let fs = OverlayFs::new(
2871+
Some(layer(&upper)),
2872+
vec![layer(&lower)],
2873+
Config {
2874+
do_import: true,
2875+
..Default::default()
2876+
},
2877+
)
2878+
.unwrap();
2879+
fs.import().unwrap();
2880+
(fs, lower, upper)
2881+
}
2882+
2883+
/// The lookup counts of the root, of `dir` and of its children.
2884+
fn lookup_counts(fs: &OverlayFs, dir: Inode) -> Vec<(String, u64)> {
2885+
let node = fs.get_all_inode(dir).unwrap();
2886+
let mut counts = vec![
2887+
(
2888+
"/".to_string(),
2889+
fs.root_node().lookups.load(Ordering::Relaxed),
2890+
),
2891+
(".".to_string(), node.lookups.load(Ordering::Relaxed)),
2892+
];
2893+
for (name, child) in node.childrens.lock().unwrap().iter() {
2894+
counts.push((name.clone(), child.lookups.load(Ordering::Relaxed)));
2895+
}
2896+
counts.sort();
2897+
counts
2898+
}
2899+
2900+
// READDIRPLUS takes a lookup of each entry it delivers, which the kernel
2901+
// forgets later. It takes none of "." and "..", which the kernel never
2902+
// forgets, and none of an entry that doesn't make it into the reply.
2903+
#[test]
2904+
fn test_readdirplus_lookups() {
2905+
let (fs, _lower, _upper) = readdirplus_fs();
2906+
let ctx = Context::default();
2907+
let name = CString::new("d").unwrap();
2908+
let dir = fs.lookup(&ctx, FUSE_ROOT_ID, &name).unwrap().inode;
2909+
let handle = fs.opendir(&ctx, dir, 0).unwrap().0.unwrap();
2910+
let before = lookup_counts(&fs, dir);
2911+
let is_dot = |name: &[u8]| name == b"." || name == b"..";
2912+
2913+
// The buffer fills up on the first entry after the dots.
2914+
fs.readdirplus(&ctx, dir, handle, 8192, 0, &mut |e, _| {
2915+
Ok(if is_dot(e.name) { 1 } else { 0 })
2916+
})
2917+
.unwrap();
2918+
assert_eq!(lookup_counts(&fs, dir), before);
2919+
2920+
// The callback fails on the first entry after the dots.
2921+
let err = fs
2922+
.readdirplus(&ctx, dir, handle, 8192, 2, &mut |_, _| {
2923+
Err(Error::from_raw_os_error(libc::EIO))
2924+
})
2925+
.unwrap_err();
2926+
assert_eq!(err.raw_os_error(), Some(libc::EIO));
2927+
assert_eq!(lookup_counts(&fs, dir), before);
2928+
2929+
// A full listing takes one lookup of each child, and none of the
2930+
// dots.
2931+
let mut delivered = 0;
2932+
fs.readdirplus(&ctx, dir, handle, 8192, 0, &mut |e, _| {
2933+
delivered += 1;
2934+
Ok(e.name.len())
2935+
})
2936+
.unwrap();
2937+
assert_eq!(delivered, 6);
2938+
let expected: Vec<_> = before
2939+
.iter()
2940+
.map(|(name, n)| match name.as_str() {
2941+
"/" | "." => (name.clone(), *n),
2942+
_ => (name.clone(), n + 1),
2943+
})
2944+
.collect();
2945+
assert_eq!(lookup_counts(&fs, dir), expected);
2946+
2947+
fs.releasedir(&ctx, dir, 0, handle).unwrap();
2948+
}
28332949
}

0 commit comments

Comments
 (0)