naett has had a complete libcurl backend all along; we just never built it,
so Linux ran with HTTPS_NOT_AVAILABLE. That means no homebrew store over
HTTPS, and RetroAchievements talking to plain http://retroachievements.org.
libcurl is loaded with dlopen rather than linked, the same way we handle the
Vulkan loader, so it stays a soft dependency: we need the curl headers at
build time, but a build made here still starts on a machine without libcurl
installed - it just reports HTTPS as unavailable, exactly like today. Distro
packagers get the behavior they'd expect either way, and certificate
validation comes free from the system CA store.
New net::HTTPSAvailable() answers "did that work", and SDLMain folds it into
SYSPROP_SUPPORTS_HTTPS, which everything downstream already degrades on.
Four fixes to the backend itself, all noted in ext/naett/README-ppsspp.md:
- panic() called exit(1) on a pipe or curl_multi_perform failure. Taking the
emulator down because a download failed isn't acceptable - the backend now
disables itself and requests complete with naettGenericError.
- CURLINFO_RESPONSE_CODE writes a long into res->code, which is an int. Eight
bytes into four, getting away with it only because the next field absorbs
the zeroes.
- curl_easy_setopt is varargs and wants a long for these options; int literals
and int variables are UB on LP64.
- naettPlatformCloseResponse called through a null function pointer when
libcurl was missing. Found by testing that path, which segfaulted.
CI needs libcurl4-openssl-dev (curl-dev on Alpine) or it would quietly keep
building without HTTPS.
naett has been a submodule pinned at v0.3.3; upstream has had no commits since
April 2024, and we want to carry local changes (next up: a libcurl-backed HTTPS
path for Linux). It's ~1500 lines of MIT C, smaller than several things we
already vendor, so bring it in-tree and drop the submodule.
Also drop the generated single-file amalgam (naett.c) that every build system
was compiling, and build src/*.c directly instead - otherwise the file you edit
isn't the file that gets compiled, which is a trap for anyone patching this.
example/ and testrig/ (a whole Android Studio project) are gone with it.
Two changes were needed to make the sources build on their own, both noted in
ext/naett/README-ppsspp.md along with the upstream commit:
- naett_internal.h now includes naett.h, which the amalgam pulled in first.
- naett_linux.c now includes stdio.h/stdlib.h. It calls exit/calloc/realloc/
free/fprintf without ever including either, and only got away with it because
naett_core.c sat above it in the concatenation.
No functional change - Linux still has HTTPS_NOT_AVAILABLE set, so it doesn't
build naett at all yet.
Rename naett to naett-lib
Written as (2 || 4 || 8 || misaligned), so every 2/4/8-byte access got labeled
'(unaligned)' and a genuinely misaligned larger access never reached the struct
branch. Log text only.
A failed FindTransferFramebuffer leaves dstRect zero-initialized, and RASTER_COLOR is
0, so the channel check passes and vfb is read through a null pointer. Only reachable
for a depth-source transfer to an address with no matching framebuffer.
Check dstBuffer first, like every other use of dstRect.vfb below.
The per-game override filename from [games] went straight to LoadFromVFS with no
check, while the [hashes] filenames and ReplacedTexture::Prepare both run theirs
through HasParentDirComponent. For a directory-backed pack DirectoryReader resolves
it against the pack directory, so '../../..' reads anything on disk - and texture
packs are third-party downloads. Check it the same way. (Zip-backed packs weren't
affected.)
Turning replacement off mid-session did 'delete vfs_; vfs_ = nullptr;' without
updating the cached ReplacedTextures that hold the same pointer - LoadIni has a fixup
loop for exactly this when it swaps the VFS, and the disable path needed one too.
Decimate(ALL) right after doesn't help: it only frees their data, it doesn't erase
the entries. A texture still PENDING (or one whose try_lock in Decimate failed) then
used the freed VFS from a worker thread, or from ~ReplacedTexture's ReleaseFile.
Clear the back-pointers, and make the destructor and Prepare() tolerate a null one.
The POSIX handler returned early for any si_code other than SEGV_MAPERR/SEGV_ACCERR,
without passing the signal on. Returning from a fault handler re-executes the
faulting instruction, so anything that keeps faulting - an MTE fault on Android
arm64, a protection-key or shadow-stack fault - became a livelock at 100% CPU rather
than a crash with a usable report. It also swallowed SIGSEGV sent via kill()
(si_code SI_USER), and meant whatever handler was installed before us, such as a
crash reporter, never ran.
The code to chain properly was already there, just inside the 'we couldn't handle
this address' branch further down - factored out and used for both.
This branch is live on the Switch and the BSDs - ppsspp_config.h only maps LINUX to
Android and Linux - and three of its functions didn't do what their callers assume.
time_now_raw() is documented and used as nanoseconds, and from_time_raw() scales it
by 1/nanos, but this built a double of *seconds* and returned it through the uint64_t
return type: the fraction was truncated away and the result was off by a factor of a
billion. Return real monotonic nanoseconds, like every other platform branch.
time_now_unix_utc() just forwarded to time_now_raw(), which is now explicitly a
monotonic clock with no relation to the epoch (and before this, was seconds truncated
to a whole number). Read the wall clock.
Instant took gettimeofday's tv_usec into nsecs_ and stored tv_sec as the start, while
ElapsedNanos() subtracts those from clock_gettime(CLOCK_MONOTONIC) - so it mixed two
different clocks *and* two different units, then applied a microsecond borrow to a
nanosecond difference. Elapsed times were nonsense and could come out negative.
InitMemorySizeForGame read all of disc0:/UMD_DATA.BIN into a vector and then copied
it into a string, with no size limit, from an image we don't control - and the
DISC_ID that gets us here is equally forgeable, it just has to match one of the 16
g_HDRemasters entries. A real UMD_DATA.BIN is a few dozen bytes; anything larger is
a mistake or an attack, so check the size before reading.
The POSIX path used mlock() as though it were a mutex. mlock only pins pages in
RAM - it provides no mutual exclusion at all, so the read-modify-write of the
cross-process instance counter was unsynchronized. Two instances launched at the
same moment could both come away with PPSSPP_ID == 1, at which point both pass
IsFirstInstance() and write ppsspp.ini over each other, and both compute the same
adhoc local IP. Take an advisory lock on the shm fd instead. (The Windows path was
already fine - it uses a named mutex.)
Also, next/total are uint8_t in a segment that outlives the processes using it, so
next climbs across runs and wraps. Landing on 0 is worse than it looks: it isn't a
valid instance id, IsFirstInstance() fails, and config saving is silently disabled
from then on. Skip past it on wrap.
The x86-64 path searched for free memory near the code, and if it found some,
committed to it - if that VirtualAlloc failed, ptr was left null and we returned
null, never reaching the else branch that exists precisely to say "can still run,
thanks to RipAccessible".
Finding a free region isn't the same as being able to reserve it. VirtualAlloc
rounds a non-null lpAddress down to the 64K allocation granularity while
SearchForFreeMem only guarantees page alignment, so the rounded-down base can land
back inside a committed region; a concurrent allocation between the VirtualQuery and
the VirtualAlloc does it too. Callers don't check the result - AllocCodeSpace stores
it unchecked and the emitters write from there - so this turned into a wild write
rather than a clean JIT-unavailable fallback.
Init() only runs on game load and Shutdown() nulls lua_ back out, but ImDebugger::Frame
draws the Lua console outside its PSP_IsInited() block, and whether the console is open
is persisted config. So opening it, exiting to the menu, and typing anything other than
the built-in clear/help/history dereferenced null.
The three framebuffer upload paths took Memory::GetPointerUnchecked() on a
GE-supplied source address and then read height rows of it, without ever checking
that span was mapped. Only the destination was validated (and DoBlockTransfer's own
memcpy is carefully guarded, so the intent was clearly there). A copy whose source
starts near the end of RAM walks straight off the end of the view. Clamp the row
count to what's actually mapped, and warn when we do.
GhidraClient dereferenced getArray()->value for both "symbols" and "types" without a
null check, and the getTag() test underneath could never catch it - getArray() has
already filtered by tag, so it returns either a JSON_ARRAY node or nullptr. Any
HTTP 200 that parses as JSON but isn't the shape we expect - {}, a bare array, an
incompatible ghidra-rest-api, or the host/port pointed at some other JSON service -
crashed the worker thread. FetchTypes() runs first, so that's the one you'd hit.
RiscV and LoongArch CPU detection divided TotalLogicalCount() by ProcessorCount()
before checking it. ProcessorCount() returns 0 whenever /proc/cpuinfo can't be read
or doesn't parse, which is SIGFPE during static init of the cpu_info global - before
anything could handle it. The existing <= 0 guard sat after the division.
314 pspautotests pass; frametests show the same 3 pre-existing failures as master.
NewThreadExecutor::Run pushed a std::thread per connection and only ever joined them
in the destructor, so a server leaked a joinable thread object for every connection
it had ever served. Measured with 60 connect/disconnect cycles against the debugger:
handle count +60 before, +1 after. Each worker now flags itself done as its last act,
and Run() reaps the finished ones first. Only the accept thread calls Run(), so the
flag is the only thing that needs to be atomic.
Note this doesn't bound how many connections can be in flight at once - it just stops
the finished ones from piling up.
Separately, a received close code was echoed straight back. RFC 6455 7.4.1 reserves
1004, 1005, 1006 and 1015 for describing how a connection ended locally, so they must
never go on the wire - echoing one back would be our protocol violation rather than
the client's. Send PROTOCOL_ERROR when they give us something we can't repeat.
RequestHeader::GetParamValue indexed parts[1] without checking the size. A query
parameter with no '=' at all ("?foo") makes SplitString return a single element, so
both the DEBUG_LOG and the assignment read off the end of the vector. Nothing calls
GetParamValue today, so this is latent rather than live, but it's driven straight
off the request line.
The 64-bit frame length was assembled with header[n] << 24 on uint8_t values, which
promote to int - a byte >= 0x80 in the top position shifts into the sign bit and then
sign-extends when widened to uint64_t. The resulting size was always rejected, just
by the wrong check and via signed overflow to get there. Cast first.
OutputSink::Block() had the same shape as the InputSink one this branch already
fixed: a broken socket is reported ready immediately and forever, so waiting on it
is a spin. Bail if the sink already knows it's broken.
GetStringErrorMsg had the strerror_r result test backwards. The XSI variant returns
0 on success, so every successful lookup returned "Unknown error"; and under glibc
with _GNU_SOURCE the GNU variant is selected instead, which returns the message by
pointer and typically leaves the buffer untouched, so it returned an empty string.
Either way GetLastErrorMsg() was useless on Linux, Android and macOS. Pick the right
handling by overload resolution rather than guessing which signature we got.
KeyMap's "no gamepad button mapped to cancel" fallback pushed into confirmKeys
instead of cancelKeys - and pushed the confirm button. So unmapping cancel left no
gamepad way out of menus, and duplicated an entry in the confirm list.
ControlMapper::AddListener mutated listeners_ without taking mutex_, while
RemoveListener takes it and the input thread iterates the vector under it. Opening a
screen while an axis is moving could reallocate it mid-iteration. The comment about
piggybacking on a screenmanager mutex was stale - there isn't one.
Config's two std::stof calls on PostShaderSetting values ran on user-editable ini
text with no try/catch, so a malformed entry called std::terminate during startup
config load. Use the same checked sscanf that LoadGameConfig already uses.
(CmdLine.cpp and Compatibility.cpp have the same pattern; not touched here.)
The screenshot downscale path leaked its final buffer on every downscaled shot,
which savestate thumbnails hit on every save at 3x and above.
HandleUploadPost is registered unconditionally, so closing the Upload screen left an
unauthenticated file-write endpoint live for as long as anything else kept the server
up. Check the flag in the handler.
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01DCPmm7FoQUoqrbMdhfqhQ2
Same root cause as the websocket spin this branch fixes, different symptom, so it
belongs with the InputSink EOF change rather than apart from it.
ReadBinaryUntilTerminator() returns 0 both when nothing has arrived yet and when
the peer is gone, and the multipart loop only exits on finding its terminator - so
a cancelled browser upload left a handler thread spinning forever. That also hangs
web server shutdown, since it joins its handler threads.
Now that InputSink reports EOF, ask it.
(Moved here from misc-correctness-fixes, which will be rebased on top of this.)
Follow-up to the previous commit, from Nemoumbra's questions - which found a worse
spin than the one that fix addressed.
InputSink couldn't tell "nothing right now" from "peer is gone": Fill() treats
recv() == 0 as no data and only sets hasError_ on a real error. Block() then waits
with WaitUntilReady(), which reports a closed socket as ready immediately and
forever, so TakeExact() looped on it without ever returning. A client that
disconnects with half a frame buffered - easy to do while blasting messages - put
the server in an infinite loop inside TakeExact, never even returning to Process().
Measured 7.95 CPU-seconds over 8 seconds; 0.08 after.
So: track EOF explicitly (sticky atEnd_, exposed as AtEnd()), and have Block() give
up when nothing more can arrive.
That information was being thrown away in three more places:
* Process() only tried to fill when the sink was already empty, so a disconnect went
unnoticed for as long as there were leftovers - and if those leftovers were a
partial frame, the read above never completed. Always fill, and close once the
peer is gone and we've consumed what it sent.
* ReadPending() uses TakeAtMost(), which returns 0 both for "nothing right now" and
"nothing ever again", and then reported success having consumed nothing. Ask the
sink which it was.
* Both TakeExact() call sites answered a failed read with POLICY_VIOLATION, blaming
the client for a protocol error when it had simply disconnected. Check the sink
and report ABNORMAL when that's what happened.
Also stop queueing data once our own close frame is queued. RFC 6455 5.5.1 forbids
data frames after a close, and beyond the protocol, anything appended afterwards
keeps the buffers non-empty and starves the "everything is flushed" check that ends
the connection. Observed the server pumping 167MB of log broadcasts after being
asked to close.
The repeated close-and-discard is now one helper.
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01DCPmm7FoQUoqrbMdhfqhQ2
Reported by Nemoumbra: the debugger server could get stuck in a tight select()
loop after a lot of traffic, burning a core.
Once OutputSink hits a real send() error it latches hasError_, after which Flush()
returns immediately without consuming anything, so out_->Empty() is false forever.
Process() waited for that to empty before finishing the close, kept the fd in the
write set, and select() reports an errored socket as ready every time - so it
returned true on every lap without ever making progress, and WebSocketDebuggerLoop
span. This needs sentClose_ to be set for it to be unrecoverable, since otherwise
the read side notices the disconnect and closes; a client that sends CLOSE (or
trips a protocol error) while output is backed up gets exactly that. Reproduced
with a client that queues ~120MB of responses, sends CLOSE, then resets the
connection without reading: 6.02 CPU-seconds over 6 seconds before, 0.06 after.
Treat an output error as fatal to the connection instead.
Also, select() returning -1 always returned true, so any error that doesn't fix
itself (a bad fd rather than EINTR) was a second busy-loop with no wait at all.
EINTR retries, everything else closes.
Finally, SendFlush() erased the drained bytes off the front of outBuf_ every lap.
With a backlog that's a memmove of the whole buffer per lap, i.e. quadratic in the
backlog, which burns CPU on its own while draining a slow client. Track a consumed
offset and only compact once the dead prefix is worth reclaiming.
Co-Authored-By: Claude Opus 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01DCPmm7FoQUoqrbMdhfqhQ2
The positional serialization means swapping two Mount() calls breaks every old
savestate just as thoroughly as adding one does, so say so. Also note that the
mount order spans more than MountFileSystems() - the UMD mounts are added first,
by MountGameISO, before we get there.
Henrik Rydgård
dependabot[bot]
keycross
