Fix memory leaks and bad-free on shutdown and crypto write (#375)

* Fix memory leaks in prpc_sockpath and prpc_main_loop

prpc_sockpath allocated home via ppath_home but never freed it before
returning. prpc_main_loop had three early-return paths that leaked
sockpath before the normal free at bind() success.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* Fix memory leaks and bad-free in cache and crypto sector log

pcache.c: cache_timer and pcache_clean called pmem_free(he->value)
directly instead of he->free(he->value), skipping the registered free
callback. This caused all cached SSL connections, TLS sessions, and
crypto decoders to leak their internal mbedtls state on eviction and
shutdown. Same bug fixed in pcache_clean_oneof.

pfscrypto.c: ptree_for_each_element_call_safe passed bare free() to
free psync_sector_inlog_t nodes, but those are allocated via pmem_malloc
which prepends a 16-byte header. Add free_sector_inlog() helper that
calls pmem_free and use it at both call sites.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* Fix bad-free in psync_interval_tree_free

Interval tree nodes are allocated via pmem_malloc, which prepends a
16-byte header. Passing bare free() to ptree_for_each_element_call_safe
freed the wrong address. Add free_interval_tree_node() helper that uses
pmem_free and use it in psync_interval_tree_free.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* Fix bad-free in pcache callbacks using pmem-allocated values

pcache_add callers in ppagecache.c and pfstasks.c registered bare
free() as the eviction callback, but the stored values were allocated
with pmem_malloc/pmem_malloc_array which prepends a 16-byte header.
After the pcache_clean fix that now correctly invokes callbacks, these
bad-frees became fatal. Replace with static helpers that call pmem_free
with the correct subsystem.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* Fix memory leaks in pfs_reopen_file_for_writing and clean_uploads_for_task

pfs.c: encsymkey from pcryptofolder_filencoder_key_get was freed on all
error paths in pfs_reopen_file_for_writing but not on the success path
that returns 1 after ppagecache_copy_to_file_locked.

pfsupload.c: fr from psql_fetchall_int was never freed in
clean_uploads_for_task; add pmem_free after the upload loop.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* Fix spurious 'not mounted' error on FUSE3 shutdown

In FUSE3 mode, pfs_do_stop called fuse_unmount followed by fuse_exit.
The FUSE thread then called fuse_destroy, which tried to unmount again,
producing "fusermount3: not mounted".

For FUSE3, fuse_exit is sufficient to stop the loop; fuse_destroy handles
the unmount. Restrict the explicit fuse_unmount call to FUSE2 only.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Levi Neely <lkn@darkstar.example.net>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
Levi Neely 2026-03-10 13:51:14 +01:00 committed by GitHub
parent 478c1a85a9
commit 9c103041c3
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 34 additions and 25 deletions

View File

@ -81,7 +81,7 @@ static void cache_timer(psync_timer_t timer, void *ptr) {
pthread_mutex_lock(&cachelocks[hash_to_lock(he->hash)]); pthread_mutex_lock(&cachelocks[hash_to_lock(he->hash)]);
psync_list_del(&he->list); psync_list_del(&he->list);
pthread_mutex_unlock(&cachelocks[hash_to_lock(he->hash)]); pthread_mutex_unlock(&cachelocks[hash_to_lock(he->hash)]);
pmem_free(PMEM_SUBSYS_OTHER, he->value); he->free(he->value);
pmem_free(PMEM_SUBSYS_OTHER, he); pmem_free(PMEM_SUBSYS_OTHER, he);
ptimer_stop(timer); ptimer_stop(timer);
} }
@ -229,7 +229,7 @@ void pcache_clean() {
he = psync_list_element(l1, cache_entry_t, list); he = psync_list_element(l1, cache_entry_t, list);
if (!ptimer_stop(he->timer)) { if (!ptimer_stop(he->timer)) {
psync_list_del(l1); psync_list_del(l1);
pmem_free(PMEM_SUBSYS_OTHER, he->value); he->free(he->value);
pmem_free(PMEM_SUBSYS_OTHER, he); pmem_free(PMEM_SUBSYS_OTHER, he);
} }
} }
@ -256,7 +256,7 @@ void pcache_clean_oneof(const char **prefixes, size_t cnt) {
continue; continue;
if (!ptimer_stop(he->timer)) { if (!ptimer_stop(he->timer)) {
psync_list_del(l1); psync_list_del(l1);
pmem_free(PMEM_SUBSYS_OTHER, he->value); he->free(he->value);
pmem_free(PMEM_SUBSYS_OTHER, he); pmem_free(PMEM_SUBSYS_OTHER, he);
} }
} }

View File

@ -2240,6 +2240,7 @@ pfs_reopen_file_for_writing(psync_openfile_t *of) {
} }
} }
of->currentsize = of->initialsize; of->currentsize = of->initialsize;
pmem_free(PMEM_SUBSYS_OTHER, encsymkey);
return 1; return 1;
} }
cr = pfs_task_add_modified_file(of->currentfolder, of->currentname, cr = pfs_task_add_modified_file(of->currentfolder, of->currentname,
@ -3615,29 +3616,15 @@ static void pfs_do_stop(void) {
if (started == 1) { if (started == 1) {
char *mp; char *mp;
struct stat st_before, st_after;
struct timespec ts = {0, 100000000}; struct timespec ts = {0, 100000000};
mp = psync_fuse_get_mountpoint(); mp = psync_fuse_get_mountpoint();
#if FUSE_USE_VERSION < 30
if (mp) { if (mp) {
struct stat st_before;
if (stat(mp, &st_before) == 0) { if (stat(mp, &st_before) == 0) {
#if FUSE_USE_VERSION >= 30
fuse_unmount(psync_fuse);
#else
fuse_unmount(mp, psync_fuse_channel); fuse_unmount(mp, psync_fuse_channel);
psync_fuse_channel = NULL; psync_fuse_channel = NULL;
#endif
clock_gettime(CLOCK_REALTIME, &ts);
// Check if the mountpoint is still accessible
if (stat(mp, &st_after) == 0) {
if (st_before.st_dev == st_after.st_dev) {
pdbg_logf(D_WARNING, "FUSE filesystem may not have unmounted properly");
}
} else if (errno != ENOENT) {
pdbg_logf(D_WARNING, "Unexpected error after unmount: %s",
strerror(errno));
}
} else { } else {
pdbg_logf(D_WARNING, "Mountpoint not accessible before unmount: %s", pdbg_logf(D_WARNING, "Mountpoint not accessible before unmount: %s",
strerror(errno)); strerror(errno));
@ -3645,6 +3632,7 @@ static void pfs_do_stop(void) {
} else { } else {
pdbg_logf(D_ERROR, "Failed to get mountpoint"); pdbg_logf(D_ERROR, "Failed to get mountpoint");
} }
#endif
pdbg_logf(D_NOTICE, "running fuse_exit"); pdbg_logf(D_NOTICE, "running fuse_exit");
fuse_exit(psync_fuse); fuse_exit(psync_fuse);

View File

@ -531,6 +531,10 @@ int pfs_crpt_read_new(psync_openfile_t *of, char *buf,
return rd; return rd;
} }
static void free_sector_inlog(psync_sector_inlog_t *e) {
pmem_free(PMEM_SUBSYS_OTHER, e);
}
static void static void
pfs_crypto_set_sector_log_offset(psync_openfile_t *of, pfs_crypto_set_sector_log_offset(psync_openfile_t *of,
psync_crypto_sectorid_t sectorid, psync_crypto_sectorid_t sectorid,
@ -1093,7 +1097,7 @@ static int pfs_crypto_do_finalize_log(psync_openfile_t *of, int fullsync) {
pmem_free(PMEM_SUBSYS_OTHER, flog); pmem_free(PMEM_SUBSYS_OTHER, flog);
return -EIO; return -EIO;
} }
ptree_for_each_element_call_safe(of->sectorsinlog, psync_sector_inlog_t, tree, free); ptree_for_each_element_call_safe(of->sectorsinlog, psync_sector_inlog_t, tree, free_sector_inlog);
of->sectorsinlog = PSYNC_TREE_EMPTY; of->sectorsinlog = PSYNC_TREE_EMPTY;
ret = pfs_crypto_log_flush_and_process(of, flog, 0, 1); ret = pfs_crypto_log_flush_and_process(of, flog, 0, 1);
pfile_delete(flog); pfile_delete(flog);
@ -1690,7 +1694,7 @@ static int pfs_crpt_truncate_to_zero(psync_openfile_t *of) {
psync_interval_tree_add(&of->writeintervals, 0, psync_interval_tree_add(&of->writeintervals, 0,
pfs_crpt_crypto_size(of->initialsize)); pfs_crpt_crypto_size(of->initialsize));
} }
ptree_for_each_element_call_safe(of->sectorsinlog, psync_sector_inlog_t, tree, free); ptree_for_each_element_call_safe(of->sectorsinlog, psync_sector_inlog_t, tree, free_sector_inlog);
of->sectorsinlog = PSYNC_TREE_EMPTY; of->sectorsinlog = PSYNC_TREE_EMPTY;
of->currentsize = 0; of->currentsize = 0;
pfs_crypto_kill_extender_locked(of); pfs_crypto_kill_extender_locked(of);

View File

@ -1116,6 +1116,10 @@ int pfs_task_unlink(psync_fsfolderid_t folderid, const char *name) {
return 0; return 0;
} }
static void free_file_history_record(void *ptr) {
pmem_free(PMEM_SUBSYS_SYNC, ptr);
}
static void add_history_record(psync_fileid_t fileid, psync_folderid_t folderid, static void add_history_record(psync_fileid_t fileid, psync_folderid_t folderid,
const char *name) { const char *name) {
file_history_record *rec; file_history_record *rec;
@ -1131,7 +1135,7 @@ static void add_history_record(psync_fileid_t fileid, psync_folderid_t folderid,
return; return;
rec->folderid = folderid; rec->folderid = folderid;
memcpy(rec->name, name, len); memcpy(rec->name, name, len);
pcache_add(key, rec, PSYNC_FS_FILE_LOC_HIST_SEC, free, 1); pcache_add(key, rec, PSYNC_FS_FILE_LOC_HIST_SEC, free_file_history_record, 1);
} }
int pfs_task_rename_file(psync_fsfileid_t fileid, int pfs_task_rename_file(psync_fsfileid_t fileid,

View File

@ -318,6 +318,7 @@ static int clean_uploads_for_task(psock_t *api, psync_uploadid_t taskid) {
} else } else
pmem_free(PMEM_SUBSYS_UPLOAD, res); pmem_free(PMEM_SUBSYS_UPLOAD, res);
} }
pmem_free(PMEM_SUBSYS_UPLOAD, fr);
sql = psql_prepare("DELETE FROM fstaskupload WHERE fstaskid=?"); sql = psql_prepare("DELETE FROM fstaskupload WHERE fstaskid=?");
psql_bind_uint(sql, 1, taskid); psql_bind_uint(sql, 1, taskid);
psql_run_free(sql); psql_run_free(sql);

View File

@ -143,9 +143,13 @@ void psync_interval_tree_remove(psync_interval_tree_t **tree, uint64_t from,
} }
} }
static void free_interval_tree_node(psync_interval_tree_t *e) {
pmem_free(PMEM_SUBSYS_OTHER, e);
}
void psync_interval_tree_free(psync_interval_tree_t *tree) { void psync_interval_tree_free(psync_interval_tree_t *tree) {
if (tree) if (tree)
ptree_for_each_element_call_safe(&tree->tree, psync_interval_tree_t, tree, free); ptree_for_each_element_call_safe(&tree->tree, psync_interval_tree_t, tree, free_interval_tree_node);
} }
static psync_interval_tree_t * static psync_interval_tree_t *

View File

@ -509,6 +509,10 @@ static int wait_shared_api() {
return ret; return ret;
} }
static void free_binresult_cache(void *ptr) {
pmem_free(PMEM_SUBSYS_CACHE, ptr);
}
static void set_urls(psync_urls_t *urls, binresult *res) { static void set_urls(psync_urls_t *urls, binresult *res) {
pthread_mutex_lock(&url_cache_mutex); pthread_mutex_lock(&url_cache_mutex);
if (res) { if (res) {
@ -737,7 +741,7 @@ static void release_urls(psync_urls_t *urls) {
etime = papi_find_result2(urls->urls, "expires", PARAM_NUM)->num; etime = papi_find_result2(urls->urls, "expires", PARAM_NUM)->num;
if (etime > ctime + 3600) { if (etime > ctime + 3600) {
psync_get_string_id(buff, "URLS", urls->hash); psync_get_string_id(buff, "URLS", urls->hash);
pcache_add(buff, urls->urls, etime - ctime - 3600, free, 2); pcache_add(buff, urls->urls, etime - ctime - 3600, free_binresult_cache, 2);
urls->urls = NULL; urls->urls = NULL;
} }
} }

View File

@ -241,11 +241,13 @@ void prpc_main_loop() {
char *sockpath = prpc_sockpath(); char *sockpath = prpc_sockpath();
if ((fd = socket(AF_UNIX, SOCK_STREAM, 0)) == -1) { if ((fd = socket(AF_UNIX, SOCK_STREAM, 0)) == -1) {
pdbg_logf(D_ERROR, "Unix socket error failed to open %s", sockpath); pdbg_logf(D_ERROR, "Unix socket error failed to open %s", sockpath);
pmem_free(PMEM_SUBSYS_OTHER, sockpath);
return; return;
} }
if (fchmod(fd, 0600) == -1) { if (fchmod(fd, 0600) == -1) {
pdbg_logf(D_ERROR, "Failed to set socket permissions"); pdbg_logf(D_ERROR, "Failed to set socket permissions");
pmem_free(PMEM_SUBSYS_OTHER, sockpath);
return; return;
} }
@ -257,9 +259,10 @@ void prpc_main_loop() {
if (bind(fd, (struct sockaddr *)&addr, strlen(sockpath) + sizeof(addr.sun_family)) == -1) { if (bind(fd, (struct sockaddr *)&addr, strlen(sockpath) + sizeof(addr.sun_family)) == -1) {
pdbg_logf(D_ERROR, "Unix socket bind error"); pdbg_logf(D_ERROR, "Unix socket bind error");
pmem_free(PMEM_SUBSYS_OTHER, sockpath);
return; return;
} }
pmem_free(PMEM_SUBSYS_OTHER, sockpath); pmem_free(PMEM_SUBSYS_OTHER, sockpath);
if (listen(fd, 5) == -1) { if (listen(fd, 5) == -1) {
@ -335,5 +338,6 @@ char *prpc_sockpath() {
} }
snprintf(sockpath, len, "%s%s", home, subdir); snprintf(sockpath, len, "%s%s", home, subdir);
pmem_free(PMEM_SUBSYS_OTHER, home);
return sockpath; return sockpath;
} }