Replace malloc'd receive buffer with stack array, fix the broken
validation (msg->length compared against POVERLAY_BUFSIZE rather
than max_value_size which excluded the header), fix the payload-read
loop target (msg->length already includes the header, so the old
`header_size + msg->length` limit over-read), add the missing
`msg->length > total_read` guard before memcpy, and add EINTR
handling in the read loop. Eliminates heap over-read when the daemon
sends msg->length > POVERLAY_BUFSIZE.
Ref GH #184.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* Fix pcl-3la.1: saturating addition for bytestou in pstatus_upload_recalc
Replace unchecked uint64_t summation in pstatus_upload_recalc() with
overflow-safe saturating addition. Before adding each file's size,
check if the result would wrap around UINT64_MAX; if so, saturate to
UINT64_MAX instead of silently wrapping.
Ref GH #293.
* Fix pcl-3la.2: remove spurious fuse_loop_mt while-loop and exit(0)
fuse_loop_mt() already loops internally; wrapping it in an additional
while loop caused repeated re-entry after normal exit. Also removes
the exit(0) call in the shutdown path so fuse_destroy() and other
cleanup run properly on shutdown.
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>
Replace indefinite pthread_cond_wait() calls in pstatus_wait() with
pthread_cond_timedwait() using a 5-second timeout computed via
clock_gettime(CLOCK_REALTIME). On timeout, the while loop re-evaluates
the wait condition, preventing indefinite hang when the state machine
stalls.
Ref GH #292.
Co-authored-by: Levi Neely <lkn@darkstar.example.net>
- Use sig_atomic_t flag instead of calling unsafe functions in signal handlers
- Remove pdbg_logf() and exit() calls from psync_signal_handler
- Check shutdown_requested flag in main loops
- Export flag for cross-module access
Co-authored-by: Levi Neely <lkn@darkstar.example.net>
- connect_res(): add SO_ERROR check after EINPROGRESS + wait_writable;
a refused/reset connection makes the socket writable with SO_ERROR
set — previously returned the socket as 'connected' when it was not.
Added psock_check_so_error() helper using getsockopt(SO_ERROR).
- wait_readable(), wait_writable(), wait_ssl_ready(): wrap select() in
do{...}while(errno==EINTR) with fd_set reinit on each iteration.
Signal delivery during blocking waits previously caused immediate
SOCKET_ERROR return, dropping live connections on any signal.
Linux updates tv on EINTR to reflect remaining time; reinitializing
fd_set before each retry ensures a clean select state.
- psync_socket_read_plain(), psync_socket_readall_plain(),
psync_socket_readall_plain_thread(): add EINTR to EAGAIN/EWOULDBLOCK
retry condition on read(). EINTR is a non-fatal interruption; retrying
is correct and consistent with the non-blocking socket model.
- psync_socket_read_noblock_plain(): return PSYNC_SOCKET_WOULDBLOCK on
EINTR rather than -1 (fatal error); callers handle WOULDBLOCK by
scheduling a retry.
- psync_socket_writeall_plain(), psync_socket_writeall_plain_thread():
add EINTR retry path on write() without a write-timeout wait, since
EINTR does not indicate the socket became unwritable.
- psock_connect(): add null check on malloc(sizeof(psock_t)); on OOM,
close the connected fd and free the SSL context before returning NULL.
Co-authored-by: Levi Neely <lkn@darkstar.example.net>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
prpc.c:
- on_request: fix total_size computation. The original formula
sizeof(uint32_t)+sizeof(uint64_t)+response->length had two bugs:
(1) sizeof(uint32_t)+sizeof(uint64_t)=12 but offsetof(rpc_message_t,value)=16
due to struct alignment padding between the uint32_t type and uint64_t length
fields; and (2) respond() was storing full message size in response->length,
double-counting the header. Fix: use offsetof(rpc_message_t,value)+response->length,
consistent with readResponse() in rpcclient.cpp which reads a fixed header_size
of offsetof(rpc_message_t,value) bytes then reads msg->length payload bytes.
- respond: store payload length only in response->length (value_length+1),
not full message size, to match the client protocol expectation.
- prpc_init: add null check on malloc return value.
- prpc_register: restore old handler table and return -1 if prpc_init fails.
papi.c:
- papi_result_thread: add missing MAX_API_RESPONSE_SIZE guard (present in
papi_result but absent here), preventing server-controlled unbounded malloc.
- papi_result, papi_result_thread: add null checks on malloc before passing
pointer to psock_readall.
- papi_result_async: add MAX_API_RESPONSE_SIZE check and malloc null check
on reader->respsize path.
- calc_ret_len: add _NEED_DATA(1) guard before ARRAY and HASH while-loop
conditions; empty containers with datalen=0 caused out-of-bounds read.
Co-authored-by: Levi Neely <lkn@darkstar.example.net>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
status_change_thread reads psync_status.filestodownload, filestoupload,
localisfull, and remoteisfull under statusmutex, while pstatus_download_recalc,
pstatus_upload_recalc, and pstatus_set write them without statusmutex (or under
a different mutex). TSan reports the race at pqevent.c:274.
Replace all writes of these four fields with __atomic_store_n and capture
atomic snapshots in status_change_thread before the condition, using the
snapshots for both comparison and the status_old update after struct copy.
Also use atomic loads in calc_status() for filestodownload and filestoupload.
Closes#335
Co-authored-by: Levi Neely <lkn@darkstar.example.net>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
psync_status.status is written in pstatus_set() (pstatus.c:263) after
releasing status_internal_mutex, while status_change() (pclsync_lib.cpp:435)
reads it concurrently from the callback thread without any lock. TSan
reports the race between T9 (write in pstatus_set) and the status callback
thread (read in status_change).
Replace all reads and writes of psync_status.status with
__atomic_load_n/__atomic_store_n (__ATOMIC_RELAXED) across pstatus.c,
pqevent.c, and pclsync_lib.cpp. In status_change_thread, capture the
atomic value once into cur_status before the condition to avoid multiple
inconsistent loads. In status_change(), capture cur_status at entry and
use it throughout, also propagating it to the copied status_ struct.
Closes#334
Co-authored-by: Levi Neely <lkn@darkstar.example.net>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
psync_current_time is a global time_t written by timer_thread without
holding timer_mutex, while ptimer_register reads it under timer_mutex.
TSan reports the race at ptimer.c:152 (write) and ptimer.c:216 (read).
Replace all reads with __atomic_load_n and all writes with
__atomic_store_n using __ATOMIC_RELAXED. This covers the race sites in
ptimer.c and all external readers in plocalscan.c, pnetlibs.c, and
psynclib.c that access psync_current_time without any mutex.
Closes#333
Co-authored-by: Levi Neely <lkn@darkstar.example.net>
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Acquire mutex before reading tm->refcnt to prevent race with
psync_task_dec_refcnt(). Fixes ThreadSanitizer warning.
Fixes#332
Co-authored-by: Levi Neely <lkn@darkstar.example.net>
In psync_unlink(), putil_wipe(psync_my_pass, sizeof(psync_my_pass))
only wiped 8 bytes (pointer size) instead of the full password string.
Changed to strlen(psync_my_pass) and added NULL check.
Fixes pcl-zqv.9.1 (partial)
GH #276
Co-authored-by: Levi Neely <lkn@darkstar.example.net>
strcmp() at line 315 short-circuits on first differing byte, leaking
timing info about trusted fingerprints.
Replace with constant-time comparison using memcmp and bitwise OR
accumulation across all trusted entries.
Fixes GH #239
Co-authored-by: Levi Neely <lkn@darkstar.example.net>
reader->data = malloc(reader->respsize) at line 462 is not checked
for NULL before goto again continues the loop and dereferences it.
Add NULL check and return ASYNC_RES_READY with result=NULL on failure.
Fixes GH #238
Co-authored-by: Levi Neely <lkn@darkstar.example.net>
cbidx = request->type - 20 computed before checking lower bound.
If request->type < calbacks_lower_band, cbidx wraps to large value
causing out-of-bounds array access.
Move bounds check before subtraction and use calbacks_lower_band
instead of hardcoded 20.
Fixes GH #235
Co-authored-by: Levi Neely <lkn@darkstar.example.net>
msgErr was declared as char[1024] but cast to char** and passed to
ptools_backend_call(). The function writes a char* into *err,
corrupting the first sizeof(char*) bytes of the array.
Change to char *errPtr = NULL and free it after use.
Fixes GH #216
Co-authored-by: Levi Neely <lkn@darkstar.example.net>
Loop variable j was declared inside the loop and reset to 0 on every
iteration. All branches had early continue, so j++ never executed.
Change j to i - reqParCnt to correctly index into optionalParams array.
Also fix paramtype check to use j instead of i.
Fixes GH #215
Co-authored-by: Levi Neely <lkn@darkstar.example.net>
When eventid has PEVENT_TYPE_FOLDER set, the code allocated
psync_file_event_t size but cast to psync_folder_event_t.
Swap the sizeof() calls to match the actual struct types used.
Fixes GH #229
Co-authored-by: Levi Neely <lkn@darkstar.example.net>
Three functions had a consistent typo (missing 'i' in builder):
- psync_list_bulder_push_num
- psync_list_bulder_pop_num
- psync_list_bulder_add_element
Renamed to correct spelling in plist.h, plist.c, and all callers.
Fixes GH #219
Co-authored-by: Levi Neely <lkn@darkstar.example.net>
excepions is read without holding timer_ex_mutex, but
ptimer_exception_handler() modifies it under the mutex.
Move the read inside the critical section to prevent the race.
Fixes GH #227
Co-authored-by: Levi Neely <lkn@darkstar.example.net>
pthread_create() return value is ignored. If it fails, the allocated
thread_data is leaked.
Check return value; on error, log and free the data.
Fixes GH #221
Co-authored-by: Levi Neely <lkn@darkstar.example.net>
keyParams is only allocated when pCnt > 0 but freed unconditionally.
When pCnt == 0, free() is called on an uninitialized pointer (UB).
Initialize to NULL so free() is a no-op when allocation is skipped.
Fixes GH #214
Co-authored-by: Levi Neely <lkn@darkstar.example.net>
mmap(2) returns MAP_FAILED ((void*)-1) on error, not NULL. The check
'if (likely(ret))' treats MAP_FAILED as truthy, so the emergency retry
path is never reached and callers receive MAP_FAILED as a valid pointer.
Normalize pmem_mmap() to return NULL on failure (converting MAP_FAILED
to NULL in the mmap path). This confines the platform-specific error
handling to one place and eliminates the need for preprocessor checks
in pmem_mmap_safe() and psync_mmap_anon_emergency().
Fixes GH #226
Co-authored-by: Levi Neely <lkn@darkstar.example.net>
- Wrap std::stoull() in try/catch to prevent daemon crash on invalid folder ID
- Add proper read loop for RPC messages to handle partial reads
- Validate msg->length before memcpy to prevent heap over-read
- Handle daemon bugs gracefully (EOF before full message)
- Fix operator precedence in sync remove command
Fixes#205, #206
Co-authored-by: Levi Neely <lkn@darkstar.example.net>
- Add return statement in catch(...) block to prevent fallthrough
- Fix operator precedence in GetState() to capture Call() return value
Fixes#201, #202
Co-authored-by: Levi Neely <lkn@darkstar.example.net>
- Add return statement in catch(...) block to prevent fallthrough
- Fix operator precedence in GetState() to capture Call() return value
Fixes#201, #202
Co-authored-by: Levi Neely <lkn@darkstar.example.net>
Levi Neely
Levi Neely