[GH-ISSUE #75] fifofs: fifo_close leaks the fifoinfo and its two sockets when the last close sees v_usecount > 1 #68

New issue

Closed

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

gitea-mirror commented

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

Owner

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

Symptom

A fifo's struct fifoinfo and its two sockets are never freed if, at the last close, something else holds a reference on the vnode that isn't an open (for example a concurrent lookup or stat). Found by the #68 investigation.

Cause

fifo_close (bsd/miscfs/fifofs/fifo_vnops.c ~443-461) decrements fi_readers/fi_writers, but it frees the fifoinfo only when vp->v_usecount <= 1. If a non-open reference makes it > 1, the function returns early and v_fifoinfo stays allocated with no readers or writers.
Nothing frees it later:
- fifo_inactive only unlocks the vnode.
- fifo_reclaim is nullop.
- The filesystems' fifo op tables (for example ffs ffs_fifoop_entries) reclaim through ffs_reclaim, which doesn't know about v_fifoinfo.
So when the vnode is recycled, the fifoinfo and two sockets are lost. The stale v_un also goes with the vnode, which is the field #67 tripped over.

Fix direction

Free the fifoinfo when the last reader and writer are gone (fi_readers == 0 && fi_writers == 0), not based on v_usecount. Later BSDs do this.
Make sure the revoke/vclean close path (VOP_CLOSE with neither FREAD nor FWRITE) can't leave it behind either, for every filesystem that uses fifofs (ufs/ffs, ext2fs, hfs, nfs specfs/fifo tables).

Originally created by @raynorpat on GitHub (Oct 20, 2026). Original GitHub issue: https://github.com/RhapsodiOS/RhapsodiOS/issues/75 ## Symptom A fifo's `struct fifoinfo` and its two sockets are never freed if, at the last `close`, something else holds a reference on the vnode that isn't an open (for example a concurrent lookup or `stat`). Found by the #68 investigation. ## Cause - `fifo_close` (`bsd/miscfs/fifofs/fifo_vnops.c` ~443-461) decrements `fi_readers`/`fi_writers`, but it frees the fifoinfo only when `vp->v_usecount <= 1`. If a non-open reference makes it `> 1`, the function returns early and `v_fifoinfo` stays allocated with no readers or writers. - Nothing frees it later: - `fifo_inactive` only unlocks the vnode. - `fifo_reclaim` is `nullop`. - The filesystems' fifo op tables (for example ffs `ffs_fifoop_entries`) reclaim through `ffs_reclaim`, which doesn't know about `v_fifoinfo`. - So when the vnode is recycled, the fifoinfo and two sockets are lost. The stale `v_un` also goes with the vnode, which is the field #67 tripped over. ## Fix direction - Free the fifoinfo when the last reader and writer are gone (`fi_readers == 0 && fi_writers == 0`), not based on `v_usecount`. Later BSDs do this. - Make sure the revoke/vclean close path (`VOP_CLOSE` with neither FREAD nor FWRITE) can't leave it behind either, for every filesystem that uses fifofs (ufs/ffs, ext2fs, hfs, nfs specfs/fifo tables).

gitea-mirror closed this issue

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

gitea-mirror commented

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

Author

Owner

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

Fixed by a9ee24f2b, merged into ffs-perf in f68d4802c.

Reproduced on the base kernel (#67 fix, cksum 606865771). fifoleak.c does 1000 rounds of mkfifo, open O_RDWR, close, unlink, while one child loops stat() and another loops utimes() on the path. utimes holds a vnode reference while it sleeps in a synchronous inode update, so the last close sees v_usecount > 1.

kalloc.16 (fifoinfo) went 1184 → 2041 (+857).
The socket zone went 38 → 1752 (+1714).

Revoke leaked too: vclean holds its own reference when it calls VOP_CLOSE. It also left threads asleep in fifo_open forever.

Fix:

fifo_release frees the fifoinfo once there are no readers, no writers and no sleepers.
A new fi_busy counts threads asleep with the vnode unlocked (in fifo_open, fifo_read and fifo_write). fifo_unbusy relocks the vnode, drops the count and calls fifo_release.
The revoke close (neither FREAD nor FWRITE set) detaches v_fifoinfo and wakes every sleeper. The fifoinfo is freed at once or by the last sleeper to leave, so it can't be leaked or freed twice.
A woken fifo_open returns EBADF, and a blocked reader gets EOF.

No filesystem changes were needed.

Verified on the fixed kernel (cksum 3755032458):

fifoleak: zone counts flat over two runs of 1000 (kalloc.16 1188 → 1190 → 1190, socket 31 → 31 → 31).
Revoke: 2 × 50 rounds with a sleeper in open, a sleeper in read, and an idle fd. Everyone returned and zones stayed flat.
#67 pair test: stress-misc fifo beside stress-net fdpass for about 40 minutes, 48 fifo and 90 fdpass runs, all PASS.
check-misc, check-net and check-fs pass. soak -t 600 -j 4 has no failures. fsck matches the untouched base image.

Left open: fifo_printinfo (reachable only through vprint) doesn't check for a NULL v_fifoinfo.

@raynorpat commented on GitHub (Oct 20, 2026): Fixed by a9ee24f2b, merged into ffs-perf in f68d4802c. **Reproduced** on the base kernel (#67 fix, cksum 606865771). `fifoleak.c` does 1000 rounds of mkfifo, open O_RDWR, close, unlink, while one child loops `stat()` and another loops `utimes()` on the path. `utimes` holds a vnode reference while it sleeps in a synchronous inode update, so the last close sees `v_usecount > 1`. - kalloc.16 (fifoinfo) went 1184 → 2041 (+857). - The socket zone went 38 → 1752 (+1714). Revoke leaked too: vclean holds its own reference when it calls VOP_CLOSE. It also left threads asleep in `fifo_open` forever. **Fix:** - `fifo_release` frees the fifoinfo once there are no readers, no writers and no sleepers. - A new `fi_busy` counts threads asleep with the vnode unlocked (in `fifo_open`, `fifo_read` and `fifo_write`). `fifo_unbusy` relocks the vnode, drops the count and calls `fifo_release`. - The revoke close (neither FREAD nor FWRITE set) detaches `v_fifoinfo` and wakes every sleeper. The fifoinfo is freed at once or by the last sleeper to leave, so it can't be leaked or freed twice. - A woken `fifo_open` returns EBADF, and a blocked reader gets EOF. No filesystem changes were needed. **Verified** on the fixed kernel (cksum 3755032458): - **fifoleak:** zone counts flat over two runs of 1000 (kalloc.16 1188 → 1190 → 1190, socket 31 → 31 → 31). - **Revoke:** 2 × 50 rounds with a sleeper in open, a sleeper in read, and an idle fd. Everyone returned and zones stayed flat. - **#67 pair test:** `stress-misc fifo` beside `stress-net fdpass` for about 40 minutes, 48 fifo and 90 fdpass runs, all PASS. - `check-misc`, `check-net` and `check-fs` pass. `soak -t 600 -j 4` has no failures. fsck matches the untouched base image. **Left open:** `fifo_printinfo` (reachable only through `vprint`) doesn't check for a NULL `v_fifoinfo`.