From ee06985d4b4c0a69a0fec75afad9546504710638 Mon Sep 17 00:00:00 2001 From: Mariano Abad Date: Thu, 1 Oct 2026 13:08:21 -0300 Subject: [PATCH] release 0.5.10: a setuid, setgid or file-capability binary ignores the environment and loads no GL open_drm_auto() read DRM_DEVICE whenever the process was not root. A binary that gained privilege when it started (setuid, setgid, or CAP_SYS_ADMIN through file capabilities) runs with an environment the invoking user sets. Read DRM_DEVICE and the DRMTAP_* variables with secure_getenv(), which returns nothing for such a binary, and do not load the GL libraries there: they read their own environment. A scanout that needs a GPU detile fails closed in such a process with an error that says why. README, SECURITY.md, CONTRIBUTING.md, the header and a helper comment now say what the code does. --- CHANGELOG.md | 28 +++++++++++++++++++ README.md | 4 ++- SECURITY.md | 6 ++-- bindings/rust/libdrmtap-sys/Cargo.toml | 2 +- bindings/rust/libdrmtap-sys/README.md | 5 +++- bindings/rust/libdrmtap-sys/csrc/drm_grab.c | 16 +++++++++-- .../rust/libdrmtap-sys/csrc/drmtap-helper.c | 5 ++-- bindings/rust/libdrmtap-sys/csrc/drmtap.c | 13 +++++---- bindings/rust/libdrmtap-sys/csrc/drmtap.h | 9 ++++-- bindings/rust/libdrmtap-sys/csrc/gpu_egl.c | 11 ++++++-- bindings/rust/libdrmtap/Cargo.toml | 4 +-- bindings/rust/libdrmtap/README.md | 5 +++- contrib/integrations/rustdesk/README.md | 3 +- helper/drmtap-helper.c | 5 ++-- include/drmtap.h | 9 ++++-- meson.build | 2 +- src/drm_grab.c | 16 +++++++++-- src/drmtap.c | 13 +++++---- src/gpu_egl.c | 11 ++++++-- 19 files changed, 125 insertions(+), 42 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4d392f7..8598d09 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,33 @@ the `libdrmtap` wrapper crate all share ONE version. 0.5.0 declared that move an did not complete it - the wrapper still shipped 0.3.4 pinned to a `-sys` range that could not reach 0.5.0 - so the shared line only actually holds from 0.5.1. +## [0.5.10] - 2026-10-01 + +### Security: a setuid, setgid or file-capability binary ignores the environment and loads no GL + +When the library picks the card itself (no `device_path` in the config), it read +`DRM_DEVICE` whenever the process was not root. A program that holds `CAP_SYS_ADMIN` +through file capabilities, is setuid to a user other than root, or is setgid, runs +with an environment the invoking user sets, so that user could choose which node it +opens. The library now reads `DRM_DEVICE` and its own `DRMTAP_*` variables with +`secure_getenv()`, which returns nothing in a setuid (root or not), setgid or +file-capability binary. Root was already covered for `DRM_DEVICE` (0.4.11). A program +can still turn on logging with `drmtap_config.debug`. + +In such a binary the library also no longer loads the GL libraries for the EGL detile: +they are third-party code that reads its own environment variables. Grabs there take +the paths of a build without EGL: a CPU deswizzle where one exists, a fail-closed error +where none does (measured on i915: -ENOTSUP, with an error that names the cause), and a +framebuffer that states no modifier is read as linear. 0.5.9 detiled in that process. +A consumer that needs the detile runs unprivileged and grabs through the helper, which +loads no GL. A process that runs as root without a setuid bit is unchanged. + +The README said `DRM_DEVICE` is ignored for "root / `CAP_SYS_ADMIN`" callers; it now +names the cases the code covers. SECURITY.md said the variable cannot redirect which +device the helper opens; in the helper model it can, because the helper opens the path +the unprivileged library picked, and the helper still refuses anything outside +`/dev/dri/`. + ## [0.5.9] - 2026-10-01 ### Added: `drmtap_crtc_refresh()`, the exact refresh of the captured CRTC @@ -996,6 +1023,7 @@ entry point is additive and would not on its own have justified more than a patc - amdgpu EGL detile fix, privileged-helper hardening, and a batch of full-audit fixes. +[0.5.10]: https://github.com/fxd0h/libdrmtap/releases/tag/v0.5.10 [0.5.9]: https://github.com/fxd0h/libdrmtap/releases/tag/v0.5.9 [0.5.8]: https://github.com/fxd0h/libdrmtap/releases/tag/v0.5.8 [0.5.7]: https://github.com/fxd0h/libdrmtap/releases/tag/v0.5.7 diff --git a/README.md b/README.md index 47bd7f3..b05a821 100644 --- a/README.md +++ b/README.md @@ -305,12 +305,14 @@ meson test -C build --suite integration | Variable | Effect | |---|---| -| `DRM_DEVICE` | DRM node to open (e.g. `/dev/dri/card0`). **Ignored for privileged (root / `CAP_SYS_ADMIN`) callers** — a privileged capture service uses its configured `device_path` or the KMS auto-scan, so the environment cannot redirect which device it opens. Honored only for unprivileged runs. | +| `DRM_DEVICE` | DRM node to open (e.g. `/dev/dri/card0`). **Ignored for privileged callers** (root, or a setuid, setgid or file-capability binary) — a privileged capture service uses its configured `device_path` or the KMS auto-scan, so the environment cannot redirect which device it opens. Honored only for unprivileged runs. | | `DRMTAP_DEBUG` | Set to `1` for verbose debug logging on stderr. | | `DRMTAP_NO_EGL` | Set to `1` to force the CPU deswizzle/convert path (skip EGL/GLES). | | `DRMTAP_NO_IMAGE_CACHE` | Set to `1` to re-import the `EGLImage` every frame (disable the import-once cache). | | `DRMTAP_FORCE_MMAP_FAIL` | Test hook — set to `1` to force the fast-path CPU mmap to fail so the EGL-detile fallback runs. | +A setuid, setgid or file-capability binary ignores all of these: the library reads them with `secure_getenv()`. + The privilege model is described in [`SECURITY.md`](SECURITY.md). ## Performance diff --git a/SECURITY.md b/SECURITY.md index 69c443d..b11594e 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -77,8 +77,9 @@ At startup, in this order (`main()` in `helper/drmtap-helper.c`): 2. **Restricts the device path** — the device comes solely from `argv[1]` (the path the library selected). The helper does not read `DRM_DEVICE` from the environment (0.4.14 hardening), and the library itself ignores `DRM_DEVICE` - when privileged (since 0.4.11), so an env var cannot redirect which device the - privileged process opens. The still attacker-influenceable `argv[1]` is + for root (since 0.4.11) and for a setuid, setgid or file-capability binary + (since 0.5.10), but the helper opens the path the unprivileged library picked, + which can come from `DRM_DEVICE`. That attacker-influenceable `argv[1]` is canonicalized with `realpath()`, and the helper **refuses any path that does not resolve under `/dev/dri/`** before opening it. 3. **`PR_SET_NO_NEW_PRIVS`** — set via `prctl`, before seccomp; hard-fails if it @@ -246,6 +247,7 @@ Do not read the above as more locked down than it is: | Helper runs unconfined if hardening fails | Hard-fails: refuses to serve if cap-drop or seccomp cannot be established | | Third party intercepts frames or connects to the helper | Anonymous inherited socketpair, not discoverable, no listening socket | | Replacing the helper binary | setcap xattrs are cleared by the kernel on file modification; re-applying needs root | +| A setuid, setgid or file-capability consumer steered through the environment of whoever runs it | Since 0.5.10 the library reads `DRM_DEVICE` and its `DRMTAP_*` variables with `secure_getenv()` and loads no GL libraries in such a process, so it gets no EGL detile there | ### NOT protected against diff --git a/bindings/rust/libdrmtap-sys/Cargo.toml b/bindings/rust/libdrmtap-sys/Cargo.toml index 4095314..baf0fd4 100644 --- a/bindings/rust/libdrmtap-sys/Cargo.toml +++ b/bindings/rust/libdrmtap-sys/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "libdrmtap-sys" -version = "0.5.9" +version = "0.5.10" links = "drmtap" edition = "2021" authors = ["Mariano Abad "] diff --git a/bindings/rust/libdrmtap-sys/README.md b/bindings/rust/libdrmtap-sys/README.md index 4b3e118..8e490c5 100644 --- a/bindings/rust/libdrmtap-sys/README.md +++ b/bindings/rust/libdrmtap-sys/README.md @@ -46,7 +46,10 @@ libdrmtap captures screen contents at the kernel level using DRM/KMS APIs. Unlik the headers are a build requirement and those two shared libraries are a RUNTIME requirement on the target (`libegl1` and `libgles2` on Debian/Ubuntu). Without them the EGL detile is unavailable and only the CPU paths remain, - which do not cover every scanout. + which do not cover every scanout. Since 0.5.10 the library + never loads them in a setuid, setgid or file-capability process, so there the + EGL detile is unavailable too: grab through the helper from an unprivileged + process instead. The build also compiles the privileged `drmtap-helper` binary from the same embedded sources, with exploit-mitigation hardening (stack-protector-strong, diff --git a/bindings/rust/libdrmtap-sys/csrc/drm_grab.c b/bindings/rust/libdrmtap-sys/csrc/drm_grab.c index b37869f..22989ab 100644 --- a/bindings/rust/libdrmtap-sys/csrc/drm_grab.c +++ b/bindings/rust/libdrmtap-sys/csrc/drm_grab.c @@ -36,6 +36,7 @@ #include #include #include +#include #include #include #include @@ -105,7 +106,7 @@ static void close_auxiliary_gem_handles(drmtap_ctx *ctx, const drmModeFB2 *fb2) static int drmtap_force_mmap_fail(void) { static int v = -1; if (v < 0) { - const char *e = getenv("DRMTAP_FORCE_MMAP_FAIL"); + const char *e = secure_getenv("DRMTAP_FORCE_MMAP_FAIL"); v = (e && e[0] == '1') ? 1 : 0; } return v; @@ -1696,8 +1697,17 @@ static int gpu_auto_process(drmtap_ctx *ctx, void *data, (unsigned long)modifier); #ifdef HAVE_EGL /* EGL IS compiled in, so the detile was skipped or it failed at - * runtime: no dma-buf fd on this path (helper V2 pixel mode), or no - * usable render node. Point at that, not at the build. */ + * runtime: GL is not loaded in a setuid, setgid or file-capability + * process; otherwise no dma-buf fd on this path (helper V2 pixel mode), + * or no usable render node. Point at that, not at the build. */ + if (getauxval(AT_SECURE)) { + drmtap_set_error(ctx, + "scanout modifier 0x%lx needs a GPU detile, and GL is not loaded " + "in a setuid, setgid or file-capability process: grab through the " + "helper from an unprivileged process instead", + (unsigned long)modifier); + return -ENOTSUP; + } drmtap_set_error(ctx, "scanout modifier 0x%lx needs a GPU detile, and the EGL detile " "this build carries was unavailable or failed: no dma-buf fd on " diff --git a/bindings/rust/libdrmtap-sys/csrc/drmtap-helper.c b/bindings/rust/libdrmtap-sys/csrc/drmtap-helper.c index d40dd50..096268c 100644 --- a/bindings/rust/libdrmtap-sys/csrc/drmtap-helper.c +++ b/bindings/rust/libdrmtap-sys/csrc/drmtap-helper.c @@ -993,8 +993,9 @@ int main(int argc, char *argv[]) { /* The device path comes solely from argv[1] -- the value the library selected * and passed. A CAP_SYS_ADMIN process must not take device selection from the - * environment (the library itself ignores DRM_DEVICE when privileged, since - * 0.4.11), so DRM_DEVICE is deliberately NOT honored here. */ + * environment (the library ignores DRM_DEVICE for root since 0.4.11 and for a + * setuid, setgid or file-capability binary since 0.5.10), so DRM_DEVICE is + * deliberately NOT honored here. */ /* The device path is attacker-influenceable (argv / DRM_DEVICE) and we run * with CAP_SYS_ADMIN, so refuse anything that does not canonicalize under diff --git a/bindings/rust/libdrmtap-sys/csrc/drmtap.c b/bindings/rust/libdrmtap-sys/csrc/drmtap.c index 05045a2..7fc1ba8 100644 --- a/bindings/rust/libdrmtap-sys/csrc/drmtap.c +++ b/bindings/rust/libdrmtap-sys/csrc/drmtap.c @@ -104,11 +104,12 @@ static int open_drm_auto(drmtap_ctx *ctx) { * capture service (root / effective-root, e.g. the unattended --service) * must not let an attacker-influenceable environment variable redirect which * device it opens; it relies on the explicit config device_path or the KMS - * auto-scan below instead. DRM_DEVICE stays honored for unprivileged test / - * dev runs. */ + * auto-scan below instead. secure_getenv() also hides it from a setuid, setgid + * or file-capability binary, whose environment the invoking user controls. + * DRM_DEVICE stays honored for unprivileged test / dev runs. */ const char *env_dev = NULL; if (getuid() != 0 && geteuid() != 0) { - env_dev = getenv("DRM_DEVICE"); + env_dev = secure_getenv("DRM_DEVICE"); } if (env_dev) { drmtap_debug_log(ctx, "trying DRM_DEVICE=%s", env_dev); @@ -292,7 +293,7 @@ drmtap_ctx *drmtap_open(const drmtap_config *config) { } /* Check DRMTAP_DEBUG env var */ - const char *dbg_env = getenv("DRMTAP_DEBUG"); + const char *dbg_env = secure_getenv("DRMTAP_DEBUG"); if (dbg_env && dbg_env[0] == '1') { ctx->debug = 1; } @@ -302,7 +303,7 @@ drmtap_ctx *drmtap_open(const drmtap_config *config) { DRMTAP_VERSION_PATCH, getpid(), getuid()); /* Open DRM device */ - drmtap_debug_log(ctx, "device_path=[%s] DRM_DEVICE=[%s]", ctx->device_path, getenv("DRM_DEVICE") ? getenv("DRM_DEVICE") : "(null)"); + drmtap_debug_log(ctx, "device_path=[%s]", ctx->device_path); if (ctx->device_path[0]) { /* Explicit device path */ ctx->drm_fd = open(ctx->device_path, O_RDWR | O_CLOEXEC); @@ -661,7 +662,7 @@ drmtap_ctx *drmtap_open_render(const char *render_node) { ctx->fast_slots[i].prime_fd = -1; } - const char *dbg_env = getenv("DRMTAP_DEBUG"); + const char *dbg_env = secure_getenv("DRMTAP_DEBUG"); if (dbg_env && dbg_env[0] == '1') { ctx->debug = 1; } diff --git a/bindings/rust/libdrmtap-sys/csrc/drmtap.h b/bindings/rust/libdrmtap-sys/csrc/drmtap.h index 21005c7..05cedc5 100644 --- a/bindings/rust/libdrmtap-sys/csrc/drmtap.h +++ b/bindings/rust/libdrmtap-sys/csrc/drmtap.h @@ -31,7 +31,7 @@ extern "C" { * site. */ #define DRMTAP_VERSION_MAJOR 0 #define DRMTAP_VERSION_MINOR 5 -#define DRMTAP_VERSION_PATCH 9 +#define DRMTAP_VERSION_PATCH 10 /** * @brief Get the library version as a packed integer. @@ -79,7 +79,9 @@ typedef struct { * one of the six. The only way to not have the list is to build with * -Dhelper=disabled, which compiles the fork/exec path out of the library * entirely - no fork, exec or socketpair symbol is left in the .so - and - * is what a consumer that already holds CAP_SYS_ADMIN should do. + * is what a consumer that already holds CAP_SYS_ADMIN as root should do + * (since 0.5.10 a setuid, setgid or file-capability binary loads no GL, so + * it gets no EGL detile in-process). * * Note for anyone who read an older header: it listed $DRMTAP_HELPER_PATH * and /drmtap-helper. Neither was ever implemented - the @@ -90,7 +92,8 @@ typedef struct { const char *helper_path; /** Enable debug logging to stderr. - * Can also be enabled with DRMTAP_DEBUG=1 env var. */ + * Can also be enabled with DRMTAP_DEBUG=1 env var, except in a setuid, + * setgid or file-capability binary (read with secure_getenv()). */ int debug; } drmtap_config; diff --git a/bindings/rust/libdrmtap-sys/csrc/gpu_egl.c b/bindings/rust/libdrmtap-sys/csrc/gpu_egl.c index af61f77..f1d1513 100644 --- a/bindings/rust/libdrmtap-sys/csrc/gpu_egl.c +++ b/bindings/rust/libdrmtap-sys/csrc/gpu_egl.c @@ -48,6 +48,7 @@ #ifdef HAVE_EGL #include +#include #include #include @@ -140,6 +141,12 @@ static int g_gl_libs_state; /* 0 = not attempted, 1 = loaded, -1 = failed */ * but it must stay textually ABOVE them so the pfn_ declarations resolve. */ static int load_gl_libraries(void) { static pthread_mutex_t lk = PTHREAD_MUTEX_INITIALIZER; + /* -3: not in a setuid, setgid or file-capability process. The GL libraries are + * third-party code that reads its own environment variables, and there the + * invoking user sets them. */ + if (getauxval(AT_SECURE)) { + return -3; + } pthread_mutex_lock(&lk); if (g_gl_libs_state == 0) { /* -1 = the libraries are not here at all (a missing package); -2 = they loaded but a @@ -662,7 +669,7 @@ static int egl_init(drmtap_ctx *ctx, egl_state_t *state) { state->tex_height = 0; /* Escape hatch: DRMTAP_NO_IMAGE_CACHE=1 forces a fresh EGLImage import * per frame (the pre-0.4.9 behaviour) for debugging driver oddities. */ - const char *nocache = getenv("DRMTAP_NO_IMAGE_CACHE"); + const char *nocache = secure_getenv("DRMTAP_NO_IMAGE_CACHE"); state->cache_disabled = (nocache && nocache[0] == '1'); state->initialized = 1; /* Register the thread-exit backstop so a capture thread that dies without @@ -1006,7 +1013,7 @@ int drmtap_gpu_egl_available(drmtap_ctx *ctx) { /* Escape hatch: DRMTAP_NO_EGL=1 forces the CPU deswizzle/convert path (used * to exercise or debug it, and by the convert tests/fuzzer which target the * untrusted-descriptor handling that lives on the CPU side). */ - const char *no_egl = getenv("DRMTAP_NO_EGL"); + const char *no_egl = secure_getenv("DRMTAP_NO_EGL"); if (no_egl && no_egl[0] == '1') { return 0; } diff --git a/bindings/rust/libdrmtap/Cargo.toml b/bindings/rust/libdrmtap/Cargo.toml index 72b5836..f69b740 100644 --- a/bindings/rust/libdrmtap/Cargo.toml +++ b/bindings/rust/libdrmtap/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "libdrmtap" -version = "0.5.9" +version = "0.5.10" edition = "2021" # std::os::fd (BorrowedFd/OwnedFd), which the frame's fd accessors are built on, is 1.66. # Declared so a downstream on an older toolchain gets that sentence instead of a type error. @@ -16,7 +16,7 @@ keywords = ["drm", "kms", "screen-capture", "wayland", "remote-desktop"] categories = ["multimedia::video", "os::linux-apis"] [dependencies] -libdrmtap-sys = { version = "0.5.9", path = "../libdrmtap-sys" } +libdrmtap-sys = { version = "0.5.10", path = "../libdrmtap-sys" } # errno values differ by architecture (ENOTSUP is 95 on x86, 45 on SPARC): compare by name. libc = "0.2" # Optional on purpose: a third-party type in a public signature ties our semver to theirs. See the diff --git a/bindings/rust/libdrmtap/README.md b/bindings/rust/libdrmtap/README.md index bbfcdea..f0532e6 100644 --- a/bindings/rust/libdrmtap/README.md +++ b/bindings/rust/libdrmtap/README.md @@ -109,7 +109,10 @@ fn main() -> Result<(), Box> { `libGLESv2.so.2` on first use, so the build needs their headers and **the target needs those runtime libraries** (`libegl1` and `libgles2` on Debian/Ubuntu). Without them the EGL detile is unavailable and only the CPU - paths remain, which do not cover every scanout. The crate + paths remain, which do not cover every scanout. Since 0.5.10 the library + never loads them in a setuid, setgid or file-capability process, so there the + EGL detile is unavailable too: grab through the helper from an unprivileged + process instead. The crate compiles its embedded C sources statically, so there is no system `libdrmtap` install required - For unprivileged capture: `drmtap-helper`, which `libdrmtap-sys` always builds diff --git a/contrib/integrations/rustdesk/README.md b/contrib/integrations/rustdesk/README.md index 54eaccd..8c522a7 100644 --- a/contrib/integrations/rustdesk/README.md +++ b/contrib/integrations/rustdesk/README.md @@ -61,4 +61,5 @@ The upstream PR above has the full, current backend. - Linux with DRM/KMS (kernel 4.20+ for the tiled/modifier framebuffer path — Ubuntu 20.04+; linear/VM framebuffers work on older kernels) - `CAP_SYS_ADMIN` or the `drmtap-helper` setcap binary for GPU access (the helper - is built automatically by `libdrmtap-sys`) + is built automatically by `libdrmtap-sys`). A file capability on the consumer + binary itself gets no GPU detile since 0.5.10 diff --git a/helper/drmtap-helper.c b/helper/drmtap-helper.c index 3cbe29e..b74ca51 100644 --- a/helper/drmtap-helper.c +++ b/helper/drmtap-helper.c @@ -993,8 +993,9 @@ int main(int argc, char *argv[]) { /* The device path comes solely from argv[1] -- the value the library selected * and passed. A CAP_SYS_ADMIN process must not take device selection from the - * environment (the library itself ignores DRM_DEVICE when privileged, since - * 0.4.11), so DRM_DEVICE is deliberately NOT honored here. */ + * environment (the library ignores DRM_DEVICE for root since 0.4.11 and for a + * setuid, setgid or file-capability binary since 0.5.10), so DRM_DEVICE is + * deliberately NOT honored here. */ /* The device path is attacker-influenceable (argv / DRM_DEVICE) and we run * with CAP_SYS_ADMIN, so refuse anything that does not canonicalize under diff --git a/include/drmtap.h b/include/drmtap.h index 21005c7..05cedc5 100644 --- a/include/drmtap.h +++ b/include/drmtap.h @@ -31,7 +31,7 @@ extern "C" { * site. */ #define DRMTAP_VERSION_MAJOR 0 #define DRMTAP_VERSION_MINOR 5 -#define DRMTAP_VERSION_PATCH 9 +#define DRMTAP_VERSION_PATCH 10 /** * @brief Get the library version as a packed integer. @@ -79,7 +79,9 @@ typedef struct { * one of the six. The only way to not have the list is to build with * -Dhelper=disabled, which compiles the fork/exec path out of the library * entirely - no fork, exec or socketpair symbol is left in the .so - and - * is what a consumer that already holds CAP_SYS_ADMIN should do. + * is what a consumer that already holds CAP_SYS_ADMIN as root should do + * (since 0.5.10 a setuid, setgid or file-capability binary loads no GL, so + * it gets no EGL detile in-process). * * Note for anyone who read an older header: it listed $DRMTAP_HELPER_PATH * and /drmtap-helper. Neither was ever implemented - the @@ -90,7 +92,8 @@ typedef struct { const char *helper_path; /** Enable debug logging to stderr. - * Can also be enabled with DRMTAP_DEBUG=1 env var. */ + * Can also be enabled with DRMTAP_DEBUG=1 env var, except in a setuid, + * setgid or file-capability binary (read with secure_getenv()). */ int debug; } drmtap_config; diff --git a/meson.build b/meson.build index 498df40..8764f01 100644 --- a/meson.build +++ b/meson.build @@ -1,7 +1,7 @@ # SPDX-License-Identifier: MIT project('libdrmtap', 'c', - version: '0.5.9', + version: '0.5.10', license: 'MIT', default_options: [ 'c_std=c11', diff --git a/src/drm_grab.c b/src/drm_grab.c index b37869f..22989ab 100644 --- a/src/drm_grab.c +++ b/src/drm_grab.c @@ -36,6 +36,7 @@ #include #include #include +#include #include #include #include @@ -105,7 +106,7 @@ static void close_auxiliary_gem_handles(drmtap_ctx *ctx, const drmModeFB2 *fb2) static int drmtap_force_mmap_fail(void) { static int v = -1; if (v < 0) { - const char *e = getenv("DRMTAP_FORCE_MMAP_FAIL"); + const char *e = secure_getenv("DRMTAP_FORCE_MMAP_FAIL"); v = (e && e[0] == '1') ? 1 : 0; } return v; @@ -1696,8 +1697,17 @@ static int gpu_auto_process(drmtap_ctx *ctx, void *data, (unsigned long)modifier); #ifdef HAVE_EGL /* EGL IS compiled in, so the detile was skipped or it failed at - * runtime: no dma-buf fd on this path (helper V2 pixel mode), or no - * usable render node. Point at that, not at the build. */ + * runtime: GL is not loaded in a setuid, setgid or file-capability + * process; otherwise no dma-buf fd on this path (helper V2 pixel mode), + * or no usable render node. Point at that, not at the build. */ + if (getauxval(AT_SECURE)) { + drmtap_set_error(ctx, + "scanout modifier 0x%lx needs a GPU detile, and GL is not loaded " + "in a setuid, setgid or file-capability process: grab through the " + "helper from an unprivileged process instead", + (unsigned long)modifier); + return -ENOTSUP; + } drmtap_set_error(ctx, "scanout modifier 0x%lx needs a GPU detile, and the EGL detile " "this build carries was unavailable or failed: no dma-buf fd on " diff --git a/src/drmtap.c b/src/drmtap.c index 05045a2..7fc1ba8 100644 --- a/src/drmtap.c +++ b/src/drmtap.c @@ -104,11 +104,12 @@ static int open_drm_auto(drmtap_ctx *ctx) { * capture service (root / effective-root, e.g. the unattended --service) * must not let an attacker-influenceable environment variable redirect which * device it opens; it relies on the explicit config device_path or the KMS - * auto-scan below instead. DRM_DEVICE stays honored for unprivileged test / - * dev runs. */ + * auto-scan below instead. secure_getenv() also hides it from a setuid, setgid + * or file-capability binary, whose environment the invoking user controls. + * DRM_DEVICE stays honored for unprivileged test / dev runs. */ const char *env_dev = NULL; if (getuid() != 0 && geteuid() != 0) { - env_dev = getenv("DRM_DEVICE"); + env_dev = secure_getenv("DRM_DEVICE"); } if (env_dev) { drmtap_debug_log(ctx, "trying DRM_DEVICE=%s", env_dev); @@ -292,7 +293,7 @@ drmtap_ctx *drmtap_open(const drmtap_config *config) { } /* Check DRMTAP_DEBUG env var */ - const char *dbg_env = getenv("DRMTAP_DEBUG"); + const char *dbg_env = secure_getenv("DRMTAP_DEBUG"); if (dbg_env && dbg_env[0] == '1') { ctx->debug = 1; } @@ -302,7 +303,7 @@ drmtap_ctx *drmtap_open(const drmtap_config *config) { DRMTAP_VERSION_PATCH, getpid(), getuid()); /* Open DRM device */ - drmtap_debug_log(ctx, "device_path=[%s] DRM_DEVICE=[%s]", ctx->device_path, getenv("DRM_DEVICE") ? getenv("DRM_DEVICE") : "(null)"); + drmtap_debug_log(ctx, "device_path=[%s]", ctx->device_path); if (ctx->device_path[0]) { /* Explicit device path */ ctx->drm_fd = open(ctx->device_path, O_RDWR | O_CLOEXEC); @@ -661,7 +662,7 @@ drmtap_ctx *drmtap_open_render(const char *render_node) { ctx->fast_slots[i].prime_fd = -1; } - const char *dbg_env = getenv("DRMTAP_DEBUG"); + const char *dbg_env = secure_getenv("DRMTAP_DEBUG"); if (dbg_env && dbg_env[0] == '1') { ctx->debug = 1; } diff --git a/src/gpu_egl.c b/src/gpu_egl.c index af61f77..f1d1513 100644 --- a/src/gpu_egl.c +++ b/src/gpu_egl.c @@ -48,6 +48,7 @@ #ifdef HAVE_EGL #include +#include #include #include @@ -140,6 +141,12 @@ static int g_gl_libs_state; /* 0 = not attempted, 1 = loaded, -1 = failed */ * but it must stay textually ABOVE them so the pfn_ declarations resolve. */ static int load_gl_libraries(void) { static pthread_mutex_t lk = PTHREAD_MUTEX_INITIALIZER; + /* -3: not in a setuid, setgid or file-capability process. The GL libraries are + * third-party code that reads its own environment variables, and there the + * invoking user sets them. */ + if (getauxval(AT_SECURE)) { + return -3; + } pthread_mutex_lock(&lk); if (g_gl_libs_state == 0) { /* -1 = the libraries are not here at all (a missing package); -2 = they loaded but a @@ -662,7 +669,7 @@ static int egl_init(drmtap_ctx *ctx, egl_state_t *state) { state->tex_height = 0; /* Escape hatch: DRMTAP_NO_IMAGE_CACHE=1 forces a fresh EGLImage import * per frame (the pre-0.4.9 behaviour) for debugging driver oddities. */ - const char *nocache = getenv("DRMTAP_NO_IMAGE_CACHE"); + const char *nocache = secure_getenv("DRMTAP_NO_IMAGE_CACHE"); state->cache_disabled = (nocache && nocache[0] == '1'); state->initialized = 1; /* Register the thread-exit backstop so a capture thread that dies without @@ -1006,7 +1013,7 @@ int drmtap_gpu_egl_available(drmtap_ctx *ctx) { /* Escape hatch: DRMTAP_NO_EGL=1 forces the CPU deswizzle/convert path (used * to exercise or debug it, and by the convert tests/fuzzer which target the * untrusted-descriptor handling that lives on the CPU side). */ - const char *no_egl = getenv("DRMTAP_NO_EGL"); + const char *no_egl = secure_getenv("DRMTAP_NO_EGL"); if (no_egl && no_egl[0] == '1') { return 0; }