Skip to content

Commit 3b9342b

Browse files
committed
fix(nfs): derive filehandle generation from mount source
The default nfsserve `id_to_fh` embeds `SystemTime::now()` at startup as the generation number, so every restart invalidates every previously- issued handle. macOS NFS client caches filehandles across `umount` and remount of the same mount point, so after `umount → kill nfs server → restart → mount`, the kernel keeps using the cached handles. The server returns NFS3ERR_STALE, and the macOS client *silently drops the write* (WRITE RPC never reaches the server, dd reports success, an explicit fsync(2) on the file returns ESTALE). Symptom in practice: opening an existing file for write after a remount on macOS appears to succeed but the data never lands in the bucket. Override `id_to_fh` / `fh_to_id` on NFSAdapter to use a generation number derived from the mount source identifier (`bucket/<id>` or `<type>/<repo>/<rev>`) via FNV-1a. Same source → same gen across restarts, so cached client handles stay valid. Different sources → different gens, so a handle from bucket A is properly rejected when mounting bucket B. Std's `DefaultHasher` is per-process randomized, which would defeat the whole point — hence the inline FNV-1a (no new deps). Tests cover: determinism, cross-source rejection, malformed handles, and the simulated-restart round-trip that locks in the regression fix.
1 parent 91b959d commit 3b9342b

2 files changed

Lines changed: 178 additions & 1 deletion

File tree

src/nfs.rs

Lines changed: 170 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use std::time::{SystemTime, UNIX_EPOCH};
55

66
use async_trait::async_trait;
77
use nfsserve::nfs::{
8-
fattr3, fileid3, filename3, ftype3, nfspath3, nfsstat3, nfsstring, nfstime3, sattr3, set_atime, set_gid3,
8+
fattr3, fileid3, filename3, ftype3, nfs_fh3, nfspath3, nfsstat3, nfsstring, nfstime3, sattr3, set_atime, set_gid3,
99
set_mode3, set_mtime, set_size3, set_uid3, specdata3,
1010
};
1111
use nfsserve::tcp::{NFSTcp, NFSTcpListener};
@@ -22,14 +22,34 @@ pub struct NFSAdapter {
2222
virtual_fs: Arc<VirtualFs>,
2323
handle_pool: Arc<Mutex<HandlePool>>,
2424
read_only: bool,
25+
/// Stable generation number embedded in every NFS filehandle this adapter
26+
/// hands out. Derived from the mount source identifier (FNV-1a hash of
27+
/// `bucket/<id>` or `<type>/<repo>/<rev>`) so it survives process
28+
/// restarts. See `id_to_fh` for the why.
29+
fh_gen: u64,
30+
}
31+
32+
/// FNV-1a 64-bit. Deterministic across processes and platforms (`std`'s
33+
/// `DefaultHasher` is randomized per-process, which defeats the whole point
34+
/// here). Quality is fine for what we use it for: an opaque tag attached to
35+
/// every filehandle.
36+
fn fnv1a_64(bytes: &[u8]) -> u64 {
37+
let mut hash: u64 = 0xcbf29ce484222325;
38+
for &b in bytes {
39+
hash ^= u64::from(b);
40+
hash = hash.wrapping_mul(0x100000001b3);
41+
}
42+
hash
2543
}
2644

2745
impl NFSAdapter {
2846
pub fn new(virtual_fs: Arc<VirtualFs>, read_only: bool) -> Self {
47+
let fh_gen = fnv1a_64(virtual_fs.source_identifier().as_bytes());
2948
Self {
3049
virtual_fs,
3150
handle_pool: Arc::new(Mutex::new(HandlePool::new())),
3251
read_only,
52+
fh_gen,
3353
}
3454
}
3555

@@ -147,6 +167,60 @@ impl NFSFileSystem for NFSAdapter {
147167
}
148168
}
149169

170+
/// Mint an NFS filehandle with a source-derived generation number.
171+
///
172+
/// The default `nfsserve` implementation embeds `SystemTime::now()` at
173+
/// server startup, which means every restart invalidates every handle the
174+
/// client previously cached. That's a problem on macOS (and to a lesser
175+
/// extent Linux): the kernel NFS client keeps filehandles alive across
176+
/// `umount` + remount on the same mountpoint as an attribute-cache
177+
/// optimization. When the server comes back with a fresh generation
178+
/// number, those cached handles get NFS3ERR_STALE on the next operation —
179+
/// and at least the macOS client *silently discards pending writes* on
180+
/// the stale handle (the WRITE RPC is buffered, then dropped without an
181+
/// error reaching userspace).
182+
///
183+
/// Reproducer on macOS:
184+
/// 1. `hf-mount-nfs bucket X/y /tmp/m` → write a file → `umount /tmp/m`
185+
/// 2. Restart `hf-mount-nfs` on the same `/tmp/m`, mount again
186+
/// 3. `dd of=/tmp/m/existing-file ...` reports success, but no WRITE RPC
187+
/// ever reaches the server; an explicit `fsync(2)` returns ESTALE.
188+
///
189+
/// Fix: derive the generation number from the *mount source*
190+
/// (`bucket/<id>` or `<type>/<repo>/<rev>`) rather than the start time.
191+
/// The same mount target hands out the same generation across restarts,
192+
/// so cached client handles stay valid. Different sources produce
193+
/// different generations, so a handle from `bucket A` is correctly
194+
/// rejected when later mounting `bucket B`.
195+
///
196+
/// Trade-off: inode numbers within a source are not guaranteed stable
197+
/// across restarts (hf-mount allocates them in tree-listing order). If
198+
/// the listing changes between mounts, a client cached handle may map to
199+
/// a *different* file than it expected. The risk is bounded — clients
200+
/// re-LOOKUP on cache miss — but it's a slight reduction in safety
201+
/// compared to upstream's strict "every handle expires on restart"
202+
/// stance. We accept it because silent write loss is worse.
203+
fn id_to_fh(&self, id: fileid3) -> nfs_fh3 {
204+
let mut data = Vec::with_capacity(16);
205+
data.extend_from_slice(&self.fh_gen.to_le_bytes());
206+
data.extend_from_slice(&id.to_le_bytes());
207+
nfs_fh3 { data }
208+
}
209+
210+
fn fh_to_id(&self, fh: &nfs_fh3) -> Result<fileid3, nfsstat3> {
211+
if fh.data.len() != 16 {
212+
return Err(nfsstat3::NFS3ERR_BADHANDLE);
213+
}
214+
let gen_bytes = u64::from_le_bytes(fh.data[0..8].try_into().expect("checked len above"));
215+
if gen_bytes != self.fh_gen {
216+
// Different source (or pre-fix server build): handle isn't ours.
217+
return Err(nfsstat3::NFS3ERR_BADHANDLE);
218+
}
219+
Ok(u64::from_le_bytes(
220+
fh.data[8..16].try_into().expect("checked len above"),
221+
))
222+
}
223+
150224
async fn lookup(&self, dirid: fileid3, filename: &filename3) -> Result<fileid3, nfsstat3> {
151225
let name = nfs_name(filename).map_err(|_| nfsstat3::NFS3ERR_NOENT)?;
152226
self.virtual_fs
@@ -1027,6 +1101,101 @@ mod tests {
10271101
assert!(pool.handles.len() <= HANDLE_POOL_CAPACITY + 1);
10281102
}
10291103

1104+
// ── filehandle stability across server restarts ─────────────────────
1105+
1106+
/// FNV-1a is deterministic — same input, same output. The whole point of
1107+
/// using it over `std::hash::DefaultHasher` is that `DefaultHasher`
1108+
/// randomizes per-process, which would defeat the cross-restart fix.
1109+
#[test]
1110+
fn fnv1a_is_deterministic_for_same_input() {
1111+
assert_eq!(
1112+
fnv1a_64(b"bucket/XciD/hf-mount-test"),
1113+
fnv1a_64(b"bucket/XciD/hf-mount-test")
1114+
);
1115+
assert_eq!(fnv1a_64(b""), fnv1a_64(b""));
1116+
// Known-good vector for FNV-1a 64 of the empty string (the basis).
1117+
// Locks the implementation against accidental algorithm changes.
1118+
assert_eq!(fnv1a_64(b""), 0xcbf29ce484222325);
1119+
}
1120+
1121+
#[test]
1122+
fn fnv1a_distinguishes_sources() {
1123+
let a = fnv1a_64(b"bucket/XciD/hf-mount-test");
1124+
let b = fnv1a_64(b"bucket/XciD/other-bucket");
1125+
let c = fnv1a_64(b"model/gpt2/main");
1126+
assert_ne!(a, b);
1127+
assert_ne!(a, c);
1128+
assert_ne!(b, c);
1129+
}
1130+
1131+
/// Build the same NFS filehandle layout `id_to_fh` produces, manually.
1132+
/// This isolates the byte-layout/round-trip logic from the need to
1133+
/// construct a full `NFSAdapter` (which would require a real
1134+
/// `VirtualFs`).
1135+
fn build_fh(generation: u64, id: fileid3) -> nfs_fh3 {
1136+
let mut data = Vec::with_capacity(16);
1137+
data.extend_from_slice(&generation.to_le_bytes());
1138+
data.extend_from_slice(&id.to_le_bytes());
1139+
nfs_fh3 { data }
1140+
}
1141+
1142+
fn parse_fh(adapter_gen: u64, fh: &nfs_fh3) -> Result<fileid3, nfsstat3> {
1143+
if fh.data.len() != 16 {
1144+
return Err(nfsstat3::NFS3ERR_BADHANDLE);
1145+
}
1146+
let g = u64::from_le_bytes(fh.data[0..8].try_into().unwrap());
1147+
if g != adapter_gen {
1148+
return Err(nfsstat3::NFS3ERR_BADHANDLE);
1149+
}
1150+
Ok(u64::from_le_bytes(fh.data[8..16].try_into().unwrap()))
1151+
}
1152+
1153+
/// Two adapters built for the *same* mount source must accept each
1154+
/// other's filehandles. This is the property that makes
1155+
/// `umount + kill nfs server + restart nfs server + remount` not
1156+
/// silently drop writes on the kernel NFS client (specifically: macOS,
1157+
/// which caches filehandles across umount of the same mount point).
1158+
///
1159+
/// Regression: pre-fix, `nfsserve`'s default `id_to_fh` embeds
1160+
/// `SystemTime::now()` at server startup, so the gen differs every
1161+
/// restart and the kernel's cached handles end up STALE.
1162+
#[test]
1163+
fn filehandle_survives_simulated_server_restart() {
1164+
let source = "bucket/XciD/hf-mount-test";
1165+
// Server lifetime #1: mint a handle for fileid 42.
1166+
let gen_run_1 = fnv1a_64(source.as_bytes());
1167+
let h_run_1 = build_fh(gen_run_1, 42);
1168+
1169+
// Server lifetime #2: same source → same gen.
1170+
let gen_run_2 = fnv1a_64(source.as_bytes());
1171+
assert_eq!(gen_run_1, gen_run_2);
1172+
1173+
// The handle minted by run #1 round-trips against run #2.
1174+
assert_eq!(parse_fh(gen_run_2, &h_run_1).expect("handle must round-trip"), 42);
1175+
}
1176+
1177+
/// Cross-source handles must still be rejected — a handle minted by
1178+
/// bucket A must not silently decode as a valid id when later mounted on
1179+
/// bucket B (otherwise stale client cache could leak across unrelated
1180+
/// mounts).
1181+
#[test]
1182+
fn filehandle_rejected_when_source_differs() {
1183+
let gen_a = fnv1a_64(b"bucket/XciD/hf-mount-test");
1184+
let gen_b = fnv1a_64(b"bucket/XciD/other-bucket");
1185+
let h_from_a = build_fh(gen_a, 42);
1186+
assert!(matches!(parse_fh(gen_b, &h_from_a), Err(nfsstat3::NFS3ERR_BADHANDLE)));
1187+
}
1188+
1189+
/// Malformed handles (wrong length) are rejected.
1190+
#[test]
1191+
fn filehandle_rejects_wrong_length() {
1192+
let generation = fnv1a_64(b"bucket/x/y");
1193+
let short = nfs_fh3 { data: vec![0u8; 8] };
1194+
let long = nfs_fh3 { data: vec![0u8; 32] };
1195+
assert!(matches!(parse_fh(generation, &short), Err(nfsstat3::NFS3ERR_BADHANDLE)));
1196+
assert!(matches!(parse_fh(generation, &long), Err(nfsstat3::NFS3ERR_BADHANDLE)));
1197+
}
1198+
10301199
#[test]
10311200
fn handle_pool_acquire_shared_stacks_pins() {
10321201
let mut pool = HandlePool::new();

src/virtual_fs/mod.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1436,6 +1436,14 @@ impl VirtualFs {
14361436
}
14371437
}
14381438

1439+
/// Stable identifier for the mount source ("bucket/<id>" or "<type>/<repo>/<rev>").
1440+
/// Used by the NFS adapter to derive a generation number that survives
1441+
/// process restarts (so cached client filehandles don't go STALE across
1442+
/// `umount`+remount on the same mount point — see nfs.rs).
1443+
pub fn source_identifier(&self) -> String {
1444+
self.hub_client.source().to_string()
1445+
}
1446+
14391447
pub fn default_uid(&self) -> u32 {
14401448
self.uid
14411449
}

0 commit comments

Comments
 (0)