procfs: add safe procfs API and harden proc operations

.github/workflows/ci.yml

@@ -58,11 +58,19 @@ jobs:
       - uses: taiki-e/install-action@cargo-llvm-cov
       - uses: taiki-e/install-action@nextest
 
-      - name: cargo nextest
+      - name: unit tests
         run: cargo llvm-cov --no-report nextest
-      - name: cargo test --doc
+      - name: doctests
         run: cargo llvm-cov --no-report --doc
 
+      # Run the unit tests as root.
+      # NOTE: Ideally this would be configured in .cargo/config.toml so it
+      #       would also work locally, but unfortunately it seems cargo doesn't
+      #       support cfg(feature=...) for target runner configs.
+      #       See <https://github.com/rust-lang/cargo/issues/14306>.
+      - name: unit tests (root)
+        run: CARGO_TARGET_X86_64_UNKNOWN_LINUX_GNU_RUNNER='sudo -E' cargo llvm-cov --no-report --features _test_as_root nextest
+
       - name: calculate coverage
         run: cargo llvm-cov report
       - name: generate coverage html

Cargo.toml

@@ -35,6 +35,10 @@ travis-ci = { repository = "openSUSE/libpathrs" }
 [lib]
 crate-type = ["rlib", "cdylib", "staticlib"]
 
+[features]
+# Only used for tests.
+_test_as_root = []
+
 [profile.release]
 # Enable link-time optimisations.
 lto = true

cbindgen.toml

@@ -70,6 +70,8 @@ aligned_n = "__CBINDGEN_ALIGNED"
 
 [export]
 exclude = [
+	# CReturn is a rust-only typedef.
+	"CReturn",
 	# Don't export the RESOLVE_* definitions.
 	"RESOLVE_NO_XDEV",
 	"RESOLVE_NO_MAGICLINKS",
@@ -80,6 +82,7 @@ exclude = [
 
 # Clean up the naming of structs.
 [export.rename]
+"CProcfsBase" = "pathrs_proc_base_t"
 
 # Error API.
 "CError" = "pathrs_error_t"

contrib/bindings/python/pathrs.py

@@ -24,7 +24,15 @@
 
 from _pathrs import ffi, lib as libpathrs_so
 
-__all__ = ["Root", "Handle", "Error"]
+__all__ = [
+	# core api
+	"Root", "Handle",
+	# procfs api
+	"PROC_SELF", "PROC_THREAD_SELF",
+	"proc_open", "proc_open_raw", "proc_readlink",
+	# error api
+	"Error",
+]
 
 def _cstr(pystr):
 	return ffi.new("char[]", pystr.encode("utf8"))
@@ -35,6 +43,9 @@ def _pystr(cstr):
 def _pyptr(cptr):
 	return int(ffi.cast("uintptr_t", cptr))
 
+def _cbuffer(size):
+	return ffi.new("char[%d]" % (size,))
+
 
 class Error(Exception):
 	def __init__(self, message, *_, errno=None):
@@ -120,6 +131,16 @@ def fileno(self):
 	def leak(self):
 		self._fd = None
 
+	def fdopen(self, mode="r"):
+		try:
+			fd = self.fileno()
+			file = os.fdopen(fd, mode)
+			self.leak()
+			return file
+		except:
+			fd.close()
+			raise
+
 	@classmethod
 	def from_raw_fd(cls, fd):
 		return cls(fd)
@@ -190,6 +211,40 @@ def convert_mode(mode):
 	# We don't care about "b" or "t" since that's just a Python thing.
 	return flags
 
+
+PROC_SELF = libpathrs_so.PATHRS_PROC_SELF
+PROC_THREAD_SELF = libpathrs_so.PATHRS_PROC_THREAD_SELF
+
+def proc_open(base, path, mode="r", extra_flags=0):
+	flags = convert_mode(mode) | extra_flags
+	return proc_open_raw(base, path, flags).fdopen(mode)
+
+def proc_open_raw(base, path, flags):
+	path = _cstr(path)
+	fd = libpathrs_so.pathrs_proc_open(base, path, flags)
+	if fd < 0:
+		raise Error._fetch(fd) or INTERNAL_ERROR
+	return WrappedFd(fd)
+
+def proc_readlink(base, path):
+	path = _cstr(path)
+	linkbuf_size = 128
+	while True:
+		linkbuf = _cbuffer(linkbuf_size)
+		n = libpathrs_so.pathrs_proc_readlink(base, path, linkbuf, linkbuf_size)
+		if n < 0:
+			raise Error._fetch(n) or INTERNAL_ERROR
+		elif n <= linkbuf_size:
+			return ffi.buffer(linkbuf, linkbuf_size)[:n].decode("latin1")
+		else:
+			# The contents were truncated. Unlike readlinkat, pathrs returns
+			# the size of the link when it checked. So use the returned size
+			# as a basis for the reallocated size (but in order to avoid a DoS
+			# where a magic-link is growing by a single byte each iteration,
+			# make sure we are a fair bit larger).
+			linkbuf_size += n
+
+
 class Handle(WrappedFd):
 	def __init__(self, file):
 		# XXX: Is this necessary?
@@ -201,15 +256,7 @@ def from_file(cls, file):
 
 	def reopen(self, mode="r", extra_flags=0):
 		flags = convert_mode(mode) | extra_flags
-		rawfile = self.reopen_raw(flags)
-		try:
-			fd = rawfile.fileno()
-			file = os.fdopen(fd, mode)
-			rawfile.leak()
-			return file
-		except:
-			rawfile.close()
-			raise
+		return self.reopen_raw(flags).fdopen(mode)
 
 	def reopen_raw(self, flags):
 		fd = libpathrs_so.pathrs_reopen(self.fileno(), flags)

go-pathrs/libpathrs_linux.go

@@ -27,6 +27,12 @@ import (
 // #include <pathrs.h>
 import "C"
 
+/*
+// This is a workaround for unsafe.Pointer() not working for non-void pointers.
+char *cast_ptr(void *ptr) { return ptr; }
+*/
+import "C"
+
 func fetchError(errId C.int) error {
 	if errId >= 0 {
 		return nil
@@ -121,3 +127,46 @@ func pathrsHardlink(rootFd uintptr, path, target string) error {
 	err := C.pathrs_hardlink(C.int(rootFd), cPath, cTarget)
 	return fetchError(err)
 }
+
+type pathrsProcBase C.pathrs_proc_base_t
+
+const (
+	pathrsProcSelf       pathrsProcBase = C.PATHRS_PROC_SELF
+	pathrsProcThreadSelf pathrsProcBase = C.PATHRS_PROC_THREAD_SELF
+)
+
+func pathrsProcOpen(base pathrsProcBase, path string, flags int) (uintptr, error) {
+	cBase := C.pathrs_proc_base_t(base)
+
+	cPath := C.CString(path)
+	defer C.free(unsafe.Pointer(cPath))
+
+	fd := C.pathrs_proc_open(cBase, cPath, C.int(flags))
+	return uintptr(fd), fetchError(fd)
+}
+
+func pathrsProcReadlink(base pathrsProcBase, path string) (string, error) {
+	cBase := C.pathrs_proc_base_t(base)
+
+	cPath := C.CString(path)
+	defer C.free(unsafe.Pointer(cPath))
+
+	size := 128
+	for {
+		linkBuf := make([]byte, size)
+		n := C.pathrs_proc_readlink(cBase, cPath, C.cast_ptr(unsafe.Pointer(&linkBuf[0])), C.ulong(len(linkBuf)))
+		switch {
+		case int(n) < 0:
+			return "", fetchError(n)
+		case int(n) <= len(linkBuf):
+			return string(linkBuf[:int(n)]), nil
+		default:
+			// The contents were truncated. Unlike readlinkat, pathrs returns
+			// the size of the link when it checked. So use the returned size
+			// as a basis for the reallocated size (but in order to avoid a DoS
+			// where a magic-link is growing by a single byte each iteration,
+			// make sure we are a fair bit larger).
+			size += int(n)
+		}
+	}
+}

go-pathrs/procfs_linux.go

@@ -0,0 +1,139 @@
+//go:build linux
+
+// libpathrs: safe path resolution on Linux
+// Copyright (C) 2019-2024 Aleksa Sarai <[email protected]>
+// Copyright (C) 2019-2024 SUSE LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package pathrs
+
+import (
+	"fmt"
+	"os"
+	"runtime"
+)
+
+type ProcBase int
+
+const (
+	unimplementedProcBaseRoot ProcBase = iota
+	// Use /proc/self. For most programs, this is the standard choice.
+	ProcBaseSelf
+	// Use /proc/thread-self. In multi-threaded programs where one thread has
+	// a different CLONE_FS, it is possible for /proc/self to point the wrong
+	// thread and so /proc/thread-self may be necessary.
+	ProcBaseThreadSelf
+)
+
+func (b ProcBase) toPathrsBase() (pathrsProcBase, error) {
+	switch b {
+	case ProcBaseSelf:
+		return pathrsProcSelf, nil
+	case ProcBaseThreadSelf:
+		return pathrsProcThreadSelf, nil
+	default:
+		return 0, fmt.Errorf("invalid proc base: %v", b)
+	}
+}
+
+// ProcHandleCloser is a callback that needs to be called when you are done
+// operating on an *os.File fetched using ProcThreadSelfOpen.
+type ProcHandleCloser func()
+
+// TODO: Consider exporting procOpen once we have ProcBaseRoot.
+
+func procOpen(base ProcBase, path string, flags int) (*os.File, ProcHandleCloser, error) {
+	pathrsBase, err := base.toPathrsBase()
+	if err != nil {
+		return nil, nil, err
+	}
+	switch base {
+	case ProcBaseSelf:
+		fd, err := pathrsProcOpen(pathrsBase, path, flags)
+		if err != nil {
+			return nil, nil, err
+		}
+		return os.NewFile(fd, "/proc/self/"+path), nil, nil
+	case ProcBaseThreadSelf:
+		runtime.LockOSThread()
+		fd, err := pathrsProcOpen(pathrsBase, path, flags)
+		if err != nil {
+			runtime.UnlockOSThread()
+			return nil, nil, err
+		}
+		return os.NewFile(fd, "/proc/thread-self/"+path), runtime.UnlockOSThread, nil
+	}
+	panic("unreachable")
+}
+
+// ProcSelfOpen safely opens a given path from inside /proc/self/.
+//
+// This method is recommend for getting process information about the current
+// process for almost all Go processes *except* for cases where there are
+// runtime.LockOSThread threads that have changed some aspect of their state
+// (such as through unshare(CLONE_FS) or changing namespaces).
+//
+// For such non-heterogeneous processes, /proc/self may reference to a task
+// that has different state from the current goroutine and so it may be
+// preferable to use ProcThreadSelfOpen. The same is true if a user really
+// wants to inspect the current OS thread's information (such as
+// /proc/thread-self/stack or /proc/thread-self/status which is always uniquely
+// per-thread).
+//
+// Unlike ProcThreadSelfOpen, this method does not involve locking the
+// goroutine to the current OS thread and so is simpler to use.
+func ProcSelfOpen(path string, flags int) (*os.File, error) {
+	file, closer, err := procOpen(ProcBaseSelf, path, flags)
+	if closer != nil {
+		// should not happen
+		panic("non-zero closer returned from procOpen(ProcBaseSelf)")
+	}
+	return file, err
+}
+
+// ProcThreadSelfOpen safely opens a given path from inside /proc/thread-self/.
+//
+// Most Go processes have heterogeneous threads (all threads have most of the
+// same kernel state such as CLONE_FS) and so ProcSelfOpen is preferable for
+// most users.
+//
+// For non-heterogeneous threads, or users that actually want thread-specific
+// information (such as /proc/thread-self/stack or /proc/thread-self/status),
+// this method is necessary.
+//
+// Because Go can change the running OS thread of your goroutine without notice
+// (and then subsequently kill the old thread), this method will lock the
+// current goroutine to ths OS thread (with runtime.LockOSThread) and the
+// caller is responsible for unlocking the the OS thread with the
+// ProcHandleCloser callback once they are done using the returned file. This
+// callback MUST be called AFTER you have finished using the returned *os.File.
+// This callback is completely separate to (*os.File).Close, so it must be
+// called regardless of how you close the handle.
+func ProcThreadSelfOpen(path string, flags int) (*os.File, ProcHandleCloser, error) {
+	return procOpen(ProcBaseThreadSelf, path, flags)
+}
+
+// ProcReadlink safely reads the contents of a symlink from the given procfs
+// base.
+//
+// This is effectively equivalent to doing a Proc*Open(O_PATH|O_NOFOLLOW) of
+// the path and then doing unix.Readlinkat(fd, ""), but with the benefit that
+// thread locking is not necessary for ProcBaseThreadSelf.
+func ProcReadlink(base ProcBase, path string) (string, error) {
+	pathrsBase, err := base.toPathrsBase()
+	if err != nil {
+		return "", err
+	}
+	return pathrsProcReadlink(pathrsBase, path)
+}