Skip to content

libdrmtap 0.5.7

Choose a tag to compare

@fxd0h fxd0h released this 20 Sep 19:34
· 22 commits to main since this release

Mostly the Rust wrapper. No C code changed: the ABI and the helper wire protocol are
the same as 0.5.6, and the only behavioural difference in the library is the version
macros, so drmtap_version() now packs 0.5.7. One public header comment is corrected,
and it was wrong in a way worth reading if you have ever hardened an install. The version moves
because the four artifacts share one line. The two wrapper reports behind it come from
anonymix007, who is the first person outside the project to build on the split path.

Added: grab_desc() in the safe wrapper (#60)

The wrapper exposed grab() and grab_mapped() and nothing else, and grab() alone
cannot describe a scanout. drmtap_frame_info carries seven fields; drmtap_dmabuf_desc
additionally carries num_planes, offsets[4], pitches[4] and the two HDR fields. So
from safe Rust a multi-plane (Intel CCS) or HDR scanout could not be imported at all:
you held the fd and no way to know where the planes sit inside it.

pub fn grab_desc(&mut self) -> Result<(Frame, DmabufDesc)>

DmabufDesc deliberately carries NO file descriptor, which is a departure from the C
struct and the point of the type. The header is explicit that desc.dma_buf_fd "is an
integer valid only in THIS (exporter) process - it aliases @p frame's fd", and the
Frame owns it. Handing that integer out inside a plain struct would let it outlive
the frame, and the failure is worse than a dangling number: once closed, the kernel can
hand the same integer to an unrelated open(), so a later convert would read some other
file. Metadata only is also what makes the descriptor safe to serialize, which is the
documented flow - the fd travels out of band over SCM_RIGHTS.

Added: the frame's DMA-BUF as a real file descriptor type (#61)

pub fn dma_buf_borrowed_fd(&self) -> Option<BorrowedFd<'_>>
pub fn try_clone_fd(&self) -> io::Result<OwnedFd>

The borrow is tied to &self because that IS the contract: the frame's Drop calls
drmtap_frame_release, which closes the fd, so using it afterwards must not compile.
None is the mapped paths, where the C field holds -1 and a borrowed -1 would be a
lie. try_clone_fd dups rather than handing over the frame's own descriptor, which
would be a double close; the caveat a type cannot express is documented instead - the
dup keeps the buffer mapping alive but not the claim on the scanout, so once the frame
is released the compositor may recycle that buffer.

dma_buf_fd() -> i32 is deprecated rather than changed. Changing its signature in a
patch release would break every caller silently, since Cargo treats 0.5.6 to 0.5.7 as
compatible. The typed accessor is the one to use; the raw form goes in 0.6.

Added: Frame: Debug, printing the format as a fourcc

XR24, not 875713112. Written by hand rather than derived, for that one field, and
it falls back to hex for a code that is not four printable bytes so it can never print
something that looks like a fourcc and is not one.

Added: Error::io_error()

An io::Error accessor. The public C API returns clean errnos (-EINVAL 36 times,
-ENOTSUP 19, -EIO 18, -ENODEV 10, plus -ENOMEM, -EFBIG, -EACCES, -ENOSPC,
-EPROTO) and those convert exactly.

-1 is the one value that does not, and it is ambiguous rather than merely a sentinel.
drmtap_drm_fd() returns a bare -1, as does this wrapper when drmtap_open hands
back null - and -1 is also -EPERM, which the capture path can produce for real,
because it passes -errno through from drmModeGetFB2, drmPrimeHandleToFD and the
virtio transfer ioctls, and an unprivileged caller is exactly who hits those. Nothing
in the integer separates the two cases, so the accessor refuses to guess and answers
None. Little is lost: Error::message carries the C library's own text, which names
the call and the strerror string.

The Error::code field is untouched. It is pub, so changing its type is a breaking
release; this accessor is the additive half of that request.

Added: optional drm-fourcc feature

Frame::drm_format() and DmabufDesc::drm_format() return drm_fourcc::DrmFormat,
behind a cargo feature that is off by default. A third-party type in a public signature
ties this crate's semver to that crate's, permanently, and the wrapper sits under a
capture library other people pin. format() stays a u32 for everyone, and the new
Debug gives the readable fourcc with no dependency at all.

Changed: the Rust floor is declared

rust-version = "1.66", which is what std::os::fd costs. It was already the real
floor from the moment those accessors were written; declaring it means a downstream on
an older toolchain gets that sentence instead of a type error. Verified by building the
crate, its tests and the optional feature on 1.66.0, not only by reading stabilization
dates.

Fixed: the header's helper search list was wrong from the first commit

drmtap_config.helper_path documented where the library looks for drmtap-helper
when the direct export is denied. It advertised $DRMTAP_HELPER_PATH, an environment
variable no code has ever read - git log -S finds exactly one commit touching that
name, the initial skeleton, and it only ever touched the header - and
<exe_dir>/drmtap-helper, which is not searched either. It then omitted four of the
six paths that ARE searched: /usr/lib/rustdesk, /usr/local/bin, /usr/bin and
/usr/lib/drmtap.

That list is not decoration. find_helper() takes the first path that passes
access(X_OK) and execs it as-is, with no check of its owner or its mode, so someone
hardening a deployment from the header would have locked down the two directories it
named and left four open, while believing an environment variable was a control it is
not. The header now names the six, in the order they are tried, says what the match
does, and says plainly that setting helper_path does NOT opt out of the search: a
configured path that is not executable falls through to the list anyway. Only
-Dhelper=disabled removes it, and that is a build option, not a runtime one.

Nothing compared the two lists, which is why it survived eleven releases.
tools/check-helper-paths.sh now does, and CI runs it beside the version check.

Docs

The wrapper README, which is what crates.io serves as the crate's front page, gains the
new entry points, the feature and the toolchain floor. It is the file that shipped stale
in 0.5.1 and again in 0.5.5, so it is audited first now rather than last.