Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 47 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,52 @@ 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.11] - 2026-10-05

### Fixed: the pixels of a mapped frame could change under it at the next grab

A converted frame (a tiled or compressed scanout, or a 10/16-bit format reduced to 8 bits)
had its pixels in the conversion buffer of the context, and a frame from the pixel path of
the privileged helper had them in the receive buffer of the context. `Frame::data()` handed
that memory out with nothing tying it to the next grab, so the next grab overwrote it, and a
larger frame freed and reallocated it under the slice (measured on i915: two live frames
returned the same pointer, and a frame grabbed before a switch to a bigger mode was read from
freed memory, under asan). The wrapper now points the conversion at a buffer the frame owns,
through `drmtap_set_output_buffer()`, and copies pixels the context still owns into one. A
dropped frame leaves its buffer to the next grab, so a consumer that drops one frame per grab
does not allocate. A frame mapped straight from the scanout is still a view of it, not a copy.

Measured on i915 (Meteor Lake, a compressed scanout through the EGL detile, as root):
`grab_mapped` costs what it did in 0.5.10 within 0.1 ms, 4.6-4.8 ms at 1920x1080 and
12.0-12.2 ms at 3840x2160, whether each frame is dropped before the next grab or held past
it. The reused buffer keeps it flat where the allocator would not: with glibc forced to map
every allocation fresh (`MALLOC_MMAP_THRESHOLD_=131072`), a new buffer per grab measured
7.6 ms at 1080p and 29.4 ms at 4K, the reused one 4.7 and 12.1. The process keeps one frame
more resident (8 MB at 1080p, 33 MB at 4K), and one more for each frame it holds.

New in the C API: `drmtap_frame_owns_data()` tells a caller whether `frame->data` is memory
the frame releases, or memory the next grab on the context may overwrite.

### Fixed: `drmtap_deswizzle()` wrote past `dst` on a stride narrower than its rows

It copies 4 bytes per pixel and never checked that a row fits `dst_stride`: on a 2-byte
geometry (60 pixels on a 120-byte stride, `dst` sized `dst_stride * height`) the linear,
X-tiled and Y-tiled paths all wrote past the end of `dst` (heap-buffer-overflow under asan).
A layout it decodes now fails with `-EINVAL` before writing anything; one it cannot decode
still answers `-ENOTSUP`.

### Fixed: the Rust wrapper freed the context while a `Frame` still needed it (#63)

`Frame` and `Cursor` kept a raw copy of the context pointer and no lifetime, so safe code
could drop the `DrmTap` first: `drmtap_close` freed the context, and the release of the frame
then read `drm_fd` from the freed memory to close its GEM handle (a heap-use-after-free under asan,
measured on i915). The context now lives behind a shared handle that `DrmTap`, `Frame` and
`Cursor` hold, and `drmtap_close` runs when the last of them is dropped. No signature changes:
`DrmTap` is still `Send` and not `Sync`, and `Frame` and `Cursor` are still neither. The device
stays open while a frame or cursor from it is alive, so a cached `Cursor` keeps it open, and
dropping the last of them runs the close (on the privilege-helper path that stops the helper,
about 100 ms).

## [0.5.10] - 2026-10-01

### Security: a setuid, setgid or file-capability binary ignores the environment and loads no GL
Expand Down Expand Up @@ -1023,6 +1069,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.11]: https://github.com/fxd0h/libdrmtap/releases/tag/v0.5.11
[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
Expand Down
2 changes: 1 addition & 1 deletion bindings/rust/libdrmtap-sys/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "libdrmtap-sys"
version = "0.5.10"
version = "0.5.11"
links = "drmtap"
edition = "2021"
authors = ["Mariano Abad <weimaraner@gmail.com>"]
Expand Down
15 changes: 15 additions & 0 deletions bindings/rust/libdrmtap-sys/csrc/drm_grab.c
Original file line number Diff line number Diff line change
Expand Up @@ -1683,6 +1683,13 @@ static int gpu_auto_process(drmtap_ctx *ctx, void *data,
frame->width, frame->height,
frame->stride, frame->stride, modifier,
(size_t)frame->stride * frame->height);
if (ret == -EINVAL) {
drmtap_set_error(ctx,
"CPU deswizzle copies 4-byte pixels, and a %u-pixel row does not fit "
"the %u-byte stride of format 0x%08x", frame->width, frame->stride,
frame->format);
return ret;
}
if (ret == -ENOTSUP) {
/* CCS-compressed (or otherwise unsupported) modifier -- the CPU
* deswizzle cannot decode it and no EGL path produced linear pixels
Expand Down Expand Up @@ -1913,6 +1920,14 @@ void drmtap_frame_release(drmtap_ctx *ctx, drmtap_frame_info *frame) {
frame->dma_buf_fd = -1;
}

int drmtap_frame_owns_data(const drmtap_frame_info *frame) {
if (!frame || !frame->data) {
return 0;
}
const frame_priv_t *priv = (const frame_priv_t *)frame->_priv;
return priv && priv->mapped && frame->data == priv->mapped;
}

/* ========================================================================= */
/* Fast persistent-mmap capture (double-buffer cache) */
/* ========================================================================= */
Expand Down
27 changes: 24 additions & 3 deletions bindings/rust/libdrmtap-sys/csrc/drmtap.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ extern "C" {
* site. */
#define DRMTAP_VERSION_MAJOR 0
#define DRMTAP_VERSION_MINOR 5
#define DRMTAP_VERSION_PATCH 10
#define DRMTAP_VERSION_PATCH 11

/**
* @brief Get the library version as a packed integer.
Expand Down Expand Up @@ -270,6 +270,22 @@ int drmtap_grab_mapped_fast(drmtap_ctx *ctx, drmtap_frame_info *frame);
*/
void drmtap_frame_release(drmtap_ctx *ctx, drmtap_frame_info *frame);

/**
* @brief Whether frame->data is memory the frame itself releases.
*
* Returns 1 when frame->data is the frame's own mapping, which stays valid until
* drmtap_frame_release() on that frame whatever the context does in between. Returns 0
* when it points into memory the frame does not own: the context's conversion buffer or
* the privileged helper's receive buffer, which the next grab on the same context
* overwrites and may reallocate, or a buffer set with drmtap_set_output_buffer(). A
* caller that keeps a frame's pixels past the next grab copies them when this is 0.
* Returns 0 for a NULL frame or a frame without data.
*
* @param frame Frame filled by a grab
* @return 1 if the frame owns its pixel data, 0 otherwise
*/
int drmtap_frame_owns_data(const drmtap_frame_info *frame);

/* ========================================================================= */
/* Split capture: privileged export + unprivileged convert */
/* ========================================================================= */
Expand Down Expand Up @@ -749,11 +765,16 @@ int drmtap_set_output_buffer(drmtap_ctx *ctx, void *dst, size_t len);
* a tiled buffer relabelled linear, reported as a valid frame.
*
* @param src Source (tiled) pixel data
* @param dst Destination (linear) buffer (must be allocated by caller)
* @param dst Destination (linear) buffer, allocated by the caller. Row y
* gets width * 4 bytes at y * dst_stride and the rest of the
* row is left as it was, so dst can be a rectangle inside a
* wider image
* @param width Frame width in pixels
* @param height Frame height in pixels
* @param src_stride Source stride (bytes per row in tiled data)
* @param dst_stride Destination stride (bytes per row)
* @param dst_stride Destination stride (bytes per row). Pixels are 4 bytes: a
* layout this decodes fails with -EINVAL, before writing
* anything, on a stride narrower than width * 4
* @param modifier DRM format modifier (e.g., I915_FORMAT_MOD_X_TILED)
* @param src_size Size of the source buffer in bytes; reads are bounded by it
* (a scanout whose height is not a tile multiple would
Expand Down
17 changes: 17 additions & 0 deletions bindings/rust/libdrmtap-sys/csrc/pixel_convert.c
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,11 @@ int drmtap_deswizzle(const void *src, void *dst,
if (!src || !dst || width == 0 || height == 0) {
return -EINVAL;
}
/* Every layout decoded below copies 4 bytes per pixel, so a row has to fit the
* destination stride: on a narrower one (a 2-byte format) the rows would overlap and
* the last one would run past the end of dst. Checked per layout, so one this cannot
* decode still answers -ENOTSUP. */
const int row_fits = (size_t)width * 4 <= dst_stride;

/*
* DRM format modifiers (from drm_fourcc.h):
Expand All @@ -261,6 +266,9 @@ int drmtap_deswizzle(const void *src, void *dst,
/* Linear — just memcpy row by row (bounded by src_size; a full row always
* fits since width*4 <= src_stride, but guard the last rows defensively). */
if (modifier == 0 /* DRM_FORMAT_MOD_LINEAR */) {
if (!row_fits) {
return -EINVAL;
}
for (uint32_t y = 0; y < height; y++) {
size_t row_off = (size_t)y * src_stride;
if (row_off + (size_t)width * 4 > src_size) {
Expand All @@ -283,13 +291,19 @@ int drmtap_deswizzle(const void *src, void *dst,
if (vendor == DRM_FORMAT_MOD_VENDOR_INTEL) {
if (mod_type == 0x01) {
/* I915_FORMAT_MOD_X_TILED */
if (!row_fits) {
return -EINVAL;
}
return deswizzle_intel_x_tiled(src, dst, width, height,
src_stride, dst_stride, src_size);
} else if (mod_type == 0x02 || mod_type == 0x03) {
/* I915_FORMAT_MOD_Y_TILED (0x02)
* I915_FORMAT_MOD_Yf_TILED (0x03)
*
* Pure Y-tiled without compression — CPU deswizzle works. */
if (!row_fits) {
return -EINVAL;
}
return deswizzle_intel_y_tiled(src, dst, width, height,
src_stride, dst_stride, src_size);
}
Expand Down Expand Up @@ -343,6 +357,9 @@ int drmtap_deswizzle(const void *src, void *dst,
* buffer from a simple driver (virtio, embedded), and copying it row by row is
* the historical contract of this entry point. Keep it. */
if (modifier == DRM_FORMAT_MOD_INVALID) {
if (!row_fits) {
return -EINVAL;
}
for (uint32_t y = 0; y < height; y++) {
size_t row_off = (size_t)y * src_stride;
if (row_off + (size_t)width * 4 > src_size) {
Expand Down
3 changes: 3 additions & 0 deletions bindings/rust/libdrmtap-sys/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,9 @@ extern "C" {
pub fn drmtap_grab(ctx: *mut drmtap_ctx, frame: *mut drmtap_frame_info) -> c_int;
pub fn drmtap_grab_mapped(ctx: *mut drmtap_ctx, frame: *mut drmtap_frame_info) -> c_int;
pub fn drmtap_frame_release(ctx: *mut drmtap_ctx, frame: *mut drmtap_frame_info);
/// 1 when `frame.data` is the frame's own mapping, valid until `drmtap_frame_release`;
/// 0 when it is context-owned or caller-owned memory that the next grab may overwrite.
pub fn drmtap_frame_owns_data(frame: *const drmtap_frame_info) -> c_int;

/// Point the conversion paths at a caller-owned buffer instead of a
/// library-owned one, so a consumer that must end up with the pixels in its own
Expand Down
4 changes: 2 additions & 2 deletions bindings/rust/libdrmtap/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "libdrmtap"
version = "0.5.10"
version = "0.5.11"
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.
Expand All @@ -16,7 +16,7 @@ keywords = ["drm", "kms", "screen-capture", "wayland", "remote-desktop"]
categories = ["multimedia::video", "os::linux-apis"]

[dependencies]
libdrmtap-sys = { version = "0.5.10", path = "../libdrmtap-sys" }
libdrmtap-sys = { version = "0.5.11", 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
Expand Down
5 changes: 4 additions & 1 deletion bindings/rust/libdrmtap/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,10 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
for the case where it must outlive the frame; it dups, because the frame closes its own on drop.
The older `dma_buf_fd() -> i32` is deprecated: an integer expresses neither the ownership nor the
lifetime, and `-1` in it means "no DMA-BUF" rather than being a descriptor
- **`grab_mapped()`** — mmap'd pixel data (for software access)
- **`grab_mapped()`** — pixel data for software access. Since 0.5.11 the next grab cannot overwrite
or free a frame's pixels: a conversion writes into a buffer the frame owns, pixels still in the
context's memory are copied into one, and a dropped frame leaves its buffer to the next grab. A
frame mapped straight from the scanout (a linear framebuffer) is a view of that buffer, not a copy
- **`get_cursor()`** — cursor plane position (top-left of the image, in the CRTC's physical pixels) + ARGB image. On bare-metal drivers the hotspot reads `0`, and the `Cursor::hot_x` documentation gives the two ways to recover one. **`Cursor::hotspot_from_driver()`** (since 0.5.6) says whether that `0` is the driver's own answer: `Some(true)` means both `HOTSPOT_X` and `HOTSPOT_Y` were read, so `hot_x`/`hot_y` are the driver's coordinates even at `(0, 0)`; `Some(false)` means **at least one** was absent, so they carry no information and a hotspot has to be estimated; `None` means nothing recorded an answer for that sample, which is not the same as `Some(false)`
- **`list_displays()`** — enumerate connected monitors
- **`displays_changed()`** — hotplug detection
Expand Down
Loading
Loading