smb2: serialize context use, replace cached context pointers with generation handles

libsmb2's synchronous API is not thread-safe per context: each
smb2_context owns a socket and a PDU queue, and two overlapping
synchronous calls on one context corrupt both. The VFS SMB backend
handed contexts out round-robin with an unsynchronized cursor and let
every open file and directory cache its context pointer for its whole
lifetime, so concurrent VFS access from task threads interleaved
operations on shared contexts, two threads could race the lazy
smb_init() into double initialization, and smb_shutdown() could tear
the pool down under in-flight operations.

Worse, the pointer caching was unsafe even single-threaded: healing a
dead connection destroys the old context, so every other stream still
caching that pointer dangled, and the smb2_context_active() liveness
probe those streams ran was itself a read of freed memory.

The pool is now an array of slots, each holding a context and a
generation counter, with a lock per slot and a pool lock:

- Every libsmb2 call happens under the owning slot's lock, which
  serializes use of the non-thread-safe context.
- Streams and directory handles store a packed (slot, generation)
  handle instead of a context pointer. Destroying a slot's context -
  heal or shutdown - bumps the generation, so every outstanding handle
  on it fails closed under the slot lock without touching freed
  memory. This fixes the stale-pointer use-after-free in non-threaded
  builds too, where the locks compile to no-ops but the generation
  logic remains.
- Initialization is serialized by the pool lock, so a lazy first use
  from two threads initializes exactly once; the round-robin cursor
  and shutdown are under the same lock. Healing happens in place under
  the slot lock via the same create-and-connect helper initialization
  uses, removing the init/heal configuration drift fixed piecemeal
  before (the heal path now shares the timeout fallback by
  construction).
- The pool is a fixed-capacity static array (64 slots; the menu caps
  the setting at 20), which removes the pool allocation failure paths.
  Lock objects are created when the configuration is first published -
  on the main thread during startup, before any thread can reach an
  smb:// path, since operations refuse to run unconfigured - and are
  deliberately never destroyed, so no thread can block on a freed
  lock.
- retro_vfs_readdir_smb() returned a pointer to a function-local
  static, which two threads reading different directories raced on
  regardless of pool locking; the entry now lives in the directory
  handle.

The lock order is pool before slot where both are needed; operations
on an open handle take only the slot lock, so reads and writes on
different contexts do not contend.

Resource cost when SMB is never used: publication of the configuration
(at startup and after each config load) creates exactly one bootstrap
mutex and performs a handful of scalar stores - no contexts, no
sockets, no network initialization. The per-slot locks are created
under the pool lock on first actual smb:// access, sized to the
configured pool, so a session that never touches SMB never allocates
them; the heavy machinery remains fully lazy.

Verified offline before commit:

- Sanitizer harness linking the real translation unit and the real
  rthreads slock against scripted libsmb2 stubs (real vendored headers
  supply the prototypes; stub handle bookkeeping mirrors libsmb2
  freeing outstanding handles with their context). Single-threaded
  scenarios cover the lifecycle: unconfigured refusal, explicit values
  respected, zero-value fallbacks, capacity clamping, the KRB5 probe
  and explicit-KRB5 failure, and file/directory operations across a
  forced heal, including stale-generation reads, closes after the
  context was destroyed, and directory reads after shutdown. The
  multi-threaded scenario runs 8 threads x 400 iterations of
  stat/open/read/seek/tell/close and opendir/readdir/closedir against
  a chaos thread toggling transport death every 2 ms to force
  continuous heals, starting from unserialized lazy initialization.
  ASan+UBSan+LSan clean and TSan clean on all of it, with zero
  contexts and zero handles alive after shutdown.
- Negative test: the same multi-threaded harness against the
  translation unit compiled without HAVE_THREADS (locks compiled out)
  fails immediately under TSan with a data race in smb_slot_acquire,
  confirming the harness detects the defect class this commit removes.
- Full Makefile build with --enable-builtinsmbclient and threads,
  clean link, no new warnings outside vendored deps. Settings table
  drift check and griffin-order unity compile clean.

Known limitation, unchanged: settings changes only take effect for
contexts created after publication; the pool is not rebuilt while
initialized.
This commit is contained in:
LibretroAdmin authored and libretroadmin committed 2026-08-10 02:20:54 +02:00
1 parent 448088dcc2
commit 3c7447d9d0
2 files changed
+490 -355

No files matched your search

File diff suppressed because it is too large. Load diff
+12 -1
View File
@@ -38,8 +38,19 @@ struct smbc_dirent {
};
typedef struct {
struct smb2_context *ctx;
/* Checked by the generic VFS layer; non-NULL on success. */
struct smb2dir *dir;
/* Pool slot the directory was opened on, plus the slot generation
* at open time. Operations resolve these to a context under the
* slot lock and fail closed when the generation no longer matches
* (context healed or shut down), instead of caching a context
* pointer that dangles once the context is destroyed. */
unsigned slot;
unsigned gen;
/* Storage for the entry returned by retro_vfs_readdir_smb(). Was a
* function-local static, which two threads reading different
* directories would race on regardless of any pool locking. */
struct smbc_dirent ent;
} smb_dir_handle;
bool smb_init_cfg(const struct smb_settings *new_cfg);