diff --git a/iris.toml b/iris.toml index 43ae0ae..0f037a4 100644 --- a/iris.toml +++ b/iris.toml @@ -82,7 +82,7 @@ cdrom = false # Change this if 192.168.0.x conflicts with your local network. #nat_subnet = "192.168.0.0/24" -# NFS share — requires unfsd on the host. +# NFS share # The shared directory is exported to the VM at :/path (standard NFS port 2049). # From IRIX: mount :/absolute/path /mnt (default gateway: 192.168.0.1) [nfs] diff --git a/rules/irix/nfs-dot-and-dotdot-must-resolve-server-side.md b/rules/irix/nfs-dot-and-dotdot-must-resolve-server-side.md new file mode 100644 index 0000000..7e10dd1 --- /dev/null +++ b/rules/irix/nfs-dot-and-dotdot-must-resolve-server-side.md @@ -0,0 +1,64 @@ +# NFS LOOKUP `..` must resolve on the server — the client does not synthesize it + +## Symptom + +Absolute paths on an in-core NFS share work, relative ones don't. On the guest: + +``` +# cd /mnt/share/sub +# cd .. +..: No such file or directory +# ls ../other +../other not found +``` + +`ls -a` also never shows `.` or `..`. Once it fails, it keeps failing for the +life of the mount even after the server is fixed — the client caches the +negative lookup in its DNLC, so **testing a fix needs a fresh mount**. + +## Cause + +`src/nfsudp.rs` funnelled every LOOKUP name through `valid_component()`, which +rejects `.`, `..`, empty, and anything with a separator. That is right for +CREATE/MKDIR/REMOVE/RENAME, but wrong for LOOKUP: + +- SVR4-derived clients (IRIX's is one) short-circuit `.` in `nfslookup()` but + **send `..` over the wire** on every DNLC miss — `lookuppn()` only intercepts + `..` at a *mount root* (to cross back into the covering filesystem) and at the + process root. Everywhere else the server answers it. +- So `lookup(dirid, "..")` returned `None` → `NFS3ERR_NOENT`/`NFSERR_NOENT`, and + `chdir("..")`, `../x`, and getcwd-style walks all got ENOENT. + +`NfsBacking::readdir()` also skipped `.`/`..` with a comment saying the wire +layer would synthesize them — it never did, in either the v2 or v3 encoder. + +## Fix + +No tree structure is needed: `id_to_path` already holds the full root-relative +path for every fileid, so the parent is one `PathBuf::pop()` away. + +- `parent_id()` pops one component and interns the result; `pop()` returning + `false` means we're at the root, so `..` there yields `ROOT_ID`. That keeps + containment — `..` can never leave the export. +- `lookup()` handles `.`/`..` **before** `valid_component()`, gated on + `is_dir(dirid)`. `valid_component()` is unchanged, so the mutating procedures + still refuse both names. +- `readdir()` emits `.` and `..` first. Both wire encoders name-sort before + paging and `.` < `..` < everything else in byte order, so the index-based + READDIR cookie stays stable. + +The `..` entry must carry the **same** fileid `lookup` interns for that path — +interning is keyed on the relative path, so it does. getcwd() walks up by +matching a child's fileid against the parent's directory entries; mismatched +ids there produce a wrong `pwd` rather than an error. + +## Watch out + +- Directory `nlink` is still hardcoded to 2 (`attr_from`). Now that `.`/`..` are + real entries, a directory with subdirectories should report `2 + subdirs`. + Tools using the link-count leaf optimization (`fts`, some `find`s) treat + `nlink == 2` as "no subdirectories" and stop descending. Computing it properly + costs a `read_dir` per GETATTR, which is why it was left alone. +- `..` off the *mount root* never reaches us — the client's VFS crosses back to + the covered vnode itself. Returning `ROOT_ID` is the safe answer regardless. +- Regression test: `nfsudp::tests::dot_and_dotdot_navigate_without_escaping`. diff --git a/src/nfsudp.rs b/src/nfsudp.rs index 44fe54c..2eefe98 100644 --- a/src/nfsudp.rs +++ b/src/nfsudp.rs @@ -126,9 +126,27 @@ impl NfsBacking { Some(self.attr_from(id, &md)) } + /// The fileid of `id`'s parent, clamped at the export root so `..` can never + /// walk out of the share. + fn parent_id(&mut self, id: u64) -> Option { + let mut rel = self.rel_of(id)?.clone(); + if !rel.pop() { + return Some(ROOT_ID); + } + Some(self.intern(rel)) + } + /// Resolve `name` within directory `dirid`, interning the child and - /// returning its fileid. Rejects names that could escape the export. + /// returning its fileid. `.` and `..` resolve to the directory and its + /// parent — clients send `..` over the wire on every DNLC miss, so refusing + /// it breaks relative-path navigation. Other escaping names are rejected. pub fn lookup(&mut self, dirid: u64, name: &[u8]) -> Option { + if name == b"." || name == b".." { + if !self.is_dir(dirid) { + return None; + } + return if name == b"." { Some(dirid) } else { self.parent_id(dirid) }; + } let comp = valid_component(name)?; let rel = self.rel_of(dirid)?.join(&comp); let abs = self.root.join(&rel); @@ -138,12 +156,18 @@ impl NfsBacking { Some(self.intern(rel)) } - /// List `dirid`, returning `(name, fileid, attr)` for each entry. `.` / `..` - /// are not included (the wire layer synthesizes them if a client needs them). + /// List `dirid`, returning `(name, fileid, attr)` for each entry, `.` and + /// `..` first — POSIX readdir includes them and getcwd-style walks need the + /// `..` fileid to match what LOOKUP interns. pub fn readdir(&mut self, dirid: u64) -> Option, u64, Attr)>> { let dir_rel = self.rel_of(dirid)?.clone(); let abs = self.root.join(&dir_rel); let mut out = Vec::new(); + for (name, id) in [(b".".to_vec(), dirid), (b"..".to_vec(), self.parent_id(dirid)?)] { + if let Some(attr) = self.attr(id) { + out.push((name, id, attr)); + } + } for ent in std::fs::read_dir(&abs).ok()? { let ent = ent.ok()?; let name = name_bytes(&ent.file_name()); @@ -1498,7 +1522,42 @@ mod tests { let mut b = NfsBacking::new(&root); let mut names: Vec> = b.readdir(ROOT_ID).unwrap().into_iter().map(|(n, _, _)| n).collect(); names.sort(); - assert_eq!(names, vec![b"a".to_vec(), b"b".to_vec(), b"d".to_vec()]); + assert_eq!( + names, + vec![b".".to_vec(), b"..".to_vec(), b"a".to_vec(), b"b".to_vec(), b"d".to_vec()] + ); + std::fs::remove_dir_all(&root).ok(); + } + + #[test] + fn dot_and_dotdot_navigate_without_escaping() { + let root = temp_export(); + std::fs::create_dir_all(root.join("a/b")).unwrap(); + let mut b = NfsBacking::new(&root); + let a_id = b.lookup(ROOT_ID, b"a").unwrap(); + let ab_id = b.lookup(a_id, b"b").unwrap(); + + assert_eq!(b.lookup(ab_id, b"."), Some(ab_id)); + assert_eq!(b.lookup(ab_id, b".."), Some(a_id), "`..` is the id LOOKUP interned"); + assert_eq!(b.lookup(a_id, b".."), Some(ROOT_ID)); + assert_eq!(b.lookup(ROOT_ID, b".."), Some(ROOT_ID), "`..` stops at the export root"); + + // `..` in a listing must carry the parent's fileid — getcwd walks up by + // matching the child's fileid against the parent's entries. + let ents = b.readdir(ab_id).unwrap(); + for (_, id, at) in &ents { + assert_eq!(at.fileid, *id, "entry fileid must match its attr"); + } + let named: Vec<(Vec, u64)> = ents.into_iter().map(|(n, id, _)| (n, id)).collect(); + assert!(named.contains(&(b".".to_vec(), ab_id))); + assert!(named.contains(&(b"..".to_vec(), a_id))); + + // A non-directory has neither, and the mutating ops still refuse both. + let f = b.create(ROOT_ID, b"f").unwrap(); + assert!(b.lookup(f, b".").is_none()); + assert!(b.mkdir(ROOT_ID, b"..").is_none()); + assert!(!b.rmdir(ROOT_ID, b"..")); + assert!(!b.rename(ROOT_ID, b"..", ROOT_ID, b"z")); std::fs::remove_dir_all(&root).ok(); } @@ -1527,10 +1586,11 @@ mod tests { assert!(valid_component(b"a\\b").is_none()); assert!(valid_component(b"ok.txt").is_some()); - // lookup must refuse to escape the export root. + // lookup must refuse to escape the export root: `..` at the root is the + // root itself, and no name resolves through a separator. let root = temp_export(); let mut b = NfsBacking::new(&root); - assert!(b.lookup(ROOT_ID, b"..").is_none()); + assert_eq!(b.lookup(ROOT_ID, b".."), Some(ROOT_ID)); assert!(b.lookup(ROOT_ID, b"../etc").is_none()); std::fs::remove_dir_all(&root).ok(); } @@ -1887,7 +1947,7 @@ mod tests { assert!(pages > 1, "the listing should have needed multiple pages"); all.sort(); all.dedup(); - assert_eq!(all.len(), n, "every entry returned exactly once across pages"); + assert_eq!(all.len(), n + 2, "every entry (plus . and ..) returned exactly once"); std::fs::remove_dir_all(&root).ok(); }