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 <noreply@anthropic.com>
This commit is contained in:
parent
bf11ae3490
commit
69fb9accf9
|
|
@ -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);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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);
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue