diff --git a/Makefile b/Makefile index 631f3e4..367b93e 100644 --- a/Makefile +++ b/Makefile @@ -175,7 +175,8 @@ TEST_BINS := \ tests/test_ppagecache \ tests/test_pfs_helpers \ tests/test_pdiff_helpers \ - tests/test_plocalscan_helpers + tests/test_plocalscan_helpers \ + tests/test_pcl26j_free .PHONY: test tests check clean-tests @@ -258,6 +259,11 @@ tests/test_plocalscan_helpers: $(UNIT_DIR)/test_plocalscan_helpers.c $(LIBDIR)/p -Wl,--wrap=psync_send_backup_del_event # ^ GNU ld only; wraps filter/side-effect calls in extracted helpers +tests/test_pcl26j_free: $(UNIT_DIR)/test_pcl26j_free.c $(LIBDIR)/ptree.c $(LIBDIR)/pmem.c $(LIBDIR)/pdbg.c $(LIBDIR)/putil.c $(LIBDIR)/ppath.c tests/stubs/test_stubs.c + $(CC) $(TEST_CFLAGS) $(CFLAGS) -o $@ $^ \ + -Wl,--wrap=malloc \ + -Wl,--wrap=free + tests/test_pdiff_helpers: $(UNIT_DIR)/test_pdiff_helpers.c $(LIBDIR)/pdiff_helpers.c $(LIBDIR)/pdbg.c $(LIBDIR)/pmem.c $(LIBDIR)/putil.c $(LIBDIR)/ppath.c tests/stubs/test_stubs.c $(CC) $(TEST_CFLAGS) $(CFLAGS) -o $@ $^ \ -Wl,--wrap=papi_find_result \ diff --git a/pclsync/pfs.c b/pclsync/pfs.c index 02b3c18..f68f6b9 100644 --- a/pclsync/pfs.c +++ b/pclsync/pfs.c @@ -1745,6 +1745,12 @@ static void close_if_valid(int fd) { pfile_close(fd); } +/* Free a psync_sector_inlog_t node allocated via pmem_malloc. Used as the + * callback for ptree_for_each_element_call_safe when bulk-freeing the tree. */ +static void free_sector_inlog_node(psync_sector_inlog_t *e) { + pmem_free(PMEM_SUBSYS_OTHER, e); +} + static void pfs_free_openfile(psync_openfile_t *of) { pdbg_logf(D_NOTICE, "releasing file %s", of->currentname); if (unlikely(of->writetimer != PSYNC_INVALID_TIMER)) @@ -1770,7 +1776,7 @@ static void pfs_free_openfile(psync_openfile_t *of) { } close_if_valid(of->logfile); ptree_for_each_element_call_safe( - of->sectorsinlog, psync_sector_inlog_t, tree, free); + of->sectorsinlog, psync_sector_inlog_t, tree, free_sector_inlog_node); delete_log_files(of); if (of->authenticatedints) psync_interval_tree_free(of->authenticatedints); diff --git a/pclsync/ppagecache.c b/pclsync/ppagecache.c index d115b3f..96d16eb 100644 --- a/pclsync/ppagecache.c +++ b/pclsync/ppagecache.c @@ -2238,9 +2238,15 @@ int ppagecache_read_mod_locked(psync_openfile_t *of, char *buf, return rd; } +/* Free a psync_request_range_t node allocated via pmem_malloc. Used as the + * callback for psync_list_for_each_element_call when bulk-freeing the list. */ +static void free_request_range(psync_request_range_t *range) { + pmem_free(PMEM_SUBSYS_CACHE, range); +} + static void psync_pagecache_free_request(psync_request_t *request) { psync_list_for_each_element_call(&request->ranges, psync_request_range_t, - list, free); + list, free_request_range); pmem_free(PMEM_SUBSYS_CACHE, request); } 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); } diff --git a/tests/smoke-tests/smoke-test-large-read.sh b/tests/smoke-tests/smoke-test-large-read.sh new file mode 100755 index 0000000..e88bcd5 --- /dev/null +++ b/tests/smoke-tests/smoke-test-large-read.sh @@ -0,0 +1,107 @@ +#!/bin/bash +# Smoke test: large file read through FUSE mount (pcl-26j) +# +# Regression test for bad-free heap corruption in psync_pagecache_free_request. +# A large read forces multiple psync_request_range_t allocations — one per +# non-contiguous page range — which are then freed via free_request_range(). +# The old code called bare free() on pmem_malloc-allocated nodes; this build +# uses AddressSanitizer to catch any such mismatched free. +# +# Usage: +# PCLOUD_USER=you@example.com bash tests/smoke-tests/smoke-test-large-read.sh +# +# Requirements: +# - PCLOUD_USER env var set (credentials prompted at daemon start) +# - A file >= 50 MB present in the pCloud root (REMOTE_FILE below) +# - FUSE available on the host + +set -euo pipefail + +MOUNT="${HOME}/pCloudDrive" +ASAN_LOG="/tmp/pcloudcc_asan_large_read_$$.log" +REMOTE_FILE="${SMOKE_REMOTE_FILE:-}" # override via env if needed +MIN_SIZE_MB=50 + +# ---- helpers --------------------------------------------------------------- + +die() { echo "ERROR: $*" >&2; exit 1; } + +cleanup() { + echo "--- stopping daemon ---" + echo "finalize" | ./pcloudcc -k 2>/dev/null || true + sleep 1 + # Kill any stray daemon + pkill -f "pcloudcc -d" 2>/dev/null || true + echo "--- ASAN log ($ASAN_LOG) ---" + cat "$ASAN_LOG" 2>/dev/null || echo "(empty)" +} +trap cleanup EXIT + +# ---- sanity checks --------------------------------------------------------- + +[[ -n "${PCLOUD_USER:-}" ]] || die "PCLOUD_USER not set" +[[ -x ./pcloudcc ]] || die "pcloudcc binary not found — run make first" + +# Verify the binary was built with ASAN +if ! readelf -d ./pcloudcc 2>/dev/null | grep -q "libasan\|asan"; then + echo "WARNING: pcloudcc does not appear to be an ASAN build." + echo " For a meaningful test, rebuild with:" + echo " make clean && CFLAGS='-fsanitize=address -g -O1' \\" + echo " CXXFLAGS='-fsanitize=address -g -O1' \\" + echo " LDFLAGS='-fsanitize=address' make -j\$(nproc)" + echo " Continuing anyway — crash-level corruption may still surface." +fi + +# ---- build env ------------------------------------------------------------- + +export ASAN_OPTIONS="log_path=${ASAN_LOG}:abort_on_error=0:detect_leaks=0" + +# ---- start daemon ---------------------------------------------------------- + +echo "--- starting pcloudcc daemon ---" +./pcloudcc -u "$PCLOUD_USER" -d +sleep 3 # wait for mount and initial sync + +[[ -d "$MOUNT" ]] || die "mount point $MOUNT does not exist after daemon start" + +# ---- find a large remote file ---------------------------------------------- + +if [[ -z "$REMOTE_FILE" ]]; then + echo "--- searching for a file >= ${MIN_SIZE_MB} MB in ${MOUNT} ---" + REMOTE_FILE=$(find "$MOUNT" -maxdepth 3 -type f \ + -size "+${MIN_SIZE_MB}M" -print -quit 2>/dev/null || true) +fi + +if [[ -z "$REMOTE_FILE" ]]; then + echo "SKIP: no file >= ${MIN_SIZE_MB} MB found in ${MOUNT}." + echo " Upload a large file first, or set SMOKE_REMOTE_FILE=/path/to/file." + exit 0 +fi + +echo "--- reading: $REMOTE_FILE ---" +SIZE=$(stat -c%s "$REMOTE_FILE" 2>/dev/null || echo 0) +echo " size: $((SIZE / 1024 / 1024)) MB" + +# ---- read the file, exercising the page-cache request path ----------------- + +echo "--- streaming file to /dev/null ---" +dd if="$REMOTE_FILE" of=/dev/null bs=1M 2>&1 | tail -1 +echo "--- read complete ---" + +# Give ASAN a moment to flush any deferred reports +sleep 1 + +# ---- evaluate ASAN output -------------------------------------------------- + +ASAN_ERRORS=$(grep -c "ERROR: AddressSanitizer" "$ASAN_LOG" 2>/dev/null || true) +BAD_FREE_ERRORS=$(grep -c "attempting free on address" "$ASAN_LOG" 2>/dev/null || true) + +echo "" +if [[ "$ASAN_ERRORS" -eq 0 ]]; then + echo "PASS: no AddressSanitizer errors detected during large file read" + exit 0 +else + echo "FAIL: $ASAN_ERRORS ASAN error(s) detected ($BAD_FREE_ERRORS bad-free)" + echo " See $ASAN_LOG for details" + exit 1 +fi diff --git a/tests/unit-tests/test_pcl26j_free.c b/tests/unit-tests/test_pcl26j_free.c new file mode 100644 index 0000000..591290e --- /dev/null +++ b/tests/unit-tests/test_pcl26j_free.c @@ -0,0 +1,361 @@ +/* + * Test: bad-free detection for pmem_malloc-allocated tree/list nodes (pcl-26j) + * + * Verifies that the free helpers introduced in the pcl-26j fix call + * pmem_free() — which backs up to the pmem_header_t before calling free() — + * rather than bare free() directly on the data pointer. + * + * Each of the four fixed call sites is exercised: + * 1. psync_request_range_t (ppagecache.c: psync_pagecache_free_request) + * 2. synced_down_folder (psyncer.c: psyncer_dl_queue_clear) + * 3. folder_tasks_t (ppathstatus.c: ppathstatus_init / sync_data_free) + * 4. psync_sector_inlog_t (pfs.c: pfs_free_openfile) + * + * Mechanism + * --------- + * --wrap=malloc records every raw pointer that malloc() returns. + * --wrap=free asserts that every pointer passed to free() is in that set. + * + * When the old code called bare free(data_ptr) on a pmem_malloc allocation, + * data_ptr = hdr+1 (past the pmem_header_t). That pointer was never returned + * by malloc, so the assertion would fire. With the fixed code, pmem_free() + * backs up to hdr before calling free(hdr), which IS the malloc-returned + * pointer. The assertion passes. + * + * Build: + * gcc -fsanitize=address -g -o test_pcl26j_free \ + * tests/unit-tests/test_pcl26j_free.c pclsync/pmem.c pclsync/ptree.c \ + * pclsync/pdbg.c pclsync/putil.c pclsync/ppath.c tests/stubs/test_stubs.c \ + * -I./pclsync -Wl,--wrap=malloc -Wl,--wrap=free -lpthread + */ + +#define _POSIX_C_SOURCE 200809L +#include +#include +#include +#include +#include + +#include "pmem.h" +#include "ptree.h" +#include "plist.h" + +/* ------------------------------------------------------------------ */ +/* --wrap bookkeeping: record every malloc-returned pointer */ +/* ------------------------------------------------------------------ */ + +#define MAX_ALLOCS 4096 +static void *g_alloc_ptrs[MAX_ALLOCS]; +static int g_alloc_count = 0; +static int g_bad_frees = 0; + +void *__real_malloc(size_t size); +void __real_free(void *ptr); + +void *__wrap_malloc(size_t size) { + void *p = __real_malloc(size); + if (p && g_alloc_count < MAX_ALLOCS) + g_alloc_ptrs[g_alloc_count++] = p; + return p; +} + +void __wrap_free(void *ptr) { + if (!ptr) { __real_free(ptr); return; } + for (int i = 0; i < g_alloc_count; i++) { + if (g_alloc_ptrs[i] == ptr) { + g_alloc_ptrs[i] = NULL; /* consume entry */ + __real_free(ptr); + return; + } + } + /* ptr was never returned by malloc — this is a bad free */ + fprintf(stderr, "BAD FREE: %p was not a malloc-returned pointer\n", ptr); + g_bad_frees++; + /* do NOT call free — avoid crashing so we can report all failures */ +} + +static void reset_wrap_state(void) { + memset(g_alloc_ptrs, 0, sizeof(g_alloc_ptrs)); + g_alloc_count = 0; + g_bad_frees = 0; +} + +/* ------------------------------------------------------------------ */ +/* Minimal struct replicas (mirrors of the production types) */ +/* ------------------------------------------------------------------ */ + +/* ppagecache.c */ +typedef struct { + psync_list list; + uint64_t offset; + uint64_t length; +} test_request_range_t; + +typedef struct { + psync_list ranges; +} test_request_t; + +/* psyncer.c */ +typedef struct { + psync_tree tree; + unsigned long long folderid; +} test_synced_down_folder_t; + +/* ppathstatus.c */ +typedef struct { + psync_tree tree; + unsigned long long folderid; + int child_task_cnt; + int own_tasks; +} test_folder_tasks_t; + +/* pfs.c / pfscrypto.c */ +typedef struct { + psync_tree tree; + uint32_t sectorid; + uint32_t logoffset; +} test_sector_inlog_t; + +/* ------------------------------------------------------------------ */ +/* Free helpers — exact copies of the production fix */ +/* ------------------------------------------------------------------ */ + +static void free_request_range(test_request_range_t *r) { + pmem_free(PMEM_SUBSYS_CACHE, r); +} + +static void free_synced_down_folder(test_synced_down_folder_t *f) { + pmem_free(PMEM_SUBSYS_OTHER, f); +} + +static void free_folder_tasks_node(test_folder_tasks_t *ft) { + pmem_free(PMEM_SUBSYS_OTHER, ft); +} + +static void free_sector_inlog_node(test_sector_inlog_t *e) { + pmem_free(PMEM_SUBSYS_OTHER, e); +} + +/* ------------------------------------------------------------------ */ +/* Test harness */ +/* ------------------------------------------------------------------ */ + +static int passes = 0, failures = 0; + +#define PASS(n) do { printf("PASS: %s\n", n); passes++; } while (0) +#define FAIL(n, ...) do { printf("FAIL: %s — ", n); \ + printf(__VA_ARGS__); printf("\n"); failures++; } while (0) + +/* ------------------------------------------------------------------ */ +/* Test 1: psync_request_range_t list freed via free_request_range() */ +/* ------------------------------------------------------------------ */ +static void test_request_range_free(void) { + reset_wrap_state(); + + test_request_t req; + psync_list_init(&req.ranges); + + /* Allocate 5 range nodes (simulates a large multi-range read) */ + for (int i = 0; i < 5; i++) { + test_request_range_t *r = pmem_malloc(PMEM_SUBSYS_CACHE, + sizeof(test_request_range_t)); + r->offset = (uint64_t)i * 4096; + r->length = 4096; + psync_list_add_tail(&req.ranges, &r->list); + } + + /* Exercise the fixed free path */ + psync_list_for_each_element_call(&req.ranges, test_request_range_t, + list, free_request_range); + + if (g_bad_frees == 0) + PASS("request_range: pmem_free called with header ptr, not data ptr"); + else + FAIL("request_range", "%d bad free(s) detected", g_bad_frees); +} + +/* ------------------------------------------------------------------ */ +/* Test 2: synced_down_folder tree freed via free_synced_down_folder() */ +/* ------------------------------------------------------------------ */ +static void test_synced_down_folder_free(void) { + reset_wrap_state(); + + psync_tree *root = PSYNC_TREE_EMPTY; + + /* Build a small BST of synced_down_folder nodes */ + unsigned long long fids[] = {10, 5, 15, 3, 7}; + for (int i = 0; i < 5; i++) { + test_synced_down_folder_t *f = pmem_malloc(PMEM_SUBSYS_OTHER, + sizeof(test_synced_down_folder_t)); + f->folderid = fids[i]; + memset(&f->tree, 0, sizeof(f->tree)); + + if (!root) { + ptree_add_after(&root, NULL, &f->tree); + } else { + /* Simple insertion — walk tree */ + psync_tree *cur = root, **slot = NULL; + psync_tree *parent = NULL; + while (cur) { + test_synced_down_folder_t *n = + ptree_element(cur, test_synced_down_folder_t, tree); + parent = cur; + if (f->folderid < n->folderid) { + if (!cur->left) { slot = &cur->left; break; } + cur = cur->left; + } else { + if (!cur->right) { slot = &cur->right; break; } + cur = cur->right; + } + } + if (slot) { + *slot = &f->tree; + ptree_added_at(&root, parent, &f->tree); + } + } + } + + /* Exercise the fixed free path */ + ptree_for_each_element_call_safe(root, test_synced_down_folder_t, + tree, free_synced_down_folder); + + if (g_bad_frees == 0) + PASS("synced_down_folder: pmem_free called with header ptr, not data ptr"); + else + FAIL("synced_down_folder", "%d bad free(s) detected", g_bad_frees); +} + +/* ------------------------------------------------------------------ */ +/* Test 3: folder_tasks_t tree freed via free_folder_tasks_node() */ +/* ------------------------------------------------------------------ */ +static void test_folder_tasks_free(void) { + reset_wrap_state(); + + psync_tree *root = PSYNC_TREE_EMPTY; + + /* Allocate 6 folder_tasks nodes */ + unsigned long long fids[] = {100, 50, 150, 25, 75, 125}; + for (int i = 0; i < 6; i++) { + test_folder_tasks_t *ft = pmem_malloc(PMEM_SUBSYS_OTHER, + sizeof(test_folder_tasks_t)); + ft->folderid = fids[i]; + ft->child_task_cnt = 0; + ft->own_tasks = 0; + memset(&ft->tree, 0, sizeof(ft->tree)); + + if (!root) { + ptree_add_after(&root, NULL, &ft->tree); + } else { + psync_tree *cur = root, **slot = NULL; + psync_tree *parent = NULL; + while (cur) { + test_folder_tasks_t *n = + ptree_element(cur, test_folder_tasks_t, tree); + parent = cur; + if (ft->folderid < n->folderid) { + if (!cur->left) { slot = &cur->left; break; } + cur = cur->left; + } else { + if (!cur->right) { slot = &cur->right; break; } + cur = cur->right; + } + } + if (slot) { + *slot = &ft->tree; + ptree_added_at(&root, parent, &ft->tree); + } + } + } + + /* Exercise the fixed free path */ + ptree_for_each_element_call_safe(root, test_folder_tasks_t, + tree, free_folder_tasks_node); + + if (g_bad_frees == 0) + PASS("folder_tasks: pmem_free called with header ptr, not data ptr"); + else + FAIL("folder_tasks", "%d bad free(s) detected", g_bad_frees); +} + +/* ------------------------------------------------------------------ */ +/* Test 4: psync_sector_inlog_t tree freed via free_sector_inlog_node() */ +/* ------------------------------------------------------------------ */ +static void test_sector_inlog_free(void) { + reset_wrap_state(); + + psync_tree *root = PSYNC_TREE_EMPTY; + + /* Allocate 4 sector_inlog nodes */ + uint32_t sids[] = {0, 1, 2, 3}; + for (int i = 0; i < 4; i++) { + test_sector_inlog_t *e = pmem_malloc(PMEM_SUBSYS_OTHER, + sizeof(test_sector_inlog_t)); + e->sectorid = sids[i]; + e->logoffset = (uint32_t)(i * 512); + memset(&e->tree, 0, sizeof(e->tree)); + + if (!root) { + ptree_add_after(&root, NULL, &e->tree); + } else { + psync_tree *cur = root, **slot = NULL; + psync_tree *parent = NULL; + while (cur) { + test_sector_inlog_t *n = + ptree_element(cur, test_sector_inlog_t, tree); + parent = cur; + if (e->sectorid < n->sectorid) { + if (!cur->left) { slot = &cur->left; break; } + cur = cur->left; + } else { + if (!cur->right) { slot = &cur->right; break; } + cur = cur->right; + } + } + if (slot) { + *slot = &e->tree; + ptree_added_at(&root, parent, &e->tree); + } + } + } + + /* Exercise the fixed free path */ + ptree_for_each_element_call_safe(root, test_sector_inlog_t, + tree, free_sector_inlog_node); + + if (g_bad_frees == 0) + PASS("sector_inlog: pmem_free called with header ptr, not data ptr"); + else + FAIL("sector_inlog", "%d bad free(s) detected", g_bad_frees); +} + +/* ------------------------------------------------------------------ */ +/* Test 5: bare free() on pmem_malloc pointer IS detected as bad-free */ +/* (verifies that the wrap harness itself is working) */ +/* ------------------------------------------------------------------ */ +static void test_harness_detects_bad_free(void) { + reset_wrap_state(); + + void *data = pmem_malloc(PMEM_SUBSYS_OTHER, 64); + /* Deliberately call bare free on the data pointer — should be caught */ + free(data); + + if (g_bad_frees == 1) + PASS("harness self-check: bare free(data_ptr) correctly flagged"); + else + FAIL("harness self-check", "expected 1 bad free, got %d", g_bad_frees); + + /* Reset so leak-sanitizer doesn't complain about the unfree'd block */ + reset_wrap_state(); +} + +/* ------------------------------------------------------------------ */ +int main(void) { + test_request_range_free(); + test_synced_down_folder_free(); + test_folder_tasks_free(); + test_sector_inlog_free(); + test_harness_detects_bad_free(); + + printf("\n%d passed, %d failed\n", passes, failures); + return failures ? 1 : 0; +}