From 1903223d12e6b528659ee5938b0299df95d4837e Mon Sep 17 00:00:00 2001 From: Aleksa Sarai Date: Wed, 4 Feb 2026 01:13:40 +0100 Subject: [PATCH 1/5] makefile: add EXTRA_*_FLAGS options to make packaging easier Some builders want to specify extra cargo or rustc flags when building, so making this be built-in is far more preferable. Signed-off-by: Aleksa Sarai --- CHANGELOG.md | 5 +++++ Makefile | 6 ++++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5b806f1e..41b38f8a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,11 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ## [Unreleased] ## +### Added ### +- New `EXTRA_RUSTC_FLAGS` and `EXTRA_CARGO_FLAGS` variables have been added to + our `Makefile`, making it easier for packaging tools to adjust builds while + still using `make release`. + ## [0.2.3] - 2026-01-29 ## > この閃きを俺は信じる！ diff --git a/Makefile b/Makefile index da5c053c..525cfd93 100644 --- a/Makefile +++ b/Makefile @@ -48,8 +48,10 @@ CARGO_CHECK := $(call cargo_hack,$(CARGO),check) CARGO_CLIPPY := $(call cargo_hack,$(CARGO),clippy) CARGO_LLVM_COV := $(call cargo_hack,$(CARGO_NIGHTLY),llvm-cov) -RUSTC_FLAGS := -C panic=abort -CARGO_FLAGS ?= --features=capi +EXTRA_RUSTC_FLAGS ?= +RUSTC_FLAGS := -C panic=abort $(EXTRA_RUSTC_FLAGS) +EXTRA_CARGO_FLAGS ?= +CARGO_FLAGS := --features=capi $(EXTRA_CARGO_FLAGS) SRC_FILES = $(wildcard Cargo.*) $(shell find . -name '*.rs') From 021390a42d8b6be7d4e975dd181508920950eb02 Mon Sep 17 00:00:00 2001 From: Aleksa Sarai Date: Wed, 4 Feb 2026 01:15:32 +0100 Subject: [PATCH 2/5] install: add --rust-{target,buildmode} args When doing cross-compliation (for runc), the binaries are put into a different directory and we need to support using those alternative build directories as sources of installation. Signed-off-by: Aleksa Sarai --- CHANGELOG.md | 3 +++ install.sh | 30 +++++++++++++++++++++--------- 2 files changed, 24 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 41b38f8a..ddfbfc6d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - New `EXTRA_RUSTC_FLAGS` and `EXTRA_CARGO_FLAGS` variables have been added to our `Makefile`, making it easier for packaging tools to adjust builds while still using `make release`. +- `install.sh` now accepts `--rust-target` and `--rust-buildmode` as parameters + to make cross-compilation workflows easier to write (in particular, this is + needed for runc's release scripts). ## [0.2.3] - 2026-01-29 ## diff --git a/install.sh b/install.sh index 168aedc7..37f96034 100755 --- a/install.sh +++ b/install.sh @@ -54,6 +54,7 @@ SOVERSION="$(get_so_version)" SONAME="lib$(get_crate_info name).so.$FULLVERSION" +TARGET= # Try to emulate autoconf's basic flags. DEFAULT_PREFIX=/usr/local usage() { @@ -83,6 +84,13 @@ usage() { filesystem. This is usually used for distribution packaging. You can also pass environment variables as command-line arguments. + In addition, in order to support cross-compilation and custom build modes, + you can specify which rust target architecture and build mode to copy the + files from (rather than the top-level native target/release directory). + + --rust-target=[${TARGET:=}] + --rust-buildmode=[release] + Example: In an openSUSE rpm spec, this script could be used like this: @@ -103,7 +111,7 @@ usage() { [ "$#" -gt 0 ] && exit_code=1 exit "$exit_code" } -GETOPT="$(getopt -o h --long help,prefix:,exec-prefix:,includedir:,libdir:,pkgconfigdir: -- "$@")" +GETOPT="$(getopt -o h --long help,prefix:,exec-prefix:,includedir:,libdir:,pkgconfigdir:,rust-target:,rust-buildmode: -- "$@")" eval set -- "$GETOPT" DESTDIR="${DESTDIR:-}" @@ -112,19 +120,23 @@ exec_prefix= includedir= libdir= pkgconfigdir= +rust_target="$TARGET" +rust_buildmode=release while true; do case "$1" in - --prefix) prefix="$2"; shift 2 ;; - --exec-prefix) exec_prefix="$2"; shift 2 ;; - --includedir) includedir="$2"; shift 2 ;; - --libdir) libdir="$2"; shift 2 ;; - --pkgconfigdir) pkgconfigdir="$2"; shift 2 ;; + --prefix) prefix="$2"; shift 2 ;; + --exec-prefix) exec_prefix="$2"; shift 2 ;; + --includedir) includedir="$2"; shift 2 ;; + --libdir) libdir="$2"; shift 2 ;; + --pkgconfigdir) pkgconfigdir="$2"; shift 2 ;; + --rust-target) rust_target="$2"; shift 2 ;; + --rust-buildmode) rust_buildmode="$2"; shift 2 ;; --) shift; break ;; -h | --help) usage ;; *) usage "unknown argument $1" ;; esac done - +builddir="target/$rust_target/$rust_buildmode" for extra_arg in "$@"; do if [[ "$extra_arg" = *=* ]]; then @@ -210,8 +222,8 @@ set -x install -Dt "$DESTDIR/$pkgconfigdir/" -m 0644 pathrs.pc install -Dt "$DESTDIR/$includedir/" -m 0644 include/pathrs.h # Static library. -install -Dt "$DESTDIR/$libdir" -m 0644 target/release/libpathrs.a +install -Dt "$DESTDIR/$libdir" -m 0644 "$builddir/libpathrs.a" # Shared library. -install -DT -m 0755 target/release/libpathrs.so "$DESTDIR/$libdir/$SONAME" +install -DT -m 0755 "$builddir/libpathrs.so" "$DESTDIR/$libdir/$SONAME" ln -sf "$SONAME" "$DESTDIR/$libdir/libpathrs.so.$SOVERSION" ln -sf "$SONAME" "$DESTDIR/$libdir/libpathrs.so" From cda2fc28a12f9e89e79beca29a48a004daa7b079 Mon Sep 17 00:00:00 2001 From: Aleksa Sarai Date: Wed, 4 Feb 2026 18:11:12 +0100 Subject: [PATCH 3/5] capi: do not output .symver for non-cdylib builds Outputting .symver entries for staticlib builds leads to "multiple definition" errors when linking statically, so we need to only output .symver (and our version script) when building a cdylib. Unfortunately, Rust provides no built-in mechanism to do this nicely. There is no way to get the crate type using #[cfg(...)] and even an explicit --cfg on the command-line does not show up in build.rs. The only real "Rust-y" solution would be to create a separate feature for cdylib but that is really ugly. The solution I went with was passing an environment variable during the build (LIBPATHRS_CAPI_BUILDMODE) which acts as a proxy for --crate-type (in the hopes that this might be supported properly in the future). build.rs then configures the #[cfg(cdylib)] used to control the output of .symver and only outputs a version script. We even get to use cargo:rerun-if-env-changed to make sure that the build script is actually rebuilt for every crate type change. An unset LIBPATHRS_CAPI_BUILDMODE (such as during tests) is treated like staticlib. Signed-off-by: Aleksa Sarai --- CHANGELOG.md | 15 ++++++ Cargo.toml | 1 - Makefile | 54 ++++++++++++-------- build.rs | 109 ++++++++++++++++++++++++---------------- hack/with-crate-type.sh | 24 ++++++++- src/capi/utils.rs | 2 + 6 files changed, 137 insertions(+), 68 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ddfbfc6d..12d6809f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,21 @@ and this project adheres to [Semantic Versioning](http://semver.org/). to make cross-compilation workflows easier to write (in particular, this is needed for runc's release scripts). +### Fixed ### +- Previously, `staticlib` builds of libpathrs (i.e., `libpathrs.a`) + inadvertently included symbol versioned symbols (`@@LIBPATHRS_X.Y`), which + would cause linker errors when trying to compile programs statically against + libpathrs. + + This has been resolved, but downstream users who build runc without using + `make release` will need to take care to ensure they correctly set the + `LIBPATHRS_CAPI_BUILDMODE` environment variable when building and build + `libpathrs.a` and `libpathrs.so` in **separate** `cargo build` (or `cargo + rustc`) invocations. This is mostly necessary due to [the lack of support for + `#[cfg(crate_type)]`][rust-issue20267]. + +[rust-issue20267]: https://github.com/rust-lang/rust/issues/20267 + ## [0.2.3] - 2026-01-29 ## > この閃きを俺は信じる！ diff --git a/Cargo.toml b/Cargo.toml index 96b20262..46689f75 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -48,7 +48,6 @@ rust-version = "1.63" maintenance = { status = "experimental" } [lib] -# MSRV(1.64): Replace with --crate-type={cdy,static}lib in Makefile. crate-type = ["rlib"] [features] diff --git a/Makefile b/Makefile index 525cfd93..cd0760cf 100644 --- a/Makefile +++ b/Makefile @@ -57,29 +57,41 @@ SRC_FILES = $(wildcard Cargo.*) $(shell find . -name '*.rs') .DEFAULT: debug .PHONY: debug -debug: target/debug - -target/debug: $(SRC_FILES) - # MSRV(1.64): Use cargo rustc --crate-type={cdy,static}lib. - RUSTFLAGS="$(RUSTC_FLAGS)" ./hack/with-crate-type.sh $(CARGO) build $(CARGO_FLAGS) - # For some reason, --crate-type needs separate invocations. We can't use - # #![crate_type] unfortunately, as using it with #![cfg_attr] has been - # deprecated. - #$(CARGO) rustc $(CARGO_FLAGS) --crate-type=cdylib $(RUSTC_FLAGS) - #$(CARGO) rustc $(CARGO_FLAGS) --crate-type=staticlib $(RUSTC_FLAGS) +debug: target/debug/libpathrs.so target/debug/libpathrs.a + +target/debug/libpathrs.so: LIBPATHRS_CAPI_BUILDMODE := cdylib +target/debug/libpathrs.so: $(SRC_FILES) + LIBPATHRS_CAPI_BUILDMODE=$(LIBPATHRS_CAPI_BUILDMODE) \ + hack/with-crate-type.sh --crate-type=$(LIBPATHRS_CAPI_BUILDMODE) \ + env RUSTFLAGS="$(RUSTC_FLAGS)" $(CARGO) build $(CARGO_FLAGS) + # MSRV(1.64): Switch to cargo rustc --crate-type=... + #LIBPATHRS_CAPI_BUILDMODE=$(LIBPATHRS_CAPI_BUILDMODE) $(CARGO) rustc $(CARGO_FLAGS) --crate-type=$(LIBPATHRS_CAPI_BUILDMODE) -- $(RUSTC_FLAGS) +target/debug/libpathrs.a: LIBPATHRS_CAPI_BUILDMODE := staticlib +target/debug/libpathrs.a: $(SRC_FILES) + LIBPATHRS_CAPI_BUILDMODE=$(LIBPATHRS_CAPI_BUILDMODE) \ + hack/with-crate-type.sh --crate-type=$(LIBPATHRS_CAPI_BUILDMODE) \ + env RUSTFLAGS="$(RUSTC_FLAGS)" $(CARGO) build $(CARGO_FLAGS) + # MSRV(1.64): Switch to cargo rustc --crate-type=... + #LIBPATHRS_CAPI_BUILDMODE=$(LIBPATHRS_CAPI_BUILDMODE) $(CARGO) rustc $(CARGO_FLAGS) --crate-type=$(LIBPATHRS_CAPI_BUILDMODE) -- $(RUSTC_FLAGS) .PHONY: release -release: target/release - -target/release: CARGO_FLAGS += --release -target/release: $(SRC_FILES) - # MSRV(1.64): Use cargo rustc --crate-type={cdy,static}lib. - RUSTFLAGS="$(RUSTC_FLAGS)" ./hack/with-crate-type.sh $(CARGO) build $(CARGO_FLAGS) - # For some reason, --crate-type needs separate invocations. We can't use - # #![crate_type] unfortunately, as using it with #![cfg_attr] has been - # deprecated. - #$(CARGO) rustc $(CARGO_FLAGS) --crate-type=cdylib $(RUSTC_FLAGS) - #$(CARGO) rustc $(CARGO_FLAGS) --crate-type=staticlib $(RUSTC_FLAGS) +release: CARGO_FLAGS += --release +release: target/release/libpathrs.so target/release/libpathrs.a + +target/release/libpathrs.so: LIBPATHRS_CAPI_BUILDMODE := cdylib +target/release/libpathrs.so: $(SRC_FILES) + LIBPATHRS_CAPI_BUILDMODE=$(LIBPATHRS_CAPI_BUILDMODE) \ + hack/with-crate-type.sh --crate-type=$(LIBPATHRS_CAPI_BUILDMODE) \ + env RUSTFLAGS="$(RUSTC_FLAGS)" $(CARGO) build $(CARGO_FLAGS) + # MSRV(1.64): Switch to cargo rustc --crate-type=... + #LIBPATHRS_CAPI_BUILDMODE=$(LIBPATHRS_CAPI_BUILDMODE) $(CARGO) rustc $(CARGO_FLAGS) --crate-type=$(LIBPATHRS_CAPI_BUILDMODE) -- $(RUSTC_FLAGS) +target/release/libpathrs.a: LIBPATHRS_CAPI_BUILDMODE := staticlib +target/release/libpathrs.a: $(SRC_FILES) + LIBPATHRS_CAPI_BUILDMODE=$(LIBPATHRS_CAPI_BUILDMODE) \ + hack/with-crate-type.sh --crate-type=$(LIBPATHRS_CAPI_BUILDMODE) \ + env RUSTFLAGS="$(RUSTC_FLAGS)" $(CARGO) build $(CARGO_FLAGS) + # MSRV(1.64): Switch to cargo rustc --crate-type=... + #LIBPATHRS_CAPI_BUILDMODE=$(LIBPATHRS_CAPI_BUILDMODE) $(CARGO) rustc $(CARGO_FLAGS) --crate-type=$(LIBPATHRS_CAPI_BUILDMODE) -- $(RUSTC_FLAGS) .PHONY: smoke-test smoke-test: diff --git a/build.rs b/build.rs index 51c726da..2457ecaa 100644 --- a/build.rs +++ b/build.rs @@ -35,55 +35,76 @@ use std::{env, io::Write}; use tempfile::NamedTempFile; fn main() { - // Add DT_SONAME and other ELF metadata to our cdylibs. We can't check the - // crate-type here directly, but we can at least avoid needless warnings for - // "cargo build" by only emitting this when the capi feature is enabled. + println!("cargo:rerun-if-env-changed=CARGO_FEATURE_CAPI"); + println!("cargo:rerun-if-env-changed=LIBPATHRS_CAPI_BUILDMODE"); if cfg!(feature = "capi") { - let name = "pathrs"; - // TODO: Since we use symbol versioning, it seems quite unlikely that we - // would ever bump the major version in the SONAME, so we should - // probably hard-code this or define it elsewhere. - let major = env::var("CARGO_PKG_VERSION_MAJOR").unwrap(); - println!("cargo:rustc-cdylib-link-arg=-Wl,-soname,lib{name}.so.{major}"); + // Add DT_SONAME and other ELF metadata to our cdylibs. We can't check + // the crate-type here directly, so we have our Makefile define a + // special environment variable instead (for whatever reason, even + // --cfg=... doesn't seem to propagate to build.rs. This means that each + // crate-type needs to be built separately (with --cargo-type). + // + // The alternative would be to make cdylib/symvers a feature, which + // seems even more prone to breakage -- at least your build will + // explicitly fail if you don't specifying LIBPATHRS_CAPI_BUILDMODE. + let is_cdylib = match env::var("LIBPATHRS_CAPI_BUILDMODE") + .unwrap_or_else(|_| "".to_string()) + .as_str() + { + "cdylib" => { + println!("cargo:rustc-cfg=cdylib"); + true + } + "staticlib" | "" => false, + v => panic!("unknown LIBPATHRS_CAPI_BUILDMODE={v} value"), + }; + if is_cdylib { + let name = "pathrs"; + // TODO: Since we use symbol versioning, it seems quite unlikely that we + // would ever bump the major version in the SONAME, so we should + // probably hard-code this or define it elsewhere. + let major = env::var("CARGO_PKG_VERSION_MAJOR").unwrap(); + println!("cargo:rustc-cdylib-link-arg=-Wl,-soname,lib{name}.so.{major}"); - let (mut version_script_file, version_script_path) = - NamedTempFile::with_prefix("libpathrs-version-script.") - .expect("mktemp") - .keep() - .expect("persist mktemp"); - let version_script_path = version_script_path - .to_str() - .expect("mktemp should be utf-8 safe string"); - writeln!( - version_script_file, - // All of the symbol versions are done with in-line .symver entries. - // This version script is only needed to define the version nodes - // (and their dependencies). - // FIXME: "local" doesn't appear to actually hide symbols in the - // output .so. For more information about getting all of this to - // work nicely, see . - r#" + let (mut version_script_file, version_script_path) = + NamedTempFile::with_prefix("libpathrs-version-script.") + .expect("mktemp") + .keep() + .expect("persist mktemp"); + let version_script_path = version_script_path + .to_str() + .expect("mktemp should be utf-8 safe string"); + writeln!( + version_script_file, + // All of the symbol versions are done with in-line .symver entries. + // This version script is only needed to define the version nodes + // (and their dependencies). + // FIXME: "local" doesn't appear to actually hide symbols in the + // output .so. For more information about getting all of this to + // work nicely, see . + r#" LIBPATHRS_0.1 {{ }}; LIBPATHRS_0.2 {{ local: *; }} LIBPATHRS_0.1; "# - ) - .expect("write version script"); - println!("cargo:rustc-cdylib-link-arg=-Wl,--version-script={version_script_path}"); + ) + .expect("write version script"); + println!("cargo:rustc-cdylib-link-arg=-Wl,--version-script={version_script_path}"); - // The above version script (and our .symver setup) conflicts with the - // version script and options used by Rust when linking with GNU ld. - // Thankfully, lld Just Works(TM) out of the box so we can use it. - // - // Rust 1.90 switched to lld by default for x86, but for older versions - // and other architectures it is necessary to specify the linker as lld - // (there was also a rustflag for this but it was unstable until Rust - // 1.90). - // - // Unfortunately, while there are some clever tricks you could use for - // GNU ld (such as writing an ld wrapper and executing it with "cc -B"), - // doing so produces useless symbol versions so it's better to just - // require lld. Debian bullseye and later all have lld, so this is a - // non-issue for packagers. - println!("cargo:rustc-cdylib-link-arg=-fuse-ld=lld"); + // The above version script (and our .symver setup) conflicts with the + // version script and options used by Rust when linking with GNU ld. + // Thankfully, lld Just Works(TM) out of the box so we can use it. + // + // Rust 1.90 switched to lld by default for x86, but for older versions + // and other architectures it is necessary to specify the linker as lld + // (there was also a rustflag for this but it was unstable until Rust + // 1.90). + // + // Unfortunately, while there are some clever tricks you could use for + // GNU ld (such as writing an ld wrapper and executing it with "cc -B"), + // doing so produces useless symbol versions so it's better to just + // require lld. Debian bullseye and later all have lld, so this is a + // non-issue for packagers. + println!("cargo:rustc-cdylib-link-arg=-fuse-ld=lld"); + } } } diff --git a/hack/with-crate-type.sh b/hack/with-crate-type.sh index 75c318e5..6a768e27 100755 --- a/hack/with-crate-type.sh +++ b/hack/with-crate-type.sh @@ -35,12 +35,32 @@ set -Eeuo pipefail +bail() { + echo "$*" >&2 + exit 1 +} + SRC_ROOT="$(readlink -f "$(dirname "${BASH_SOURCE[0]}")/..")" -backup="$(mktemp "$SRC_ROOT/Cargo.toml.XXXXXX")" + +# Only parse the first argument if it is --crate-type=foo. The rest of the +# arguments get passed as-is. +crate_types=("rlib" "staticlib" "cdylib") +case "${1:-}" in + --crate-type=*) + arg="${1#*=}" + IFS=', ' read -r -a crate_types <<<"$arg" + shift 1 + ;; +esac + +[ "$#" -gt 0 ] || bail "usage: with-crate-type.sh [--crate-type=...] [...]" set -x -sed -i".${backup##*.}" '/^crate-type/ s/=.*/= ["rlib", "cdylib", "staticlib"]/' "$SRC_ROOT/Cargo.toml" +backup="$(mktemp "$SRC_ROOT/Cargo.toml.XXXXXX")" +sed -i".${backup##*.}" \ + "/^crate-type/ s/=.*/= [$(printf '"%s",' "${crate_types[@]}")]/" \ + "$SRC_ROOT/Cargo.toml" # shellcheck disable=SC2064 # We want to expand the variables immediately. trap "mv '$backup' '$SRC_ROOT/Cargo.toml'" EXIT diff --git a/src/capi/utils.rs b/src/capi/utils.rs index ab6b9a68..187c692a 100644 --- a/src/capi/utils.rs +++ b/src/capi/utils.rs @@ -73,6 +73,8 @@ use libc::{c_char, c_int, size_t}; macro_rules! symver { () => {}; (@with-meta $(#[$meta:meta])* $($block:tt)+) => { + // Only generate .symver entries for cdylib. + #[cfg(cdylib)] // Some architectures still have unstable ASM, which stops us from // injecting the ".symver" section. You can see the list in // LoweringContext::lower_inline_asm (compiler/rustc_asm_lowering). From e4223c123b17c9e6d6ab9969535a52d0b75628d1 Mon Sep 17 00:00:00 2001 From: Aleksa Sarai Date: Fri, 6 Feb 2026 09:49:26 +1100 Subject: [PATCH 4/5] go-pathrs: fix build errors on i386 There were two separate issues when building on i386: * We used C.ulong rather than C.size_t internally when calling into readlink-related libpathrs APIs, which are different widths on i386. The solution is to just use C.size_t, which we should've used in the first place. * There was a comparison between int and 1<<31, which would overflow on i386. The solution is to cast to uint64, though this happens to cause a spurious G115 lint error from gosec which we need to mask. Signed-off-by: Aleksa Sarai --- CHANGELOG.md | 1 + go-pathrs/internal/libpathrs/libpathrs_linux.go | 4 ++-- go-pathrs/procfs/procfs_linux.go | 5 +++-- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 12d6809f..11475de4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). `libpathrs.a` and `libpathrs.so` in **separate** `cargo build` (or `cargo rustc`) invocations. This is mostly necessary due to [the lack of support for `#[cfg(crate_type)]`][rust-issue20267]. +- `go-pathrs` now correctly builds on 32-bit architectures. [rust-issue20267]: https://github.com/rust-lang/rust/issues/20267 diff --git a/go-pathrs/internal/libpathrs/libpathrs_linux.go b/go-pathrs/internal/libpathrs/libpathrs_linux.go index e8ca2a13..d54497a5 100644 --- a/go-pathrs/internal/libpathrs/libpathrs_linux.go +++ b/go-pathrs/internal/libpathrs/libpathrs_linux.go @@ -100,7 +100,7 @@ func InRootReadlink(rootFd uintptr, path string) (string, error) { size := 128 for { linkBuf := make([]byte, size) - n := C.pathrs_inroot_readlink(C.int(rootFd), cPath, C.cast_ptr(unsafe.Pointer(&linkBuf[0])), C.ulong(len(linkBuf))) + n := C.pathrs_inroot_readlink(C.int(rootFd), cPath, C.cast_ptr(unsafe.Pointer(&linkBuf[0])), C.size_t(len(linkBuf))) switch { case int(n) < C.__PATHRS_MAX_ERR_VALUE: return "", fetchError(n) @@ -301,7 +301,7 @@ func ProcReadlinkat(procRootFd int, base ProcBase, path string) (string, error) linkBuf := make([]byte, size) n := C.pathrs_proc_readlinkat( C.int(procRootFd), cBase, cPath, - C.cast_ptr(unsafe.Pointer(&linkBuf[0])), C.ulong(len(linkBuf))) + C.cast_ptr(unsafe.Pointer(&linkBuf[0])), C.size_t(len(linkBuf))) switch { case int(n) < C.__PATHRS_MAX_ERR_VALUE: return "", fetchError(n) diff --git a/go-pathrs/procfs/procfs_linux.go b/go-pathrs/procfs/procfs_linux.go index 14c861da..915e9ccd 100644 --- a/go-pathrs/procfs/procfs_linux.go +++ b/go-pathrs/procfs/procfs_linux.go @@ -56,10 +56,11 @@ var ( // *before* you call wait(2)or any equivalent method that could reap // zombies). func ProcPid(pid int) ProcBase { - if pid < 0 || pid >= 1<<31 { + if pid < 0 || uint64(pid) >= 1<<31 { panic("invalid ProcBasePid value") // TODO: should this be an error? } - return ProcBase{inner: libpathrs.ProcPid(uint32(pid))} + pid32 := uint32(pid) //nolint:gosec // G115 false positive + return ProcBase{inner: libpathrs.ProcPid(pid32)} } // ThreadCloser is a callback that needs to be called when you are done From 538985cfd910dbb132eb5c8c5630fdef6d8fc8d2 Mon Sep 17 00:00:00 2001 From: Aleksa Sarai Date: Mon, 9 Feb 2026 14:27:03 +1100 Subject: [PATCH 5/5] procfs: always use ProcfsHandleRef::open_base as a temporary If we assign ProcfsHandleRef::open_base to a variable then Rust will only drop it once we exit the scope, which is fine in general but causes issues with runc's CI. runc's CI checks for fd leaks in "runc create" (which blocks on a re-open of a FIFO) and so having an additional temporary file descriptor open at that stage leads to regressions in runc's CI. We could add this file descriptor to the allow-list but it's much nicer to just always use it as a temporary (sadly there isn't a way to mark the return value in a way that would cause a clippy lint, as far as I can see). Signed-off-by: Aleksa Sarai --- CHANGELOG.md | 7 +++++++ src/procfs.rs | 6 ++---- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 11475de4..2fa419c0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,13 @@ and this project adheres to [Semantic Versioning](http://semver.org/). rustc`) invocations. This is mostly necessary due to [the lack of support for `#[cfg(crate_type)]`][rust-issue20267]. - `go-pathrs` now correctly builds on 32-bit architectures. +- When doing `procfs` operations, previously libpathrs would internally keep a + handle to `ProcfsBase` open during the entire operation (due to `Drop` + semantics in Rust) rather than closing the file descriptor as quickly as + possible. The file descriptor would be closed soon afterwards (and thus was + not a leak) but tools that search for file descriptor leaks (such as runc's + test suite) could incorrectly classify this as a leak. We now close this + `ProcfsBase` handle far more aggressively. [rust-issue20267]: https://github.com/rust-lang/rust/issues/20267 diff --git a/src/procfs.rs b/src/procfs.rs index 38e81c89..b646cd33 100644 --- a/src/procfs.rs +++ b/src/procfs.rs @@ -513,8 +513,7 @@ impl<'fd> ProcfsHandleRef<'fd> { // present because of subset=pid and retry (for magic-links we need to // operate on the target path more than once, which makes the retry // logic easier to do upfront here). - let basedir = self.open_base(base)?; - match self.openat_raw(basedir.as_fd(), subpath, oflags) { + match self.openat_raw(self.open_base(base)?.as_fd(), subpath, oflags) { Ok(file) => return Ok(file.into()), Err(err) => { if self.is_subset && err.kind() == ErrorKind::OsError(Some(libc::ENOENT)) { @@ -629,10 +628,9 @@ impl<'fd> ProcfsHandleRef<'fd> { oflags.insert(OpenFlags::O_NOFOLLOW); // Do a basic lookup. - let basedir = self.open_base(base)?; let subpath = subpath.as_ref(); let fd = self - .openat_raw(basedir.as_fd(), subpath, oflags) + .openat_raw(self.open_base(base)?.as_fd(), subpath, oflags) .or_else(|err| { if self.is_subset && err.kind() == ErrorKind::OsError(Some(libc::ENOENT)) { // If the lookup failed due to ENOENT, and the current