fix(libc): keep guest environment strings alive across setenv and unsetenv

setenv replaced a variable's string with a new allocation and freed the
old one, and unsetenv freed it as well, so a pointer that getenv had
returned for that variable dangled. A guest that reads such a pointer
after setenv(name, value, 1) reads freed host memory: AddressSanitizer
reports a heap-use-after-free on that read.

FreeBSD 11.4's getenv.c, which holds the setenv, unsetenv and putenv the
console's libc exports next to getenv, never frees a variable's string
("No previous variables are ever freed here to avoid causing a
segmentation fault in a user's code", lib/libc/stdlib/getenv.c lines
449-520 at 8c639945a4). setenv and unsetenv mark the old string
inactive; setenv writes the new value over the latest string of that
name when the value fits and allocates a new string otherwise; putenv
strings stay the caller's and are only forgotten.

The guest environment now keeps every string it allocates with its value
capacity and an active flag and follows that code: getenv and the time
zone lookup read the name's latest active string, setenv reuses the
latest string of the name when it fits, an unset one included, and
otherwise appends a new one and deactivates the old, putenv replaces an
earlier putenv string or deactivates the setenv one, and unsetenv
deactivates them all. Built on the host next to this library, FreeBSD's
getenv.c gave the same results, errno, pointer reuse and contents of
every earlier pointer over 160,000 random calls once the putenv flag it
leaves uninitialized in new setenv strings was cleared; the debt entry
records that the console may allocate where this reuses.

guest_environment checks the reuse, that earlier values stay readable
after a longer setenv and after unsetenv, and that putenv and setenv
leave each other's strings alone.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01DXPnhogDW4zpeNCBro4Ctt

core/libs/prx/libc/src/Environment.cpp

@@ -1,10 +1,11 @@
 #include "prx/libc/include/Api.hpp"
 #include <cstdlib>
 #include <cerrno>
+#include <cstddef>
 #include <cstring>
-#include <algorithm>
 #include <memory>
 #include <mutex>
+#include <new>
 #include <string>
 #include <string_view>
 #include <vector>
@@ -13,15 +14,18 @@
 
 namespace {
 struct EnvironmentEntry {
-    std::unique_ptr<std::string> owned;
+    std::unique_ptr<char[]> owned;
     char* borrowed = nullptr;
-    char* Data() { return borrowed ? borrowed : owned->data(); }
-    bool Matches(std::string_view name) {
+    std::size_t valueSize = 0;
+    bool active = true;
+    char* Data() const { return borrowed ? borrowed : owned.get(); }
+    bool Matches(std::string_view name) const {
         const char* data = Data();
         const char* separator = std::strchr(data, '=');
         return separator && std::string_view(data, separator - data) == name;
     }
 };
+constexpr std::size_t missingEntry = static_cast<std::size_t>(-1);
 std::mutex environmentMutex;
 std::vector<EnvironmentEntry>& Environment() {
     static std::vector<EnvironmentEntry> entries;
@@ -31,28 +35,32 @@ bool ValidEnvironmentName(const char* name) {
     return name && *name && !std::strchr(name, '=');
 }
 int EnvironmentError(int error) { errno = error; return -1; }
+std::size_t LatestEntry(std::string_view name, bool onlyActive) {
+    const auto& entries = Environment();
+    for (std::size_t index = entries.size(); index-- > 0;)
+        if ((entries[index].active || !onlyActive) && entries[index].Matches(name)) return index;
+    return missingEntry;
+}
+char* ActiveValue(const char* name) {
+    const std::size_t found = LatestEntry(name, true);
+    return found == missingEntry ? nullptr : Environment()[found].Data() + std::strlen(name) + 1;
+}
 }
 
 extern "C" {
 bool CopyGuestEnvironment_nid_no_patch(const char* name, std::string& value) {
     if (!ValidEnvironmentName(name)) return false;
     std::lock_guard lock(environmentMutex);
-    for (auto& entry : Environment())
-        if (entry.Matches(name)) {
-            value = entry.Data() + std::strlen(name) + 1;
-            return true;
-        }
-    return false;
+    const char* found = ActiveValue(name);
+    if (!found) return false;
+    value = found;
+    return true;
 }
 
 char* APS5_VABI getenv_nid_postfix(const char* name) {
     if (!ValidEnvironmentName(name)) { errno = 22; return nullptr; }
-    try {
-        std::lock_guard lock(environmentMutex);
-        for (auto& entry : Environment())
-            if (entry.Matches(name)) return entry.Data() + std::strlen(name) + 1;
-        return nullptr;
-    } catch (const std::bad_alloc&) { errno = 12; return nullptr; }
+    std::lock_guard lock(environmentMutex);
+    return ActiveValue(name);
 }
 
 int APS5_VABI setenv_nid_postfix(const char* name, const char* value, int overwrite) {
@@ -60,25 +68,40 @@ int APS5_VABI setenv_nid_postfix(const char* name, const char* value, int overwr
     try {
         std::lock_guard lock(environmentMutex);
         auto& entries = Environment();
-        auto found = std::find_if(entries.begin(), entries.end(),
-            [name](auto& entry) { return entry.Matches(name); });
-        if (found != entries.end() && !overwrite) return 0;
-        EnvironmentEntry replacement{std::make_unique<std::string>(std::string(name) + '=' + value), nullptr};
-        if (found != entries.end()) *found = std::move(replacement);
-        else entries.push_back(std::move(replacement));
+        const std::size_t nameLength = std::strlen(name);
+        const std::size_t valueLength = std::strlen(value);
+        const std::size_t found = LatestEntry(name, false);
+        if (found != missingEntry) {
+            auto& entry = entries[found];
+            if (entry.active && !overwrite) return 0;
+            if (!entry.borrowed && entry.valueSize >= valueLength) {
+                std::memmove(entry.owned.get() + nameLength + 1, value, valueLength + 1);
+                entry.active = true;
+                return 0;
+            }
+        }
+        EnvironmentEntry created{std::make_unique<char[]>(nameLength + valueLength + 2), nullptr, valueLength};
+        std::memcpy(created.owned.get(), name, nameLength);
+        created.owned[nameLength] = '=';
+        std::memcpy(created.owned.get() + nameLength + 1, value, valueLength + 1);
+        entries.push_back(std::move(created));
+        if (found != missingEntry) {
+            if (entries[found].borrowed) entries.erase(entries.begin() + static_cast<std::ptrdiff_t>(found));
+            else entries[found].active = false;
+        }
         return 0;
     } catch (const std::bad_alloc&) { return EnvironmentError(12); }
 }
 
 int APS5_VABI unsetenv_nid_postfix(const char* name) {
     if (!ValidEnvironmentName(name)) return EnvironmentError(22);
-    try {
-        std::lock_guard lock(environmentMutex);
-        auto& entries = Environment();
-        const std::string key(name);
-        std::erase_if(entries, [&key](auto& entry) { return entry.Matches(key); });
-        return 0;
-    } catch (const std::bad_alloc&) { return EnvironmentError(12); }
+    std::lock_guard lock(environmentMutex);
+    auto& entries = Environment();
+    const std::string_view key(name);
+    std::erase_if(entries, [key](const EnvironmentEntry& entry) { return entry.borrowed && entry.Matches(key); });
+    for (auto& entry : entries)
+        if (entry.Matches(key)) entry.active = false;
+    return 0;
 }
 
 int APS5_VABI putenv_nid_postfix(char* text) {
@@ -87,12 +110,13 @@ int APS5_VABI putenv_nid_postfix(char* text) {
     try {
         std::lock_guard lock(environmentMutex);
         auto& entries = Environment();
-        const std::string_view name(text, separator - text);
-        auto found = std::find_if(entries.begin(), entries.end(),
-            [name](auto& entry) { return entry.Matches(name); });
-        EnvironmentEntry replacement{nullptr, text};
-        if (found != entries.end()) *found = std::move(replacement);
-        else entries.push_back(std::move(replacement));
+        const std::size_t found = LatestEntry(std::string_view(text, separator - text), true);
+        if (found != missingEntry && entries[found].borrowed) {
+            entries[found].borrowed = text;
+            return 0;
+        }
+        entries.push_back({nullptr, text});
+        if (found != missingEntry) entries[found].active = false;
         return 0;
     } catch (const std::bad_alloc&) { return EnvironmentError(12); }
 }

core/libs/tests/GuestEnvironment.cpp

@@ -60,4 +60,20 @@ int main() {
     for (int index = 0; index < 64; ++index)
         Require(setenv_nid_postfix(("ANYPS5_GUEST_ENV_GROW_4C27_" + std::to_string(index)).c_str(), "x", 1) == 0);
     Require(getenv_nid_postfix("Q7B") == kept && std::strcmp(kept, "b") == 0);
+    Require(setenv_nid_postfix("Q7C", "abc", 1) == 0);
+    char* const first = getenv_nid_postfix("Q7C");
+    Require(setenv_nid_postfix("Q7C", "xy", 1) == 0);
+    Require(getenv_nid_postfix("Q7C") == first && std::strcmp(first, "xy") == 0);
+    Require(setenv_nid_postfix("Q7C", "longer value", 1) == 0);
+    char* const second = getenv_nid_postfix("Q7C");
+    Require(second != first && std::strcmp(second, "longer value") == 0 && std::strcmp(first, "xy") == 0);
+    Require(unsetenv_nid_postfix("Q7C") == 0);
+    Require(getenv_nid_postfix("Q7C") == nullptr && std::strcmp(second, "longer value") == 0);
+    Require(setenv_nid_postfix("Q7C", "again", 0) == 0);
+    Require(getenv_nid_postfix("Q7C") == second && std::strcmp(second, "again") == 0);
+    char lent[] = "Q7C=lent";
+    Require(putenv_nid_postfix(lent) == 0);
+    Require(getenv_nid_postfix("Q7C") == lent + 4 && std::strcmp(second, "again") == 0);
+    Require(setenv_nid_postfix("Q7C", "own", 1) == 0);
+    Require(std::strcmp(getenv_nid_postfix("Q7C"), "own") == 0 && std::strcmp(lent, "Q7C=lent") == 0 && std::strcmp(second, "again") == 0);
 }

docs/dev/debt/libc.md

@@ -137,7 +137,7 @@
 - [asinf](../../../core/libs/prx/libc/src/Math.cpp) (libc) - for |x| > 1, the infinities and NaN, it stores `EDOM` before returning `(x - x) / (x - x)`, as the `libc.prx` bundled with eleven titles does, PPSA13934's included. Tiny arguments return `x` without raising `FE_INEXACT`. Otherwise this is FreeBSD msun's [`e_asinf.c`](https://github.com/freebsd/freebsd-src/blob/release/11.4.0/lib/msun/src/e_asinf.c), which gives the same bits as the `asinf` of the `libc.prx` bundled with PPSA07923 and PPSA05916 for all 2^32 arguments (run on a Windows host). The FMA-contracted builds bundled with PPSA13934 and PPSA09614 differ from it by 1 ulp on 391,714 arguments, so no single port matches every title's last bit.
 - [atanhf](../../../core/libs/prx/libc/src/Math.cpp) (libc) follows the `atanhf` of the `libc.prx` bundled with PPSA28319 (disassembly): FreeBSD msun's [`e_atanhf.c`](https://github.com/freebsd/freebsd-src/blob/59bc2f095289a10b8cfa55cbb2b45c3bd6c2d4ae/lib/msun/src/e_atanhf.c) calling msun's [`log1pf`](https://github.com/freebsd/freebsd-src/blob/59bc2f095289a10b8cfa55cbb2b45c3bd6c2d4ae/lib/msun/src/s_log1pf.c), whose paths for the positive arguments it passes are inlined here; the exported `log1pf` is still the host's. For |x| > 1, the infinities and NaN, Sony's error helper raises `FE_INVALID` and sets `errno` to `EDOM` before returning `(x - x) / (x - x)`, and for |x| = 1 it raises `FE_DIVBYZERO` and sets `errno` to `ERANGE` before returning `x / 0`; msun itself sets no `errno`. That code, run on a Windows host, gives the same bits, `errno` and floating-point exceptions as this implementation for all 2^32 arguments; it was not run on Linux. That the system `libSceLibcInternal`, which Dusklight (PPSA99305) imports while bundling a zero-return `libc.prx`, has the same code is assumed
 - [j0, j1, jn, y0, y1 and yn](../../../core/libs/prx/libc/src/Bessel.cpp) (libc) call the host's `sin`, `cos`, `sincos` and `log`, so the last bits can differ from the console. The rest follows the system `libc.prx` built as `J03093718` (disassembly), which given the console's own `sin`, `cos`, `sincos` and `log` gave the same bits and exception flags for 90 million arguments: msun's [`e_j0.c`, `e_j1.c` and `e_jn.c`](https://github.com/freebsd/freebsd-src/tree/release/10.0.0/lib/msun/src) with each multiply-add the console fuses done as one FMA. As in that release, before FreeBSD commit `186f620727`, `y0`, `y1` and `yn` return `-inf` for ±0 and `0x7ff8000000000000` for a negative argument without raising an exception. The order wraps at 32 bits as on the console: `jn(1 << 30, x)` never returns for 2^-29 <= |x| < 2^30
-- [getenv, setenv, unsetenv, putenv](../../../core/libs/prx/libc/src/Environment.cpp) (libc) keep one guest environment that starts empty, like the `environ` libkernel exports, and never reads or changes the host's. A title sees only the variables it sets itself; host variables such as `HOME` and `PATH` name host paths a guest cannot use (PS5MC, PPSA99420, exits with `XBMC_Run returned -1` when it gets the host's `HOME`). Whether the console starts a title with any variables set is not measured
+- [getenv, setenv, unsetenv, putenv](../../../core/libs/prx/libc/src/Environment.cpp) (libc) keep one guest environment that starts empty, like the `environ` libkernel exports, and never reads or changes the host's. A title sees only the variables it sets itself; host variables such as `HOME` and `PATH` name host paths a guest cannot use (PS5MC, PPSA99420, exits with `XBMC_Run returned -1` when it gets the host's `HOME`). Whether the console starts a title with any variables set is not measured. Value storage follows FreeBSD 11.4 [getenv.c](https://github.com/freebsd/freebsd-src/blob/8c639945a44588f6cfdddfb057bb5aad88735da0/lib/libc/stdlib/getenv.c#L449-L520), not measured on a console: no string is freed, so a pointer `getenv` returned stays readable after `setenv` or `unsetenv`, and `setenv` writes a value that fits over the latest string of that name. FreeBSD bases that reuse on a flag it leaves uninitialized in strings `setenv` adds, so the console may allocate where this reuses
 - [mergesort](../../../core/libs/prx/libc/src/FileAndHeap.cpp) (libSceLibcInternal, implemented in libc) checks its arguments like the console's `libSceLibcInternal.sprx` (SDK 4.5x build of `J03950001`, 0x762D0): an element size under 4 sets `errno` to `EINVAL` (22) and returns -1, an empty array returns 0, and a failed `malloc` of the count times the size plus 8 bytes returns -1. The sort is a stable bottom-up merge, not FreeBSD's natural merge with its insertion sort for short runs, so the comparator is called in a different order and number of times; the result is the same for a consistent comparator. A null base or comparator and a size product that overflows, which the console does not check, throw
 - [Exception runtime on macOS](../../../core/libs/prx/libc/src/exception/Exports.cpp) (libc) - on macOS an exception that a library throws with a plain `throw` and that nothing catches skips the guest's `std::set_terminate` handler and the guest runtime's report: it ends in libc++abi's terminate, because the host keeps libc++abi there, while on Linux and Windows libc's runtime takes these throws. The `SIGALRM` expiry of `alarm` and `setitimer` throws through the guest runtime on macOS; `NotImplemented_nid_no_patch` cannot, since host code catches its `std::runtime_error`, so an unimplemented call inside a `noexcept` export (`mmap` on an `app0.zar` file) bypasses the guest handler
 - On macOS the [write watch](../../../core/libs/prx/libc/src/GuestWriteWatch.cpp) write-protects watched pages and records the first write to each from a SIGBUS/SIGSEGV handler, because macOS has no counterpart to `userfaultfd` write-protect or `MEM_WRITE_WATCH`. The protection is real, so: