smb2: rebuild the pool when settings change, move cloud sync onto it, fix signing negotiation

Three related changes to how SMB connections are owned:

1. Settings changes now take effect without a restart. The pool
   snapshots the parameters it was built with (server, share,
   credentials, workgroup, timeout, context count, auth mode - subdir
   excluded, it only affects path building and reads the live
   configuration). A publication whose parameters differ tears the
   pool down; the next smb:// access rebuilds it lazily with the new
   values, and an identical republication leaves a live pool
   untouched. Publications run on the main thread and smb_shutdown()
   takes every lock, so in-flight operations finish first and their
   generation handles fail closed - the rebuild rides entirely on the
   machinery the pool-locking commit introduced. Combined with
   publish-on-save, the full menu flow now works end to end: change a
   setting, save, and the next SMB access uses it.

2. The cloud sync SMB driver used to connect a private context per
   sync session and hold it in unlocked driver state. It now draws
   from the shared pool: smb_pool_acquire()/smb_pool_release() hand
   out a connected context under its slot lock per operation, and
   smb_pool_heal_if_dead() gives cloud sync the same
   heal-once-and-retry policy the VFS operations use at each
   operation's first server contact - previously a transport drop
   simply failed the whole sync cycle. Per-session connection setup,
   the driver's duplicate parameter plumbing, and its unsynchronized
   context field are all gone; connection ownership has exactly one
   home. Acquisition also prefers idle slots now (slock_try_lock
   across the pool, blocking on the round-robin slot only when every
   slot is busy), so a long cloud sync transfer holding one slot does
   not stall unrelated VFS operations that round-robin onto it.

3. smb2_set_security_mode() takes the SMB2_NEGOTIATE_SIGNING_* flags,
   but the VFS backend passed the SMB2_SEC_* authentication constants
   into it, so selecting Kerberos (2) silently demanded
   signing-required (2) and the auth probe's first attempt did the
   same. Signing negotiation and authentication are independent:
   signing is now always ENABLED (the cloud sync driver already did
   this correctly) and the authentication call carries the auth mode.

Verified offline before commit:

- Sanitizer harness extended with: identical republication leaving a
  live pool untouched (zero reconnects), a changed parameter tearing
  it down and the next access rebuilding with the new values, clearing
  the server shutting SMB down entirely, signing always negotiated
  ENABLED with authentication carrying the configured constant
  (including KRB5, where the old coupling requested REQUIRED), and the
  public acquire/heal/release API single-threaded and inside the
  8-thread hammer with heal chaos. ASan+UBSan+LSan clean and TSan
  clean across all of it; the negative test (locks compiled out) still
  fails immediately under TSan.
- Full Makefile build with --enable-builtinsmbclient, threads,
  networking and cloud sync compiled in; clean link, no new warnings
  outside vendored deps. Settings table drift check, griffin-order
  unity compile, and the migration tool self-test all clean.

The stale-handle behavior on rebuild is the generation design working
as intended: streams opened before a settings change fail closed on
their next operation and callers reopen against the new pool, which is
the correct semantic for "the server I was talking to changed".
This commit is contained in:
LibretroAdmin authored and libretroadmin committed 2026-08-10 02:57:09 +02:00
1 parent 0a4aae8f9b
commit 05db1e0766
3 files changed
+258 -90

No files matched your search

+115 -3
View File
@@ -81,6 +81,23 @@ struct smb_slot
};
static struct smb_slot smb_pool[SMB_POOL_CAP];
/* The parameters the current pool was built with. A publication whose
* effective parameters differ tears the pool down so the next smb://
* access rebuilds it with the new values; an identical republication
* leaves the pool alone. subdir is excluded: it only affects path
* building, which reads the live configuration. */
struct smb_pool_params
{
char server[256];
char share[256];
char username[256];
char password[256];
char workgroup[256];
unsigned timeout;
unsigned num_contexts;
unsigned auth_mode;
};
static struct smb_pool_params smb_built;
static unsigned smb_pool_size = 0;
static unsigned smb_pool_rr = 0;
static bool smb_initialized = false;
@@ -133,6 +150,29 @@ static struct smb2_context *smb_slot_ctx(unsigned slot, unsigned gen)
return smb_pool[slot].ctx;
}
static void smb_cfg_str(char *dst, size_t sz, const char *src)
{
if (src)
strlcpy(dst, src, sz);
else
dst[0] = '\0';
}
static void smb_snapshot_params(struct smb_pool_params *p)
{
memset(p, 0, sizeof(*p));
if (!smb_cfg)
return;
smb_cfg_str(p->server, sizeof(p->server), smb_cfg->server_address);
smb_cfg_str(p->share, sizeof(p->share), smb_cfg->share);
smb_cfg_str(p->username, sizeof(p->username), smb_cfg->username);
smb_cfg_str(p->password, sizeof(p->password), smb_cfg->password);
smb_cfg_str(p->workgroup, sizeof(p->workgroup), smb_cfg->workgroup);
p->timeout = smb_cfg->timeout;
p->num_contexts = smb_cfg->num_contexts;
p->auth_mode = smb_cfg->auth_mode;
}
static unsigned smb_effective_timeout(void)
{
unsigned t = smb_cfg ? smb_cfg->timeout : 0;
@@ -171,7 +211,12 @@ static struct smb2_context *smb_connect_new_context(int auth_mode)
if (smb_cfg->workgroup && *smb_cfg->workgroup)
smb2_set_domain(ctx, smb_cfg->workgroup);
smb2_set_timeout(ctx, (int)smb_effective_timeout());
smb2_set_security_mode(ctx, (uint16_t)auth_mode);
/* smb2_set_security_mode() takes the SMB2_NEGOTIATE_SIGNING_*
* flags, not the SMB2_SEC_* authentication constants. Passing the
* auth mode here happened to request signing-required whenever
* Kerberos was selected; signing negotiation and authentication
* are independent knobs. */
smb2_set_security_mode(ctx, SMB2_NEGOTIATE_SIGNING_ENABLED);
smb2_set_authentication(ctx, auth_mode);
if (smb2_connect_share(ctx, server, share, username) < 0)
@@ -239,6 +284,23 @@ bool smb_init_cfg(const struct smb_settings *new_cfg)
return false;
#endif
smb_cfg = new_cfg;
/* If a pool is live and this publication changes any parameter it
* was built with, tear it down; the next smb:// access rebuilds it
* lazily with the new values. Publications run on the main thread
* and smb_shutdown() takes every lock, so in-flight operations
* finish first and their stale generation handles fail closed. */
if (smb_initialized)
{
struct smb_pool_params now;
smb_snapshot_params(&now);
if (memcmp(&now, &smb_built, sizeof(now)) != 0)
{
fprintf(stderr,
"smb: settings changed; rebuilding the context pool "
"on next use\n");
smb_shutdown();
}
}
return true;
}
@@ -380,6 +442,7 @@ static bool smb_init(void)
smb_pool_size = want;
smb_pool_rr = 0;
smb_snapshot_params(&smb_built);
smb_initialized = true;
SMB_POOL_UNLOCK();
@@ -405,9 +468,32 @@ static bool smb_slot_acquire(unsigned *slot, struct smb2_context **ctx)
}
s = smb_pool_rr;
smb_pool_rr = (smb_pool_rr + 1) % smb_pool_size;
#ifdef HAVE_THREADS
{
/* Prefer an idle slot: a long-held slot (a cloud sync transfer,
* a large read) should not stall unrelated operations that the
* round-robin happens to land on it. Fall back to blocking on
* the round-robin slot when everything is busy. */
unsigned n = smb_pool_size;
unsigned i, t;
bool got = false;
SMB_POOL_UNLOCK();
for (i = 0; i < n; i++)
{
t = (s + i) % n;
if (slock_try_lock(smb_slot_lock[t]))
{
s = t;
got = true;
break;
}
}
if (!got)
SMB_SLOT_LOCK(s);
}
#else
SMB_POOL_UNLOCK();
SMB_SLOT_LOCK(s);
#endif
if (!smb_pool[s].ctx)
smb_slot_recreate(s);
if (!smb_pool[s].ctx)
@@ -420,6 +506,32 @@ static bool smb_slot_acquire(unsigned *slot, struct smb2_context **ctx)
return true;
}
/* Public pool access for other in-process SMB consumers (the cloud
* sync driver): acquire returns a connected context with its slot
* lock held; every libsmb2 call on the context must happen before the
* matching release. heal_if_dead mirrors the VFS operations' retry:
* on a failed call, probe the transport and replace the context in
* place (caller keeps holding the same slot). */
bool smb_pool_acquire(unsigned *slot, struct smb2_context **ctx)
{
return smb_slot_acquire(slot, ctx);
}
void smb_pool_release(unsigned slot)
{
if (slot < SMB_POOL_CAP)
SMB_SLOT_UNLOCK(slot);
}
struct smb2_context *smb_pool_heal_if_dead(unsigned slot,
struct smb2_context *ctx)
{
if (slot >= SMB_POOL_CAP)
return NULL;
return smb_slot_heal_if_dead(slot, ctx);
}
/* Shutdown SMB context - called on exit */
void smb_shutdown(void)
{
@@ -81,6 +81,19 @@ int retro_vfs_file_error_smb(libretro_vfs_implementation_file *stream);
/* Context management */
void smb_shutdown(void);
struct smb2_context;
/* Pool access for other in-process SMB consumers (cloud sync).
* acquire returns a connected context with its slot lock held; all
* libsmb2 calls on it must happen before the matching release.
* heal_if_dead replaces a dead-transport context in place under the
* held slot; on success the returned context replaces the old one for
* the remainder of the hold. */
bool smb_pool_acquire(unsigned *slot, struct smb2_context **ctx);
void smb_pool_release(unsigned slot);
struct smb2_context *smb_pool_heal_if_dead(unsigned slot,
struct smb2_context *ctx);
#ifdef __cplusplus
}
#endif
+130 -87
View File
@@ -26,6 +26,8 @@
#include <string/stdstring.h>
#include <time/rtime.h>
#include "../../libretro-common/vfs/vfs_implementation_smb.h"
#include "../cloud_sync_driver.h"
#include "../../configuration.h"
#include "../../verbosity.h"
@@ -34,9 +36,14 @@
/* ========== State ========== */
/* Contexts come from the shared VFS SMB pool (vfs_implementation_smb):
* each operation acquires a slot-locked, already-connected context and
* releases it when done, instead of connecting a private context per
* sync session. Connection parameters, pooling, healing and rebuild on
* settings changes are all owned by the pool. Only the cloud sync
* subdirectory is state here. */
typedef struct
{
struct smb2_context *ctx;
char subdir[PATH_MAX_LENGTH];
} smb_sync_state_t;
@@ -140,14 +147,9 @@ static bool smb_sync_begin(cloud_sync_complete_handler_t cb, void *user_data)
settings_t *settings = config_get_ptr();
const char *server = settings->arrays.smb_client_server_address;
const char *share = settings->arrays.smb_client_share;
const char *user = settings->arrays.smb_client_username;
const char *password = settings->arrays.smb_client_password;
const char *workgroup = settings->arrays.smb_client_workgroup;
const char *subdir = settings->arrays.smb_client_subdir;
unsigned auth_mode = settings->uints.smb_client_auth_mode;
unsigned timeout = settings->uints.smb_client_timeout;
struct smb2_context *ctx;
int rc;
unsigned slot;
if ((!server || !*server) || (!share || !*share))
{
@@ -156,40 +158,6 @@ static bool smb_sync_begin(cloud_sync_complete_handler_t cb, void *user_data)
return true;
}
ctx = smb2_init_context();
if (!ctx)
{
RARCH_ERR(SMBPFX "Failed to create SMB context\n");
cb(user_data, NULL, false, NULL);
return true;
}
if (user && *user)
smb2_set_user(ctx, user);
if (password && *password)
smb2_set_password(ctx, password);
if (workgroup && *workgroup)
smb2_set_domain(ctx, workgroup);
if (timeout > 0)
smb2_set_timeout(ctx, (int)timeout);
smb2_set_security_mode(ctx, SMB2_NEGOTIATE_SIGNING_ENABLED);
smb2_set_authentication(ctx, (int)auth_mode);
RARCH_LOG(SMBPFX "Connecting to %s/%s as %s\n",
server, share, (!user || !*user) ? "(anonymous)" : user);
rc = smb2_connect_share(ctx, server, share,
(!user || !*user) ? NULL : user);
if (rc < 0)
{
RARCH_ERR(SMBPFX "Connect failed: %s\n", smb2_get_error(ctx));
smb2_destroy_context(ctx);
cb(user_data, NULL, false, NULL);
return true;
}
smb_st.ctx = ctx;
/* Build cloud sync subdir: {smb_client_subdir}/cloud_sync */
smb_st.subdir[0] = '\0';
{
@@ -205,39 +173,47 @@ static bool smb_sync_begin(cloud_sync_complete_handler_t cb, void *user_data)
strlcpy(smb_st.subdir + _len, "cloud_sync", sz - _len);
}
/* Ensure subdir exists */
if (!smb_pool_acquire(&slot, &ctx))
{
if (!smb_sync_ensure_dir(ctx, smb_st.subdir))
RARCH_ERR(SMBPFX "SMB pool unavailable (connection or "
"configuration failure)\n");
cb(user_data, NULL, false, NULL);
return true;
}
/* Ensure subdir exists; one heal-and-retry on a dead transport,
* the same policy the VFS operations use. */
if (!smb_sync_ensure_dir(ctx, smb_st.subdir))
{
if ((ctx = smb_pool_heal_if_dead(slot, ctx))
&& smb_sync_ensure_dir(ctx, smb_st.subdir))
; /* healed */
else
{
RARCH_ERR(SMBPFX "Failed to create subdir '%s'\n", smb_st.subdir);
smb2_disconnect_share(ctx);
smb2_destroy_context(ctx);
smb_st.ctx = NULL;
smb_pool_release(slot);
cb(user_data, NULL, false, NULL);
return true;
}
}
smb_pool_release(slot);
RARCH_LOG(SMBPFX "Connected successfully\n");
RARCH_LOG(SMBPFX "Cloud sync ready on the shared SMB pool\n");
cb(user_data, NULL, true, NULL);
return true;
}
static bool smb_sync_end(cloud_sync_complete_handler_t cb, void *user_data)
{
if (smb_st.ctx)
{
smb2_disconnect_share(smb_st.ctx);
smb2_destroy_context(smb_st.ctx);
smb_st.ctx = NULL;
}
/* Contexts belong to the shared pool; nothing to tear down. */
smb_st.subdir[0] = '\0';
cb(user_data, NULL, true, NULL);
return true;
}
static bool smb_read(const char *path, const char *file,
static bool smb_read_locked(unsigned slot, struct smb2_context *ctx,
const char *path, const char *file,
cloud_sync_complete_handler_t cb, void *user_data)
{
char smb_path[PATH_MAX_LENGTH];
@@ -252,8 +228,14 @@ static bool smb_read(const char *path, const char *file,
smb_sync_build_path(smb_path, sizeof(smb_path), smb_st.subdir, path);
/* Check if file exists */
rc = smb2_stat(smb_st.ctx, smb_path, &st);
/* Check if file exists; heal-and-retry once on a dead transport */
rc = smb2_stat(ctx, smb_path, &st);
if (rc < 0 && -rc != ENOENT)
{
struct smb2_context *fresh = smb_pool_heal_if_dead(slot, ctx);
if (fresh)
rc = smb2_stat((ctx = fresh), smb_path, &st);
}
if (rc < 0)
{
if (-rc == ENOENT)
@@ -264,18 +246,18 @@ static bool smb_read(const char *path, const char *file,
return true;
}
RARCH_ERR(SMBPFX "Stat '%s' failed: %s\n",
smb_path, smb2_get_error(smb_st.ctx));
smb_path, smb2_get_error(ctx));
cb(user_data, path, false, NULL);
return true;
}
file_size = st.smb2_size;
fh = smb2_open(smb_st.ctx, smb_path, O_RDONLY);
fh = smb2_open(ctx, smb_path, O_RDONLY);
if (!fh)
{
RARCH_ERR(SMBPFX "Failed to open '%s': %s\n",
smb_path, smb2_get_error(smb_st.ctx));
smb_path, smb2_get_error(ctx));
cb(user_data, path, false, NULL);
return true;
}
@@ -283,23 +265,23 @@ static bool smb_read(const char *path, const char *file,
buf = (uint8_t *)malloc((size_t)file_size);
if (!buf)
{
smb2_close(smb_st.ctx, fh);
smb2_close(ctx, fh);
cb(user_data, path, false, NULL);
return true;
}
chunk = smb2_get_max_read_size(smb_st.ctx);
chunk = smb2_get_max_read_size(ctx);
while (offset < file_size)
{
uint32_t to_read = (uint32_t)((file_size - offset) < chunk
? (file_size - offset) : chunk);
int n = smb2_read(smb_st.ctx, fh, buf + offset, to_read);
int n = smb2_read(ctx, fh, buf + offset, to_read);
if (n < 0)
{
RARCH_ERR(SMBPFX "Read error on '%s': %s\n",
smb_path, smb2_get_error(smb_st.ctx));
smb_path, smb2_get_error(ctx));
free(buf);
smb2_close(smb_st.ctx, fh);
smb2_close(ctx, fh);
cb(user_data, path, false, NULL);
return true;
}
@@ -308,7 +290,7 @@ static bool smb_read(const char *path, const char *file,
offset += (uint32_t)n;
}
smb2_close(smb_st.ctx, fh);
smb2_close(ctx, fh);
/* Write to local file */
rfile = filestream_open(file, RETRO_VFS_FILE_ACCESS_READ_WRITE, 0);
@@ -329,7 +311,24 @@ static bool smb_read(const char *path, const char *file,
return true;
}
static bool smb_update(const char *path, RFILE *rfile,
static bool smb_read(const char *path, const char *file,
cloud_sync_complete_handler_t cb, void *user_data)
{
unsigned slot;
struct smb2_context *ctx;
bool ret;
if (!smb_pool_acquire(&slot, &ctx))
{
cb(user_data, path, false, NULL);
return true;
}
ret = smb_read_locked(slot, ctx, path, file, cb, user_data);
smb_pool_release(slot);
return ret;
}
static bool smb_update_locked(unsigned slot, struct smb2_context *ctx,
const char *path, RFILE *rfile,
cloud_sync_complete_handler_t cb, void *user_data)
{
char smb_path[PATH_MAX_LENGTH];
@@ -341,12 +340,17 @@ static bool smb_update(const char *path, RFILE *rfile,
smb_sync_build_path(smb_path, sizeof(smb_path), smb_st.subdir, path);
/* Ensure parent directories exist */
if (!smb_sync_ensure_parent_dir(smb_st.ctx, smb_path))
/* Ensure parent directories exist; heal-and-retry once on a dead
* transport, the same policy the VFS operations use. */
if (!smb_sync_ensure_parent_dir(ctx, smb_path))
{
RARCH_ERR(SMBPFX "Failed to create parent dirs for '%s'\n", smb_path);
cb(user_data, path, false, rfile);
return true;
struct smb2_context *fresh = smb_pool_heal_if_dead(slot, ctx);
if (!fresh || !smb_sync_ensure_parent_dir((ctx = fresh), smb_path))
{
RARCH_ERR(SMBPFX "Failed to create parent dirs for '%s'\n", smb_path);
cb(user_data, path, false, rfile);
return true;
}
}
/* Read the local file into memory */
@@ -365,38 +369,38 @@ static bool smb_update(const char *path, RFILE *rfile,
filestream_read(rfile, buf, file_size);
/* Open remote file for writing (create if needed) */
fh = smb2_open(smb_st.ctx, smb_path, O_WRONLY | O_CREAT);
fh = smb2_open(ctx, smb_path, O_WRONLY | O_CREAT);
if (!fh)
{
RARCH_ERR(SMBPFX "Failed to open '%s' for writing: %s\n",
smb_path, smb2_get_error(smb_st.ctx));
smb_path, smb2_get_error(ctx));
free(buf);
cb(user_data, path, false, rfile);
return true;
}
/* Truncate to 0 in case file already existed with larger content */
smb2_ftruncate(smb_st.ctx, fh, 0);
smb2_ftruncate(ctx, fh, 0);
chunk = smb2_get_max_write_size(smb_st.ctx);
chunk = smb2_get_max_write_size(ctx);
while (offset < (uint64_t)file_size)
{
uint32_t to_write = (uint32_t)(((uint64_t)file_size - offset) < chunk
? ((uint64_t)file_size - offset) : chunk);
int n = smb2_write(smb_st.ctx, fh, buf + offset, to_write);
int n = smb2_write(ctx, fh, buf + offset, to_write);
if (n < 0)
{
RARCH_ERR(SMBPFX "Write error on '%s': %s\n",
smb_path, smb2_get_error(smb_st.ctx));
smb_path, smb2_get_error(ctx));
free(buf);
smb2_close(smb_st.ctx, fh);
smb2_close(ctx, fh);
cb(user_data, path, false, rfile);
return true;
}
offset += (uint32_t)n;
}
smb2_close(smb_st.ctx, fh);
smb2_close(ctx, fh);
free(buf);
RARCH_DBG(SMBPFX "Updated %s (%u bytes)\n",
@@ -405,7 +409,24 @@ static bool smb_update(const char *path, RFILE *rfile,
return true;
}
static bool smb_free(const char *path,
static bool smb_update(const char *path, RFILE *rfile,
cloud_sync_complete_handler_t cb, void *user_data)
{
unsigned slot;
struct smb2_context *ctx;
bool ret;
if (!smb_pool_acquire(&slot, &ctx))
{
cb(user_data, path, false, NULL);
return true;
}
ret = smb_update_locked(slot, ctx, path, rfile, cb, user_data);
smb_pool_release(slot);
return ret;
}
static bool smb_free_locked(unsigned slot, struct smb2_context *ctx,
const char *path,
cloud_sync_complete_handler_t cb, void *user_data)
{
char smb_path[PATH_MAX_LENGTH];
@@ -416,7 +437,13 @@ static bool smb_free(const char *path,
smb_sync_build_path(smb_path, sizeof(smb_path), smb_st.subdir, path);
/* Check if file exists */
rc = smb2_stat(smb_st.ctx, smb_path, &st);
rc = smb2_stat(ctx, smb_path, &st);
if (rc < 0 && -rc != ENOENT)
{
struct smb2_context *fresh = smb_pool_heal_if_dead(slot, ctx);
if (fresh)
rc = smb2_stat((ctx = fresh), smb_path, &st);
}
if (rc < 0)
{
if (-rc == ENOENT)
@@ -426,18 +453,18 @@ static bool smb_free(const char *path,
return true;
}
RARCH_ERR(SMBPFX "Stat '%s' failed: %s\n",
smb_path, smb2_get_error(smb_st.ctx));
smb_path, smb2_get_error(ctx));
cb(user_data, path, false, NULL);
return true;
}
if (settings->bools.cloud_sync_destructive)
{
rc = smb2_unlink(smb_st.ctx, smb_path);
rc = smb2_unlink(ctx, smb_path);
if (rc < 0)
{
RARCH_ERR(SMBPFX "Delete '%s' failed: %s\n",
smb_path, smb2_get_error(smb_st.ctx));
smb_path, smb2_get_error(ctx));
cb(user_data, path, false, NULL);
return true;
}
@@ -461,13 +488,13 @@ static bool smb_free(const char *path,
strlcat(new_path, ts, sizeof(new_path));
/* Ensure parent dir of backup path exists */
smb_sync_ensure_parent_dir(smb_st.ctx, new_path);
smb_sync_ensure_parent_dir(ctx, new_path);
rc = smb2_rename(smb_st.ctx, smb_path, new_path);
rc = smb2_rename(ctx, smb_path, new_path);
if (rc < 0)
{
RARCH_ERR(SMBPFX "Rename '%s' -> '%s' failed: %s\n",
smb_path, new_path, smb2_get_error(smb_st.ctx));
smb_path, new_path, smb2_get_error(ctx));
cb(user_data, path, false, NULL);
return true;
}
@@ -478,6 +505,22 @@ static bool smb_free(const char *path,
return true;
}
static bool smb_free(const char *path,
cloud_sync_complete_handler_t cb, void *user_data)
{
unsigned slot;
struct smb2_context *ctx;
bool ret;
if (!smb_pool_acquire(&slot, &ctx))
{
cb(user_data, path, false, NULL);
return true;
}
ret = smb_free_locked(slot, ctx, path, cb, user_data);
smb_pool_release(slot);
return ret;
}
/* ========== Registration ========== */
cloud_sync_driver_t cloud_sync_smb = {