[GH-ISSUE #81] kernel: mapfs vm_info lifetime holes left after #78 (unlocked holders, vmp_put free window, forced-reclaim leak) #74

New issue

Closed

opened 2026-11-03 22:03:48 -07:00 by gitea-mirror · 1 comment

gitea-mirror commented

2026-11-03 22:03:48 -07:00

Owner

Originally created by @raynorpat on GitHub (Oct 30, 2026).
Original GitHub issue: https://github.com/RhapsodiOS/RhapsodiOS/issues/81

Originally assigned to: @raynorpat on GitHub.

Follow-ups from #78 (fixes 387d97d67 and dc5864824, merged into ffs-perf in 3d017f064). All three are in src/kernel-7/kern/mapfs.c.

1. Holders that don't hold the vnode lock can touch a freed vm_info

A forced unmount's vm_info_free can run while these still use the entry:

mapfs_sync, between vm_info_unlock() and vmp_get(vmp) (~1590). It holds no use count yet, so vm_info_free sees use_count == 0 and frees it.
unmap_vnode across its blocking VOP_GETATTR and push (~509-548).
exec's unmap_vnode on a NULL v_vm_info.
vmp_invalidate running without a use count.

2. The last `vmp_put` can be freed under it

vmp_put drops use_count to 0 under vm_info_lock, unlocks, and then calls lock_clear_recursive/lock_write_done on vmp->lock. If vm_info_free takes vm_info_lock in that gap, it sees use_count == 0, doesn't mark the entry orphaned, and zfrees it. The putter then writes to freed memory.

Waiting on vmp->lock in vm_info_free isn't obviously safe. remap_vnode's mapfs_memfree(vmp1, TRUE) can hold another file's vmp->lock across a push that waits for that file's vnode lock, which the reclaiming thread holds.

3. A forced reclaim of an open file leaks

When map_count > 0, mapfs_uncache skips mapfs_memfree. vm_info_free then frees (or orphans) the vm_info but leaves its mfs map window and VM object reference behind. This is unchanged from the stock kernel.

Originally created by @raynorpat on GitHub (Oct 30, 2026). Original GitHub issue: https://github.com/RhapsodiOS/RhapsodiOS/issues/81 Originally assigned to: @raynorpat on GitHub. Follow-ups from #78 (fixes 387d97d67 and dc5864824, merged into ffs-perf in 3d017f064). All three are in `src/kernel-7/kern/mapfs.c`. ## 1. Holders that don't hold the vnode lock can touch a freed vm_info A forced unmount's `vm_info_free` can run while these still use the entry: - `mapfs_sync`, between `vm_info_unlock()` and `vmp_get(vmp)` (~1590). It holds no use count yet, so `vm_info_free` sees `use_count == 0` and frees it. - `unmap_vnode` across its blocking `VOP_GETATTR` and push (~509-548). - exec's `unmap_vnode` on a NULL `v_vm_info`. - `vmp_invalidate` running without a use count. ## 2. The last `vmp_put` can be freed under it `vmp_put` drops `use_count` to 0 under `vm_info_lock`, unlocks, and then calls `lock_clear_recursive`/`lock_write_done` on `vmp->lock`. If `vm_info_free` takes `vm_info_lock` in that gap, it sees `use_count == 0`, doesn't mark the entry orphaned, and `zfree`s it. The putter then writes to freed memory. Waiting on `vmp->lock` in `vm_info_free` isn't obviously safe. `remap_vnode`'s `mapfs_memfree(vmp1, TRUE)` can hold another file's `vmp->lock` across a push that waits for that file's vnode lock, which the reclaiming thread holds. ## 3. A forced reclaim of an open file leaks When `map_count > 0`, `mapfs_uncache` skips `mapfs_memfree`. `vm_info_free` then frees (or orphans) the vm_info but leaves its mfs map window and VM object reference behind. This is unchanged from the stock kernel.

gitea-mirror

2026-11-03 22:03:48 -07:00

closed this issue
added the
bug
label

gitea-mirror commented

2026-11-03 22:03:50 -07:00

Author

Owner

@raynorpat commented on GitHub (Oct 30, 2026):

Fixed by e8fac53e3 (plus b545c56ad, a comment), merged into ffs-perf in a5e15760e.

Lifetime rule: a thread may use a vm_info only while it holds one of two things:

The vnode lock. vclean drains it before vm_info_free.
A pin, a use_count taken under vm_info_lock (vmp_pin). It must be taken from v_vm_info before the thread first blocks, or from vm_info_queue before vm_info_lock is dropped.

Under vm_info_lock, vm_info_free dequeues the entry and clears v_vm_info. If nobody holds a pin, it frees the entry now through vm_info_release (window, object reference, cred, zfree). Otherwise it marks it orphan, and the last vmp_unpin, which is that caller's last touch, frees it. vmp_put now unlocks vmp->lock first and unpins last, which closes item 2.

Audit. These broke the rule and are fixed (mapfs.c lines from 3d017f064):

mapfs_sync 1588-1591
unmap_vnode 481 and 508-550
vmp_put 871-894
remap_vnode 570 and 605-621
mapfs_cache_trim 1036-1041
mapfs_cache_clear 1061-1063
mapfs_window_free 1011-1014
mapfs_io 1292-1568
vm_info_free 337
mapfs_fsync 1635
vmp_push_range 2034

The rest hold the vnode lock.

Why it can't deadlock:

vclean never waits for a pin or for vmp->lock. vm_info_free takes only vm_info_lock, mapfs_invalidate defers on a pinned entry, and mapfs_fsync is skipped under VXLOCK.
Pins never wait.
vm_info_release never pushes.

The VXLOCK skip in mapfs_fsync applies only to a forced reclaim of an active vnode. A mapped vnode's pager holds a vnode reference (vnode_pager.c :616), and getnewvnode panics on a free vnode with a non-zero count (vfs_subr.c :455), so an ordinary recycle never reaches it mapped. On the stock kernel a push there slept forever in vn_lock (vfs_vnops.c :545-549) and wrote nothing. Unpushed pages of a forced reclaim are dropped, and writers get EIO.

Verified:

Probes, base + probe vs fix + probe:
- mapfs_sync gap, unmap_vnode across GETATTR, vmp_put tail: panic "freed vm_info" vs no panic.
- exec: panic on NULL v_vm_info vs skipped (the exec trap that followed is #84).
- A dirty open file under umount -f hung in vclean → mapfs_fsync → vn_lock on its own VXLOCK, vs rc=0 ×3.
Leak, 20 forced unmounts of an open file: base +1 128K window, +1 vm_object, +1 vnode each; fix flat.
Data integrity: 4×800 files written by write() plus shared-mmap rewrites, no fsync, 3200 files against desiredvnodes 1328, forced recycling, remount. ok=3200, bad=0 on both stock and fix, fsck clean.
Clean kernel (cksum 4162067079): rp.sh ×3 and rp2.sh ×3 rc=0. check-fs, check-mmap, check-vm pass on UFS and NFS. Two all-category five-filesystem runs: no panic, NFS plain unmount clean, ext2/hfs/msdos fsck clean.

Not caused by this change (stock behaviour, reproduced on stock):

msync returns EIO under vnode-lock contention: vnode_pageout takes the lock with LK_NOWAIT and retries once (vnode_pager.c :897-918). 246/297 failures on stock, 250/291 with the fix.
The loopback NFS client-vnode recycle deadlocks in vinvalbuf → nfsfsync.

Left open: nfs_subs.c 1405-1427 and 1488-1511 call memfree without a pin. An object is kept if a pageout is in flight at the reclaim.

@raynorpat commented on GitHub (Oct 30, 2026): Fixed by e8fac53e3 (plus b545c56ad, a comment), merged into ffs-perf in a5e15760e. **Lifetime rule:** a thread may use a vm_info only while it holds one of two things: - **The vnode lock.** vclean drains it before `vm_info_free`. - **A pin**, a `use_count` taken under `vm_info_lock` (`vmp_pin`). It must be taken from `v_vm_info` before the thread first blocks, or from `vm_info_queue` before `vm_info_lock` is dropped. Under `vm_info_lock`, `vm_info_free` dequeues the entry and clears `v_vm_info`. If nobody holds a pin, it frees the entry now through `vm_info_release` (window, object reference, cred, zfree). Otherwise it marks it orphan, and the last `vmp_unpin`, which is that caller's last touch, frees it. `vmp_put` now unlocks `vmp->lock` first and unpins last, which closes item 2. **Audit.** These broke the rule and are fixed (`mapfs.c` lines from 3d017f064): - `mapfs_sync` 1588-1591 - `unmap_vnode` 481 and 508-550 - `vmp_put` 871-894 - `remap_vnode` 570 and 605-621 - `mapfs_cache_trim` 1036-1041 - `mapfs_cache_clear` 1061-1063 - `mapfs_window_free` 1011-1014 - `mapfs_io` 1292-1568 - `vm_info_free` 337 - `mapfs_fsync` 1635 - `vmp_push_range` 2034 The rest hold the vnode lock. **Why it can't deadlock:** - vclean never waits for a pin or for `vmp->lock`. `vm_info_free` takes only `vm_info_lock`, `mapfs_invalidate` defers on a pinned entry, and `mapfs_fsync` is skipped under VXLOCK. - Pins never wait. - `vm_info_release` never pushes. **The VXLOCK skip in `mapfs_fsync`** applies only to a forced reclaim of an active vnode. A mapped vnode's pager holds a vnode reference (`vnode_pager.c` :616), and `getnewvnode` panics on a free vnode with a non-zero count (`vfs_subr.c` :455), so an ordinary recycle never reaches it mapped. On the stock kernel a push there slept forever in `vn_lock` (`vfs_vnops.c` :545-549) and wrote nothing. Unpushed pages of a forced reclaim are dropped, and writers get EIO. **Verified:** - **Probes, base + probe vs fix + probe:** - mapfs_sync gap, `unmap_vnode` across GETATTR, `vmp_put` tail: panic "freed vm_info" vs no panic. - exec: panic on NULL `v_vm_info` vs skipped (the exec trap that followed is #84). - A dirty open file under `umount -f` hung in vclean → `mapfs_fsync` → `vn_lock` on its own VXLOCK, vs rc=0 ×3. - **Leak, 20 forced unmounts of an open file:** base +1 128K window, +1 vm_object, +1 vnode each; fix flat. - **Data integrity:** 4×800 files written by `write()` plus shared-mmap rewrites, no fsync, 3200 files against desiredvnodes 1328, forced recycling, remount. ok=3200, bad=0 on both stock and fix, fsck clean. - **Clean kernel** (cksum 4162067079): `rp.sh` ×3 and `rp2.sh` ×3 rc=0. `check-fs`, `check-mmap`, `check-vm` pass on UFS and NFS. Two all-category five-filesystem runs: no panic, NFS plain unmount clean, ext2/hfs/msdos fsck clean. **Not caused by this change** (stock behaviour, reproduced on stock): - `msync` returns EIO under vnode-lock contention: `vnode_pageout` takes the lock with LK_NOWAIT and retries once (`vnode_pager.c` :897-918). 246/297 failures on stock, 250/291 with the fix. - The loopback NFS client-vnode recycle deadlocks in `vinvalbuf` → `nfsfsync`. **Left open:** `nfs_subs.c` 1405-1427 and 1488-1511 call memfree without a pin. An object is kept if a pageout is in flight at the reclaim.