The loop may run in a thread that belongs to another device (the picloud
dispatcher forwards data from one connection's read thread to another
connection's socket), so isThreadedReadStopping() alone cannot stop it:
it sees the wrong device's flag. Stalling for at most writeTimeout()
without progress makes every write finite; the receiver resynchronizes
on the next packet signature.
PICloudClient::internalDisconnect() cleared buff without mutex_buff
while the eth read thread appends to it concurrently (close() races
with in-flight Data frames), and bytesAvailable() read buff.size()
lock-free on both PICloudClient and PICloudServer::Client. Take the
lock in both places (mutex_buff made mutable for the const getter).
prog_s_mutex/prog_r_mutex were declared but never locked, so
concurrent send()/received()/clear()/receivePacketProgress() raced on
shared buffers and fragments of different senders interleaved on the
wire. Lock the receive state (stream/packet) with prog_r_mutex in
received()/clear()/receivePacketProgress() and the whole emission of
one packet with prog_s_mutex in send().
Additionally the size field of a packet taken from the stream was
only checked for negativity: a remote peer could declare INT_MAX and
the packer accumulated memory unboundedly, swallowing all following
data as phantom payload (permanent desync, no resync). Add a
max_payload_size cap (config, default 64MB) and resynchronize byte by
byte when the declared size exceeds it, so valid packets after the
corrupted one survive.
Also drop frames whose size does not fit the 32-bit wire format
instead of wrapping the length field.
Socket is non-blocking while BlockingWrite is set for every PIEthernet
(construct()), so a stalled peer made ::send return EAGAIN and the
retry loop spun forever. The thread could no longer be stopped by
stop()/stopAndWait() (interrupt only wakes event waits), which turned
into a permanent hang for PIStreamPacker-based users such as the
picloud dispatcher. Break out of the loop when the device is
stopping and return -1.
luaL_newstate() allocates a Lua state (~2-4 MB with libraries)
in the constructor, but there was no destructor to call lua_close().
Every PILuaProgram instance leaked its entire Lua state.
Add ~PILuaProgram() that calls lua_close(PRIVATE->lua_state).
received() acquired mutex_header and mutex_session in different orders
depending on packet type:
- pt_Data path: mutex_header → mutex_session
- pt_Start path: mutex_session → mutex_header
send_process() acquires mutex_session independently. When running
concurrently with received(), the reversed lock ordering creates a
classic ABBA deadlock scenario.
Fix: enforce consistent ordering (mutex_header → mutex_session) in
all code paths. Restructured pt_Start case and buildSession() to
always acquire mutex_header before mutex_session.
The background run() thread copies allConnections() then iterates
each connection's diags_ map. If a PIConnection is deleted from
another thread between the copy and the iteration, dereferencing
the pointer is UAF. Add null check and protect diags_ iteration
with __device_pool__ lock.
Flags break_, is_sending, is_receiving, is_pause are accessed from
both the send thread (send_process) and the read thread (received)
without synchronization. Plain bool reads/writes from multiple threads
is undefined behavior per C++ standard. Convert to std::atomic<bool>
with proper initializers.
The buffer was sized as floor(sz/4)*3 but the loop ran ceil(sz/4)
times, writing 3 bytes per iteration. For sz%4!=0, this wrote past
the buffer end. Also guarded sz<4 to avoid processing trivially
short or malformed input.
The function called unlock() on thread_mutex without a matching
lock(), causing UB on Windows (releasing an unowned critical
section) or silently dropping a user-held lock.
When elapsed time exceeded the timeout, the remaining milliseconds
expression went negative and was implicitly converted to DWORD,
wrapping to ~0xFFFFFFFF (5 days) — effectively INFINITE.
Now clamps to 0 and returns false when time has elapsed.
dirent** list was uninitialized, so scandir failure (returning -1)
left it with indeterminate value. The unconditional free(list) at
the end was then UB — typically heap corruption or crash.
pop_back() called resize(size_ - 1) without checking for empty.
On empty array, uint underflow produced UINT_MAX, causing
bytesInBits(UINT_MAX) overflow and subsequent OOB access.
pop_front() already had this guard; pop_back() was missing it.
Three complex type cases (pivComplexf, pivComplexd, pivComplexld)
lacked break statements, causing fallthrough to pivTime. Setting
a complex variant from string silently overwrote the value with
PITime::fromString(), corrupting the variant's type and data.
When mmap() returned MAP_FAILED, data stayed as (void*)-1, causing
subsequent read/write/close to pass the null guard and crash.
Now resets data=nullptr, and calls shm_unlink when owner=true to
avoid orphaned shared memory objects.
The float FFT plan generator wrote 8 ints per entry (indices +0..+7)
but declared entrysize=4, causing heap overwrite into the next entry
and incorrect plan array sizing.
processFile() accessed files_[id - 1] without validating id.
When id == 0 (from a crafted network packet), id - 1 = -1 caused
out-of-bounds access. Added bounds check: id must be in range
[1, files_.size()]. Stops receive and logs error on invalid id.
fork() can return -1 on error (too many processes, out of memory).
The old code treated pid_ == -1 as a successful fork, entering the
parent branch and calling waitpid(-1, ...) which waits for ANY
child process. Added explicit check: close pipes, free memory,
and return on fork failure.
In the blocking read path (timeout_ms <= 0), the while loop had no
exit condition for readDevice() returning 0 (EOF) or -1 (error).
This caused an infinite busy-loop at 100% CPU. Added 'else break'
to exit the loop on read failure, matching the timeout branch
which already handles this case.
readDevice() did pos += size instead of pos += ret. The clamping
on the next line (pos = min(pos, data_->size_s())) masked the
issue in practice, but the semantics were wrong. Aligns with
writeDevice() which correctly uses pos += rs.size_s().
open() returns -1 on error. The check fd != 0 treated -1 as a
valid fd (since -1 != 0 is true), causing devctl(-1, ...) and
close(-1) on QNX. Fixed to fd >= 0.
File descriptor 0 is stdin (a valid fd). Using 0 as the 'not-open'
sentinel caused isCreate() to return false and destroy() to skip
close() when pipe() happened to allocate fds {0, 1} (e.g. in daemons
with closed stdio). POSIX convention: -1 = invalid fd.
- toChar(): char v was uninitialized, causing UB when
sscanf fails to match (empty string). Initialize to 0.
- operator+=(PIConstChars): loop iterated l < d.size() instead of
l < str.size(), reading past the end of str after d.enlarge().
This is a heap buffer overread with undefined behavior.
andrey
peri4