diff --git a/packaging/gamescope/PKGBUILD b/packaging/gamescope/PKGBUILD index c86767e1..2c8f8615 100644 --- a/packaging/gamescope/PKGBUILD +++ b/packaging/gamescope/PKGBUILD @@ -19,7 +19,7 @@ pkgname=punktfunk-gamescope # bump it with the marker so pacman sees a new version when only our patches moved. _gsver=3.16.25 _gsrev=5fb8dce4a09d0a68d097b9faf9513782106bc843 -pkgver="${_gsver}.pfhdr4" +pkgver="${_gsver}.pfhdr5" # 2: patch 0006 (never destroy the Vulkan device/output at exit). No capability moved, so the # `.pfhdrN` level deliberately stays put — see README.md. # 3: pin moved 8c676c39 -> 5fb8dce4 (3.16.25-1 -> 3.16.25-11), which brings upstream's own @@ -28,7 +28,12 @@ pkgver="${_gsver}.pfhdr4" # screenshots, which are upstream's paths, not ours. Patch 0001 additionally now offers # `xBGR_210LE` BEFORE `xRGB_210LE` so a third-party consumer cannot pick the one NVIDIA fills # byte-reversed. Still no capability the host probes for, so `.pfhdrN` stays at 4. -pkgrel=3 +# +# pfhdr5 / rel 1: patch 0007 fixes the PipeWire `pw_buffer->user_data` use-after-free that aborted +# a session on every capture renegotiation — i.e. on every client connect, since the host sets the +# session to the client's mode. This one DOES move `.pfhdrN`, even though it adds no capability: +# every deployed pfhdr4 binary crash-loops, so an operator has to be able to tell them apart. +pkgrel=1 pkgdesc="gamescope with 10-bit BT.2020/PQ PipeWire capture, for punktfunk HDR streaming" arch=('x86_64' 'aarch64') url="https://git.unom.io/unom/punktfunk" diff --git a/packaging/gamescope/README.md b/packaging/gamescope/README.md index 3901074b..3f00b379 100644 --- a/packaging/gamescope/README.md +++ b/packaging/gamescope/README.md @@ -17,6 +17,7 @@ The patches here add the missing half, and nothing else. See | `0004-pipewire-optionally-composite-the-external-overlay-i.patch` | `--pipewire-composite-external-overlay` (off by default): paint the external overlay layer (mangoapp — the fps/stats readout) into the capture stream | **Yes** — same shape as the cursor patch, same argument | | `0005-punktfunk-stamp-the-version-banner-with-pfhdrN.patch` | Append `+pfhdr` to the `--version` banner | **No** — ours only, retired when the functional patches above land upstream | | `0006-punktfunk-never-destroy-the-Vulkan-device-or-output-.patch` | Give `g_device` and `g_output` storage that is never destroyed, so their destructors cannot call a Vulkan driver glibc has already unloaded at `exit()` | **Yes** — a plain static-destruction-order bug, not punktfunk-specific | +| `0007-pipewire-never-leave-pw_buffer-user_data-pointing-at.patch` | Associate `pw_buffer->user_data` with its `pipewire_buffer` for every path out of `add_buffer`, clear it in `remove_buffer` (the last point both halves are known), and null-check the consumers — killing the use-after-free that aborted the session on every capture renegotiation | **Yes** — a plain use-after-free in the PipeWire buffer lifecycle | ### Why the headless patch matters @@ -40,6 +41,32 @@ fixed-function front end with no blend stage. Painting the cursor into the node for the blend, and with it a full-frame pass per frame — a gamescope session becomes the first one that can be genuinely zero-copy end to end. +### Why the buffer-lifetime patch is the one that made sessions unusable + +Patch 0007 is not a refinement — without it a managed gamescope session dies on essentially every +client connect. The host sets the session to the client's mode, the mode change renegotiates the +PipeWire stream, and the renegotiation is exactly what trips upstream's dangling +`pw_buffer->user_data`. The signature to recognise: + +``` +punktfunk-gamescope: ../src/pipewire.cpp:88: void destroy_buffer(pipewire_buffer*): + Assertion `false' failed. +#5 destroy_buffer(pipewire_buffer*).cold +``` + +It is a use-after-free wearing an `assert(false); // unreachable` as a disguise — `buffer->type` +is read out of freed memory and falls off the end of the `switch`. A zeroed slot gives the SIGSEGV +variant of the same fault instead. Two traps when triaging it: + +* **It is not HDR-specific.** The abort was first seen right after a 10-bit stream negotiated, so + it looked like the HDR path and `PUNKTFUNK_GAMESCOPE_HDR=0` looked like a workaround. It is not + — the same abort reproduces on an SDR session with no `--hdr-enabled` in the command line. + Check the failing process's actual argv before believing an HDR association. +* **`gamescope-session-plus` hides it.** When our binary crash-loops, the session script retries + and eventually comes up on the *stock* `/usr/bin/gamescope` at its default 1920×1080. So the box + lands in a working-looking game mode at the wrong resolution and without any of these patches. + Read the banner in `~/.gamescope-stdout.log`, not the fact that a session exists. + ## Why the marker exists punktfunk decides a session's shape **before** the virtual display exists: the bit depth at @@ -130,6 +157,7 @@ They are gamescope's, not ours, and they vary by distro. Two shortcuts that work sudo dnf install -y dnf-plugins-core meson ninja-build glslc sudo dnf builddep -y gamescope sudo dnf install -y xorg-x11-server-Xwayland-devel # NOT pulled by builddep; wlroots needs it +sudo dnf install -y libstdc++-static # NOT pulled by builddep; see below # Arch / SteamOS — see the makedepends in ./PKGBUILD ``` @@ -140,6 +168,19 @@ we pin, so it can come up short. `xorg-x11-server-Xwayland-devel` is the one tha nor a xserver.wrap file was found`, several minutes into an otherwise clean run. If a different one surfaces, meson names it — install and re-run with `--srcdir` so the clone is not repeated. +⚠️ `libstdc++-static` is a **punktfunk** requirement, not gamescope's, so no builddep will ever pull +it: the build script links the C++ runtime statically on purpose (see the long comment beside +`LDFLAGS` in `build-punktfunk-gamescope.sh`). Without it meson fails at configure with a message +that names neither the flag nor the package (2026-08-10, Fedora 44): + +``` +ERROR: Compiler c++ cannot compile programs. + /usr/bin/ld.bfd: cannot find -lstdc++ +``` + +The cleanup trap deletes the temp checkout on failure, taking `meson-logs/meson-log.txt` with it — +so build with `--srcdir` when diagnosing, or the evidence is gone before you can read it. + `gamescope` needs `CAP_SYS_NICE` for its realtime priority; the distro packages set it on their own binary. Mirror it if you install ours system-wide: diff --git a/packaging/gamescope/patches/0005-punktfunk-stamp-the-version-banner-with-pfhdrN.patch b/packaging/gamescope/patches/0005-punktfunk-stamp-the-version-banner-with-pfhdrN.patch index 2f4b7724..30f727ae 100644 --- a/packaging/gamescope/patches/0005-punktfunk-stamp-the-version-banner-with-pfhdrN.patch +++ b/packaging/gamescope/patches/0005-punktfunk-stamp-the-version-banner-with-pfhdrN.patch @@ -17,18 +17,19 @@ The number is a monotonic patch-set revision, so one probe answers all of them: +pfhdr2 …and --pipewire-composite-cursor +pfhdr3 …and the headless connector advertises its mode + --custom-refresh-rates +pfhdr4 …and --pipewire-composite-external-overlay + +pfhdr5 …and the PipeWire buffer use-after-free is fixed (no new capability) NOT for upstream: drop this once the functional patches land there and plain version floors answer the same questions. --- - src/meson.build | 9 ++++++++- - 1 file changed, 8 insertions(+), 1 deletion(-) + src/meson.build | 10 +++++++++- + 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/meson.build b/src/meson.build index 662f752..12fd38a 100644 --- a/src/meson.build +++ b/src/meson.build -@@ -177,7 +177,14 @@ compiler_version = cc.version() +@@ -177,7 +177,15 @@ compiler_version = cc.version() vcs_tag_cmd = ['git', 'describe', '--always', '--tags', '--dirty=+'] vcs_tag = run_command(vcs_tag_cmd, check: false).stdout().strip() @@ -40,7 +41,8 @@ index 662f752..12fd38a 100644 +# +pfhdr2 — …and `--pipewire-composite-cursor` +# +pfhdr3 — …and the headless connector advertises its mode + `--custom-refresh-rates` +# +pfhdr4 — …and `--pipewire-composite-external-overlay` -+version_tag = vcs_tag + '+pfhdr4' + ' (' + compiler_name + ' ' + compiler_version + ')' ++# +pfhdr5 — …and the PipeWire buffer use-after-free is fixed (no new capability) ++version_tag = vcs_tag + '+pfhdr5' + ' (' + compiler_name + ' ' + compiler_version + ')' gamescope_version_conf = configuration_data() gamescope_version_conf.set('VCS_TAG', version_tag) diff --git a/packaging/gamescope/patches/0007-pipewire-never-leave-pw_buffer-user_data-pointing-at.patch b/packaging/gamescope/patches/0007-pipewire-never-leave-pw_buffer-user_data-pointing-at.patch new file mode 100644 index 00000000..1e795134 --- /dev/null +++ b/packaging/gamescope/patches/0007-pipewire-never-leave-pw_buffer-user_data-pointing-at.patch @@ -0,0 +1,132 @@ +From d3a1c36bf871aaaa4ad9c2baf7170bba69742189 Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Enrico=20B=C3=BChler?= +Date: Mon, 10 Aug 2026 01:55:00 +0200 +Subject: [PATCH] pipewire: never leave pw_buffer->user_data pointing at a + freed buffer + +A gamescope session driving a PipeWire capture consumer dies as soon as the +stream renegotiates: + + gamescope: ../src/pipewire.cpp:88: void destroy_buffer(pipewire_buffer*): + Assertion `false' failed. + #4 __assert_fail + #5 destroy_buffer(pipewire_buffer*).cold + #13 destroy_buffer(pipewire_buffer*) + +with a SIGSEGV variant of the same fault when the slot happens to be zeroed +instead of stale. On a streaming host this reproduces on essentially every +client connect, because the host sets the session to the client's mode and the +mode change is what renegotiates the stream. + +pw_buffer->user_data and the pipewire_buffer behind it are associated in +exactly one place - the bottom of stream_handle_add_buffer - but that +assignment sits AFTER all four `goto error` paths, and the `error:` label is a +bare `delete buffer`. Any failing add therefore leaves the slot holding either +nothing or the previous generation's freed pointer. + +The teardown path leaks the same association even when nothing fails. +stream_handle_remove_buffer clears buffer->buffer, which is the only route from +the pipewire_buffer back to its pw_buffer. If the buffer is still `copying` it +is not deleted here at all - the steamcompmgr thread deletes it later, by which +point the pw_buffer can no longer be reached to be cleared. The slot is then a +dangling pointer for the rest of its life. + +PipeWire recycles struct pw_buffer slots across renegotiations, so the next +remove_buffer on a recycled slot dereferences freed memory. buffer->type reads +as garbage, falls off the end of the switch, and hits the default: +assert(false). The assert(buffer->buffer == nullptr) at the top passes on the +way in, which is what makes this look like an "unreachable" assert rather than +the use-after-free it is. + +Fix the association rather than the symptom: + + - set pw_buffer->user_data immediately after the allocation, so it is valid + for every path out of add_buffer, and clear it on the error path; + - clear it in remove_buffer, the last moment at which both halves are still + known, whether or not the buffer is deleted there; + - null-check the two places that consume it. + +With user_data non-null iff a live pipewire_buffer exists for that slot, the +default: case is genuinely unreachable - so it logs and returns instead of +aborting, since losing a capture buffer should not take the session down. + +Upstream: yes - a plain use-after-free in the PipeWire buffer lifecycle, with +nothing punktfunk-specific about it. +--- + src/pipewire.cpp | 33 +++++++++++++++++++++++++++++---- + 1 file changed, 29 insertions(+), 4 deletions(-) + +diff --git a/src/pipewire.cpp b/src/pipewire.cpp +index 76b3ea8..faecf04 100644 +--- a/src/pipewire.cpp ++++ b/src/pipewire.cpp +@@ -51,8 +51,13 @@ static void destroy_buffer(struct pipewire_buffer *buffer) { + case SPA_DATA_DmaBuf: + break; // nothing to do + default: +- assert(false); // unreachable +- } ++ // A buffer that never reached the point of choosing a type owns nothing to ++ // release. The association fixes above should make this unreachable, but a ++ // capture stream is not worth aborting a whole session over. ++ pwr_log.errorf("destroy_buffer: buffer has no data type (%d); nothing to release", ++ (int) buffer->type); ++ break; ++ } + + // If out_buffer == buffer, then set it to nullptr. + // We don't care about the result. +@@ -179,6 +184,13 @@ static void request_buffer(struct pipewire_state *state) + } + + struct pipewire_buffer *buffer = (struct pipewire_buffer *) pw_buffer->user_data; ++ if (buffer == nullptr) { ++ // add_buffer bailed for this slot, so it carries no state. Hand it back ++ // rather than dereferencing nothing. ++ pwr_log.errorf("dequeued a buffer with no state; requeuing"); ++ pw_stream_queue_buffer(state->stream, pw_buffer); ++ return; ++ } + buffer->copying = true; + + // Past this exchange, the PipeWire thread shares the buffer with the +@@ -464,6 +476,10 @@ static void stream_handle_add_buffer(void *user_data, struct pw_buffer *pw_buffe + + struct pipewire_buffer *buffer = new pipewire_buffer(); + buffer->buffer = pw_buffer; ++ // Associate the two halves NOW, not after the last `goto error` below: every error ++ // path deletes this buffer, and a slot left holding a stale pointer is one the next ++ // remove_buffer dereferences. ++ pw_buffer->user_data = buffer; + buffer->video_info = state->video_info; + buffer->gamescope_info = state->gamescope_info; + +@@ -583,17 +599,26 @@ static void stream_handle_add_buffer(void *user_data, struct pw_buffer *pw_buffe + goto error; + } + +- pw_buffer->user_data = buffer; +- + return; + + error: ++ pw_buffer->user_data = nullptr; + delete buffer; + } + + static void stream_handle_remove_buffer(void *data, struct pw_buffer *pw_buffer) + { + struct pipewire_buffer *buffer = (struct pipewire_buffer *) pw_buffer->user_data; ++ if (buffer == nullptr) { ++ return; ++ } ++ ++ // The LAST point at which both halves of the association are still known. Clearing ++ // buffer->buffer below makes the pw_buffer unreachable from the pipewire_buffer, and ++ // a buffer that is still `copying` is deleted later, on the steamcompmgr thread — ++ // with no way to reach back here. Sever the link now or the slot is left pointing at ++ // memory that is about to be freed. ++ pw_buffer->user_data = nullptr; + + buffer->buffer = nullptr; +