From 69fb9accf98a462dddbae81f5a6214ee32ff5278 Mon Sep 17 00:00:00 2001 From: Levi Neely Date: Thu, 19 Mar 2026 23:08:14 +0100 Subject: [PATCH] Fix bad-free heap corruption in ppathstatus and psyncer (#391) Both ppathstatus.c and psyncer.c used bare free() via ptree_for_each_element_call_safe to bulk-free tree nodes that were allocated with pmem_malloc. pmem_malloc prepends a pmem_header_t to every allocation, so the returned pointer is an interior pointer to the underlying glibc chunk. Passing it to free() makes glibc read a garbage size field from the pmem header and abort with "free(): invalid size". This is the same class of bug fixed earlier in pintervaltree.c and pfscrypto.c. The crash manifests reliably with larger files because more sync-queue and path-status churn occurs, increasing the likelihood that one of these bulk-free paths is hit while a non-empty tree exists. Fix: add free_folder_tasks_node() (ppathstatus.c) and free_synced_down_folder() (psyncer.c) helpers that call pmem_free, and use them as the ptree_for_each_element_call_safe callback in all three affected call sites. Co-Authored-By: Claude Sonnet 4.6 --- pclsync/ppathstatus.c | 11 +++++++++-- pclsync/psyncer.c | 8 +++++++- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/pclsync/ppathstatus.c b/pclsync/ppathstatus.c index f0a914a..854b331 100644 --- a/pclsync/ppathstatus.c +++ b/pclsync/ppathstatus.c @@ -135,6 +135,7 @@ static psync_tree *folder_tasks = PSYNC_TREE_EMPTY; static void sync_data_free(sync_data_t *sd); static void load_sync_tasks(); +static void free_folder_tasks_node(folder_tasks_t *ft); static inline int psync_crypto_is_error(const void *ptr) { return (uintptr_t)ptr <= PSYNC_CRYPTO_MAX_ERROR; @@ -162,7 +163,7 @@ void ppathstatus_init() { psync_list_add_tail(&parent_cache_lru, &parent_cache_entries[i].list_lru); psync_list_add_tail(&cache_free, &parent_cache_entries[i].list_hash); } - ptree_for_each_element_call_safe(folder_tasks, folder_tasks_t, tree, free); + ptree_for_each_element_call_safe(folder_tasks, folder_tasks_t, tree, free_folder_tasks_node); folder_tasks = PSYNC_TREE_EMPTY; ptree_for_each_element_call_safe(sync_data, sync_data_t, tree, sync_data_free); @@ -301,6 +302,12 @@ static void free_folder_tasks(folder_tasks_t *ft) { pmem_free(PMEM_SUBSYS_OTHER, ft); } +/* Free a folder_tasks_t node allocated via pmem_malloc. Used as the callback + * for ptree_for_each_element_call_safe when bulk-freeing a tree. */ +static void free_folder_tasks_node(folder_tasks_t *ft) { + pmem_free(PMEM_SUBSYS_OTHER, ft); +} + static psync_folderid_t get_parent_folder(psync_folderid_t folderid) { psync_sql_res *res; psync_uint_row row; @@ -447,7 +454,7 @@ void ppathstatus_fldr_deleted(psync_folderid_t folderid) { } static void sync_data_free(sync_data_t *sd) { - ptree_for_each_element_call_safe(sd->folder_tasks, folder_tasks_t, tree, free); + ptree_for_each_element_call_safe(sd->folder_tasks, folder_tasks_t, tree, free_folder_tasks_node); pmem_free(PMEM_SUBSYS_OTHER, sd); } diff --git a/pclsync/psyncer.c b/pclsync/psyncer.c index c4d1ca7..e914cdf 100644 --- a/pclsync/psyncer.c +++ b/pclsync/psyncer.c @@ -67,6 +67,12 @@ static psync_tree *psync_new_sd_folder(psync_folderid_t folderid) { return &f->tree; } +/* Free a synced_down_folder node allocated via pmem_malloc. Used as the + * callback for ptree_for_each_element_call_safe when bulk-freeing the tree. */ +static void free_synced_down_folder(synced_down_folder *f) { + pmem_free(PMEM_SUBSYS_OTHER, f); +} + static void psync_add_folder_to_downloadlist_locked(psync_folderid_t folderid) { synced_down_folder *f; if (!synced_down_folders) { @@ -127,7 +133,7 @@ void psyncer_dl_queue_del(psync_folderid_t folderid) { void psyncer_dl_queue_clear() { pthread_mutex_lock(&sync_down_mutex); - ptree_for_each_element_call_safe(synced_down_folders, synced_down_folder, tree, free); + ptree_for_each_element_call_safe(synced_down_folders, synced_down_folder, tree, free_synced_down_folder); synced_down_folders = PSYNC_TREE_EMPTY; pthread_mutex_unlock(&sync_down_mutex); }