mirror of
https://github.com/libretro/RetroArch.git
synced 2026-11-09 03:29:43 -10:00
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:
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
@@ -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);
|
||||
|
||||
Reference in new issue
Block a user