gfapi: prevent use-after-free from concurrent open during fini

[ThalesBarretto]

Summary

Add a shutting_down flag to struct glfs that prevents new gfapi operations from entering the xlator stack after glfs_fini() begins teardown. Without this, glfs_open() and 30+ other entry points can create file descriptors on a graph being torn down, causing use-after-free.

Fixes: #4666

Problem

__GLFS_ENTRY_VALIDATE_FS checks only if (!fs) — there is no shutdown gate. glfs_fini() does not set any flag at entry; ctx->cleanup_started is set inside the drain loop only after call_pool->cnt reaches 0. A concurrent glfs_open() passes validation, gets a subvol reference, and creates an fd on inodes that inode_table_destroy_all is about to destroy.

This race has been known informally for 10+ years (see #404, filed 2018, closed stale). The drain loop comment at glfs.c:1303 — "leaked frames may exist, we ignore" — acknowledges the symptom. A TLA+ formal model proves the race exists at protocol level (invariant violation at depth 5 with 19 distinct states). The real C code is worse than the model — no flag is set at fini entry at all.

Changes

api/src/glfs-internal.h

• Add gf_boolean_t shutting_down to struct glfs
• Check fs->shutting_down in __GLFS_ENTRY_VALIDATE_FS (fast path, no mutex — safe because the flag is monotonic: only transitions false to true)

api/src/glfs-resolve.c

• Check fs->shutting_down under mutex in priv_glfs_active_subvol (authoritative gate — every gfapi entry point flows through here)

api/src/glfs.c

• Set fs->shutting_down = _gf_true at the top of pub_glfs_fini, before the drain loop, under mutex with cond broadcast to wake any waiting threads
• Replace glfs_fini's call to glfs_active_subvol (which would now be rejected) with a direct fs->active_subvol read under mutex, mirroring the old_subvol handling from priv_glfs_active_subvol

Cleanup paths (glfs_fd_destroy, glfs_subvol_done) use glfs_lock with _gf_false and are unaffected.

Scope

This patch addresses Bug A from #4666 (no new operations after fini starts). Bug B (in-flight operations between drain-loop exit and graph destruction) is a separate, more complex fix requiring a barrier or re-check mechanism.

Test plan

• Verify make check passes (cmocka unit tests)
• Verify regression test tests/basic/gfapi/ suite passes
• Multi-threaded gfapi test: concurrent glfs_open/glfs_fini from two threads no longer crashes
• Verify glfs_open after glfs_fini returns NULL with errno == ESHUTDOWN
• Verify normal single-threaded lifecycle (glfs_new -> glfs_init -> glfs_open -> glfs_close -> glfs_fini) is unaffected

Related

• Implement proper cleanup sequence #404 — "Implement proper cleanup sequence" (the 2018 design issue, closed stale)
• Null pointer dereference in afr_notify() due to access after free in afr_has_quorum #4644 — afr_notify null deref during fini (open, same family)
• Don't fire the callback when cleanup is in progress #4527 — Don't fire timer callback during cleanup (open, different layer — timer infrastructure vs gfapi entry points)
• Samba MR !4474 — Samba-side fix that enforces the fd-close-before-fini contract; this patch provides the libgfapi-side enforcement

Formal verification

The TLA+ model and verification instructions are in the issue: #4666