Concurrency: atomic g_stage/g_err, snapshot pkg_diag, safe open, SQLite mutex

g_stage and g_err are now _Atomic. The backend init thread publishes
stage transitions and function-pointer assignments; HTTP request
handlers read g_stage to decide whether to call the Sony API. With the
old `volatile int` reads, nothing in the C memory model ordered the
function-pointer loads against the stage check — a stage==5 sighting
could (in theory) come before the pointer stores were visible. Default
seq_cst on _Atomic gives us the acquire/release pairing for free.

/api/pkgdiag returned g_pkg_diag_json directly with RESPMEM_PERSISTENT,
so MHD's writer thread could read the buffer while record_pkg_diag was
mid-snprintf — torn JSON or a missing NUL terminator. Snapshot under
g_mutex into a heap copy and queue with RESPMEM_MUST_FREE instead.

patchdl_net.c gets fopen_safe(): open() with O_NOFOLLOW|O_CLOEXEC and
mode 0600, then fdopen. The old fopen("wb") follows symlinks (a
malicious symlink at dest_path could redirect the write) and creates
mode 0666 (libc default). Both download paths now use it.

patchdl_appdb opens SQLite with SQLITE_OPEN_FULLMUTEX. Today the scan
runs on the startup thread only, but a future rescan triggered from the
HTTP thread would otherwise race the handle.
This commit is contained in:
Knutwurst committed 2026-06-24 20:35:31 +02:00
1 parent a6d9f939b5
commit fcb0a5c3b0
4 files changed
+43 -9

No files matched your search

+5 -1
View File
@@ -61,8 +61,12 @@ patchdl_appdb_load(patchdl_appinfo_t **out, size_t *count) {
*out = NULL;
*count = 0;
/* FULLMUTEX: today only the startup thread calls this, but future code
paths (a manual rescan triggered from the HTTP thread) would otherwise
race the SQLite handle. Cost is one mutex per call. */
if (sqlite3_open_v2(APP_DB_URI, &db,
SQLITE_OPEN_READONLY | SQLITE_OPEN_URI, NULL) != SQLITE_OK) {
SQLITE_OPEN_READONLY | SQLITE_OPEN_URI |
SQLITE_OPEN_FULLMUTEX, NULL) != SQLITE_OK) {
if (db) sqlite3_close(db);
return -1;
}
+7 -3
View File
@@ -9,6 +9,7 @@
#include <stddef.h>
#include <stdbool.h>
#include <stdint.h>
#include <stdatomic.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
@@ -112,9 +113,12 @@ static ai_get_status_fn ai_get_status;
symbols via the kernel dynlib helpers. Runs in a detached thread; the HTTP
handler reports the stage and never blocks.
stage: 0 idle, 1 resolve loader, 2 load modules, 3 resolve symbols,
4 initialize, 5 ready, negative = failure at that step. */
static volatile int g_stage;
static int g_err;
4 initialize, 5 ready, negative = failure at that step.
Stored as _Atomic so the worker's release-store and the request handlers'
acquire-loads pair properly — the function pointers they read after
stage==5 must not be reordered ahead of the stage check. */
static _Atomic int g_stage;
static _Atomic int g_err;
static pthread_mutex_t g_mtx = PTHREAD_MUTEX_INITIALIZER;
static char g_probe_json[2048]; /* filled by the backend thread */
static char g_last_content_id[AI_CONTENTID_SIZE];
+20 -3
View File
@@ -2,6 +2,7 @@
#include <arpa/inet.h>
#include <errno.h>
#include <fcntl.h>
#include <netinet/in.h>
#include <pthread.h>
#include <stdio.h>
@@ -9,6 +10,7 @@
#include <string.h>
#include <strings.h>
#include <sys/socket.h>
#include <sys/stat.h>
#include <sys/time.h>
#include <unistd.h>
@@ -33,6 +35,21 @@
#define PATCHDL_BUF_MAX_VERXML (16 * 1024 * 1024)
#define PATCHDL_BUF_MAX_MANIFEST (64 * 1024 * 1024)
#ifdef PATCHDL_HAVE_CURL
/* Replacement for fopen("wb"/"r+b") that refuses to follow a symlink at the
destination (would let a malicious symlink redirect the download) and pins
the new file's mode to 0600. Returns NULL on any open error. */
static FILE *
fopen_safe(const char *path, int rw_existing) {
int flags = O_CLOEXEC | O_NOFOLLOW;
int fd;
flags |= rw_existing ? O_RDWR : (O_WRONLY | O_CREAT | O_TRUNC);
fd = open(path, flags, 0600);
if (fd < 0) return NULL;
return fdopen(fd, rw_existing ? "r+b" : "wb");
}
#endif
#ifdef PATCHDL_HAVE_CURL
static const char *ALLOWED_HOSTS[] = {
@@ -486,7 +503,7 @@ int
patchdl_http_download_progress(const char *url, const char *dest_path,
long long *bytes_out,
patchdl_download_progress_cb cb, void *ctx) {
FILE *fp = fopen(dest_path, "wb");
FILE *fp = fopen_safe(dest_path, 0);
progress_state_t progress = { cb, ctx, 0, 0 };
int rc;
@@ -628,10 +645,10 @@ patchdl_http_download_manifest_progress(const char *manifest_url,
written continues mid-piece via an HTTP byte range (with a fall back to
re-fetching it whole if the CDN ignores the range). */
if (resume) {
fp = fopen(dest_path, "r+b");
fp = fopen_safe(dest_path, 1);
if (fp) { fseek(fp, 0, SEEK_END); have = ftell(fp); if (have < 0) have = 0; }
}
if (!fp) { fp = fopen(dest_path, "wb"); have = 0; }
if (!fp) { fp = fopen_safe(dest_path, 0); have = 0; }
if (!fp) { free(manifest.data); return -1; }
started = (have <= 0);
+11 -2
View File
@@ -2013,8 +2013,17 @@ on_request(void *cls, struct MHD_Connection *conn, const char *url,
return queue_json_owned(conn, MHD_HTTP_OK, strdup(p));
}
if (!strcmp(url, "/api/pkgdiag"))
return queue_json(conn, MHD_HTTP_OK, g_pkg_diag_json);
if (!strcmp(url, "/api/pkgdiag")) {
/* Snapshot under the lock — otherwise MHD would read g_pkg_diag_json
in-place (RESPMEM_PERSISTENT) while record_pkg_diag is mid-snprintf,
producing a torn read or a missing NUL terminator. */
char snap[sizeof(g_pkg_diag_json)];
pthread_mutex_lock(&g_mutex);
memcpy(snap, g_pkg_diag_json, sizeof(snap));
pthread_mutex_unlock(&g_mutex);
snap[sizeof(snap) - 1] = '\0';
return queue_json_owned(conn, MHD_HTTP_OK, strdup(snap));
}
/* Read-only diagnostic: re-fetch the patch manifest for a title (PatchDL can
bypass the DNS block) and dump each piece's offset/size/SHA-256 so the