From a06216ef9b4c63cb2ecc4b6de732963452631d29 Mon Sep 17 00:00:00 2001 From: leodido <120051+leodido@users.noreply.github.com> Date: Thu, 23 Apr 2026 16:38:04 +0000 Subject: [PATCH] fix: FeatureBPFFS / FeatureTraceFS verify filesystem type The probe behind FeatureBPFFS and FeatureTraceFS used os.Stat + IsDir, returning Supported=true on any host where the directory existed. On modern systemd distros /sys/fs/bpf and /sys/kernel/tracing exist by default whether or not bpffs/tracefs is actually mounted, so callers gating on these features (e.g. before bpf_obj_pin) silently got a false positive and failed at I/O time. Switch the probe to Statfs + superblock magic comparison (BPF_FS_MAGIC, TRACEFS_MAGIC). Introduce a single internal checkMount helper with an injectable statfs implementation so unit tests can exercise all branches without privileged operations. Diagnostic-only fields SystemFeatures.DebugFS and .SecurityFS keep their previous presence-only semantics (no gated Feature* exists for them today). --- CHANGELOG.md | 4 + README.md | 2 +- caps.go | 70 +++++++++++- caps_mount_statfs32_test.go | 11 ++ caps_mount_statfs64_test.go | 11 ++ caps_mount_test.go | 213 ++++++++++++++++++++++++++++++++++++ probe.go | 21 +++- probe_test.go | 18 +-- 8 files changed, 332 insertions(+), 18 deletions(-) create mode 100644 caps_mount_statfs32_test.go create mode 100644 caps_mount_statfs64_test.go create mode 100644 caps_mount_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 3807123..a2d3461 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- `Check(FeatureBPFFS)` and `Check(FeatureTraceFS)` now verify the filesystem is actually mounted with the expected superblock magic (`BPF_FS_MAGIC`, `TRACEFS_MAGIC`) instead of only checking that a directory exists at the path. Previously, both gates returned success on any system where `/sys/fs/bpf` (resp. `/sys/kernel/tracing`) existed as a directory — which is the case by default on systemd-based distros even when the corresponding pseudo-filesystem is not mounted. Callers gating on these features (e.g. before pinning maps on bpffs) would silently get a false positive and fail later at `bpf_obj_pin()`. Diagnostic-only fields `SystemFeatures.DebugFS` / `.SecurityFS` keep their previous presence-only semantics. + ## [0.2.0] — 2026-02-18 ### Changed diff --git a/README.md b/README.md index 6e0ebae..e312aca 100644 --- a/README.md +++ b/README.md @@ -238,7 +238,7 @@ JSON output example: | Capabilities and runtime gates | CAP_BPF, CAP_SYS_ADMIN, CAP_PERFMON, unprivileged BPF disabled, BPF stats enabled | | Syscalls | `bpf()`, `perf_event_open()` | | JIT | enabled, hardened, kallsyms, memory limit, `CONFIG_BPF_JIT_ALWAYS_ON` | -| Filesystems | tracefs, debugfs, securityfs, bpffs | +| Filesystems | tracefs, debugfs, securityfs, bpffs (gated `tracefs` and `bpffs` checks verify the filesystem is mounted with the expected superblock magic) | | Namespaces | initial user namespace, initial PID namespace | | Parameterized workload requirements | program type, map type, helper-per-program-type via requirement items | | Mitigation context | Spectre v1/v2 vulnerability status | diff --git a/caps.go b/caps.go index 8468d41..83ea5db 100644 --- a/caps.go +++ b/caps.go @@ -4,6 +4,7 @@ package kfeatures import ( "errors" + "fmt" "os" "strconv" "strings" @@ -111,9 +112,11 @@ const ( bpffsPath = "/sys/fs/bpf" ) -// probeFilesystemMount checks if any of the given paths exist and are directories. -// Returns Supported=true if at least one path is a mounted directory. -func probeFilesystemMount(paths ...string) ProbeResult { +// probeFilesystemPresent checks whether any of the given paths exists and is a +// directory. It is a presence-only probe: it does not verify the filesystem +// type. Used for diagnostic-only signals (DebugFS, SecurityFS) where we only +// care that the directory is reachable. +func probeFilesystemPresent(paths ...string) ProbeResult { for _, path := range paths { info, err := os.Stat(path) if err != nil { @@ -126,6 +129,67 @@ func probeFilesystemMount(paths ...string) ProbeResult { return ProbeResult{Supported: false} } +// statfs is the package-level Statfs implementation used by checkMount. +// Tests substitute a fake to exercise the mount-state code paths without +// requiring privileged operations. +var statfs = unix.Statfs + +// checkMount verifies that the given path is mounted with a filesystem whose +// f_type matches magic. Returns nil on match. Returns a typed error when the +// path is missing, the syscall fails, or the filesystem type does not match. +// +// The comparison casts st.Type to uint32 because Statfs_t.Type is signed on +// some architectures and the well-known magic numbers are documented as +// unsigned 32-bit values. +func checkMount(path string, magic uint32) error { + var st unix.Statfs_t + if err := statfs(path, &st); err != nil { + if errors.Is(err, unix.ENOENT) { + return fmt.Errorf("%s: not found", path) + } + return fmt.Errorf("statfs %s: %w", path, err) + } + if uint32(st.Type) != magic { + return fmt.Errorf("%s: not mounted with expected filesystem (got 0x%x, want 0x%x)", + path, uint32(st.Type), magic) + } + return nil +} + +// probeFilesystemMounted verifies that path is mounted with the expected +// filesystem type (identified by its superblock magic number). Used for +// readiness gates (FeatureBPFFS, FeatureTraceFS) where directory presence +// alone is misleading: systemd creates many of these directories at boot +// regardless of whether the corresponding pseudo-filesystem is mounted. +func probeFilesystemMounted(path string, magic uint32) ProbeResult { + if err := checkMount(path, magic); err != nil { + return ProbeResult{Supported: false, Error: err} + } + return ProbeResult{Supported: true} +} + +// probeFilesystemMountedAny returns Supported=true if any of the candidate +// path/magic pairs is mounted. The Error from the last attempt is preserved +// when no candidate matches, to aid diagnostics. +func probeFilesystemMountedAny(candidates ...mountCandidate) ProbeResult { + var last error + for _, c := range candidates { + if err := checkMount(c.path, c.magic); err == nil { + return ProbeResult{Supported: true} + } else { + last = err + } + } + return ProbeResult{Supported: false, Error: last} +} + +// mountCandidate pairs a filesystem path with the magic number expected for +// its superblock f_type. +type mountCandidate struct { + path string + magic uint32 +} + // probeBPFSyscall checks if the bpf() syscall is available. // It issues a minimal BPF_PROG_TYPE_UNSPEC command that is guaranteed to fail // with EINVAL (syscall exists) or ENOSYS (syscall not available). diff --git a/caps_mount_statfs32_test.go b/caps_mount_statfs32_test.go new file mode 100644 index 0000000..036bcf8 --- /dev/null +++ b/caps_mount_statfs32_test.go @@ -0,0 +1,11 @@ +//go:build linux && (386 || arm || mips || mipsle) + +package kfeatures + +import "golang.org/x/sys/unix" + +// setStatfsType assigns magic to st.Type on architectures where the kernel +// exposes f_type as a 32-bit signed integer. +func setStatfsType(st *unix.Statfs_t, magic uint32) { + st.Type = int32(magic) +} diff --git a/caps_mount_statfs64_test.go b/caps_mount_statfs64_test.go new file mode 100644 index 0000000..cadf829 --- /dev/null +++ b/caps_mount_statfs64_test.go @@ -0,0 +1,11 @@ +//go:build linux && (amd64 || arm64 || ppc64 || ppc64le || mips64 || mips64le || riscv64 || s390x || loong64) + +package kfeatures + +import "golang.org/x/sys/unix" + +// setStatfsType assigns magic to st.Type on architectures where the kernel +// exposes f_type as a 64-bit signed integer. +func setStatfsType(st *unix.Statfs_t, magic uint32) { + st.Type = int64(magic) +} diff --git a/caps_mount_test.go b/caps_mount_test.go new file mode 100644 index 0000000..bf0e25e --- /dev/null +++ b/caps_mount_test.go @@ -0,0 +1,213 @@ +//go:build linux + +package kfeatures + +import ( + "errors" + "strings" + "testing" + + "golang.org/x/sys/unix" +) + +// withFakeStatfs swaps the package-level statfs implementation for the +// duration of t and restores the original on cleanup. +func withFakeStatfs(t *testing.T, fake func(path string, st *unix.Statfs_t) error) { + t.Helper() + prev := statfs + statfs = fake + t.Cleanup(func() { statfs = prev }) +} + +// fakeStatfsTable returns a statfs implementation that serves results from +// a path → (magic, error) table. Paths absent from the table return ENOENT. +func fakeStatfsTable(table map[string]struct { + magic uint32 + err error +}) func(string, *unix.Statfs_t) error { + return func(path string, st *unix.Statfs_t) error { + entry, ok := table[path] + if !ok { + return unix.ENOENT + } + if entry.err != nil { + return entry.err + } + // Statfs_t.Type is int64 on 64-bit architectures and int32 on + // 32-bit ones; setStatfsType handles both via build-tagged files. + setStatfsType(st, entry.magic) + return nil + } +} + +func TestCheckMount(t *testing.T) { + const ( + path = "/some/mount" + want = unix.BPF_FS_MAGIC + ) + + t.Run("matching magic returns nil", func(t *testing.T) { + withFakeStatfs(t, fakeStatfsTable(map[string]struct { + magic uint32 + err error + }{ + path: {magic: want}, + })) + if err := checkMount(path, want); err != nil { + t.Fatalf("checkMount() = %v, want nil", err) + } + }) + + t.Run("magic mismatch returns descriptive error", func(t *testing.T) { + withFakeStatfs(t, fakeStatfsTable(map[string]struct { + magic uint32 + err error + }{ + path: {magic: unix.TMPFS_MAGIC}, + })) + err := checkMount(path, want) + if err == nil { + t.Fatal("checkMount() = nil, want error") + } + msg := err.Error() + for _, want := range []string{path, "not mounted with expected filesystem"} { + if !strings.Contains(msg, want) { + t.Errorf("error %q missing %q", msg, want) + } + } + }) + + t.Run("missing path returns not-found error", func(t *testing.T) { + withFakeStatfs(t, fakeStatfsTable(map[string]struct { + magic uint32 + err error + }{})) + err := checkMount(path, want) + if err == nil { + t.Fatal("checkMount() = nil, want error") + } + if !strings.Contains(err.Error(), "not found") { + t.Errorf("error %q should report not found", err.Error()) + } + }) + + t.Run("syscall errno is wrapped", func(t *testing.T) { + withFakeStatfs(t, fakeStatfsTable(map[string]struct { + magic uint32 + err error + }{ + path: {err: unix.EACCES}, + })) + err := checkMount(path, want) + if err == nil { + t.Fatal("checkMount() = nil, want error") + } + if !errors.Is(err, unix.EACCES) { + t.Errorf("checkMount() error chain should contain EACCES, got %v", err) + } + if !strings.Contains(err.Error(), "statfs") { + t.Errorf("error %q should mention statfs", err.Error()) + } + }) +} + +func TestProbeFilesystemMounted(t *testing.T) { + t.Run("mounted with expected magic", func(t *testing.T) { + withFakeStatfs(t, fakeStatfsTable(map[string]struct { + magic uint32 + err error + }{ + "/sys/fs/bpf": {magic: unix.BPF_FS_MAGIC}, + })) + got := probeFilesystemMounted("/sys/fs/bpf", unix.BPF_FS_MAGIC) + if !got.Supported { + t.Fatalf("Supported = false (err=%v), want true", got.Error) + } + if got.Error != nil { + t.Errorf("Error = %v, want nil", got.Error) + } + }) + + t.Run("directory exists but wrong filesystem", func(t *testing.T) { + withFakeStatfs(t, fakeStatfsTable(map[string]struct { + magic uint32 + err error + }{ + "/sys/fs/bpf": {magic: unix.SYSFS_MAGIC}, + })) + got := probeFilesystemMounted("/sys/fs/bpf", unix.BPF_FS_MAGIC) + if got.Supported { + t.Error("Supported = true, want false (wrong FS magic)") + } + if got.Error == nil { + t.Error("Error = nil, want descriptive error") + } + }) + + t.Run("path missing", func(t *testing.T) { + withFakeStatfs(t, fakeStatfsTable(nil)) + got := probeFilesystemMounted("/missing", unix.BPF_FS_MAGIC) + if got.Supported { + t.Error("Supported = true, want false") + } + }) +} + +func TestProbeFilesystemMountedAny(t *testing.T) { + primary := mountCandidate{path: "/sys/kernel/tracing", magic: unix.TRACEFS_MAGIC} + fallback := mountCandidate{path: "/sys/kernel/debug/tracing", magic: unix.TRACEFS_MAGIC} + + t.Run("primary mounted", func(t *testing.T) { + withFakeStatfs(t, fakeStatfsTable(map[string]struct { + magic uint32 + err error + }{ + primary.path: {magic: unix.TRACEFS_MAGIC}, + })) + got := probeFilesystemMountedAny(primary, fallback) + if !got.Supported { + t.Errorf("Supported = false (err=%v), want true", got.Error) + } + }) + + t.Run("fallback mounted", func(t *testing.T) { + withFakeStatfs(t, fakeStatfsTable(map[string]struct { + magic uint32 + err error + }{ + fallback.path: {magic: unix.TRACEFS_MAGIC}, + })) + got := probeFilesystemMountedAny(primary, fallback) + if !got.Supported { + t.Errorf("Supported = false (err=%v), want true", got.Error) + } + }) + + t.Run("neither mounted", func(t *testing.T) { + withFakeStatfs(t, fakeStatfsTable(nil)) + got := probeFilesystemMountedAny(primary, fallback) + if got.Supported { + t.Error("Supported = true, want false") + } + if got.Error == nil { + t.Error("Error = nil, want last attempt's error") + } + }) + + t.Run("magic mismatch on both", func(t *testing.T) { + withFakeStatfs(t, fakeStatfsTable(map[string]struct { + magic uint32 + err error + }{ + primary.path: {magic: unix.SYSFS_MAGIC}, + fallback.path: {magic: unix.SYSFS_MAGIC}, + })) + got := probeFilesystemMountedAny(primary, fallback) + if got.Supported { + t.Error("Supported = true, want false") + } + if got.Error == nil || !strings.Contains(got.Error.Error(), "not mounted") { + t.Errorf("Error = %v, want magic-mismatch message", got.Error) + } + }) +} diff --git a/probe.go b/probe.go index 5d3be88..d6f3a5b 100644 --- a/probe.go +++ b/probe.go @@ -241,12 +241,23 @@ func ProbeWith(opts ...ProbeOption) (*SystemFeatures, error) { sf.JITLimit = probeJITLimit() } - // Probe filesystem mounts + // Probe filesystem mounts. + // + // TraceFS and BPFFS are gated features (FeatureTraceFS, FeatureBPFFS): a + // caller asking "is bpffs ready?" wants to know whether they can pin maps, + // not whether some directory exists at /sys/fs/bpf. We therefore verify + // the filesystem is actually mounted with the expected superblock magic. + // + // DebugFS and SecurityFS are diagnostic-only fields (no Feature* gate), + // so the looser presence-only check is sufficient for them. if cfg.filesystems { - sf.TraceFS = probeFilesystemMount(tracefsPath, tracefsFallbackPath) - sf.DebugFS = probeFilesystemMount(debugfsPath) - sf.SecurityFS = probeFilesystemMount(securityfsPath) - sf.BPFFS = probeFilesystemMount(bpffsPath) + sf.TraceFS = probeFilesystemMountedAny( + mountCandidate{path: tracefsPath, magic: unix.TRACEFS_MAGIC}, + mountCandidate{path: tracefsFallbackPath, magic: unix.TRACEFS_MAGIC}, + ) + sf.DebugFS = probeFilesystemPresent(debugfsPath) + sf.SecurityFS = probeFilesystemPresent(securityfsPath) + sf.BPFFS = probeFilesystemMounted(bpffsPath, unix.BPF_FS_MAGIC) } // Probe CPU mitigations and JIT-always-on diff --git a/probe_test.go b/probe_test.go index 57d6eab..c88024b 100644 --- a/probe_test.go +++ b/probe_test.go @@ -171,27 +171,27 @@ func TestProbeWith_WithAll(t *testing.T) { } } -func TestProbeFilesystemMount(t *testing.T) { +func TestProbeFilesystemPresent(t *testing.T) { t.Run("existing directory", func(t *testing.T) { dir := t.TempDir() - result := probeFilesystemMount(dir) + result := probeFilesystemPresent(dir) if !result.Supported { - t.Error("probeFilesystemMount should return Supported=true for existing directory") + t.Error("probeFilesystemPresent should return Supported=true for existing directory") } }) t.Run("nonexistent path", func(t *testing.T) { - result := probeFilesystemMount("/nonexistent/path/that/should/not/exist") + result := probeFilesystemPresent("/nonexistent/path/that/should/not/exist") if result.Supported { - t.Error("probeFilesystemMount should return Supported=false for nonexistent path") + t.Error("probeFilesystemPresent should return Supported=false for nonexistent path") } }) t.Run("fallback to second path", func(t *testing.T) { dir := t.TempDir() - result := probeFilesystemMount("/nonexistent/path", dir) + result := probeFilesystemPresent("/nonexistent/path", dir) if !result.Supported { - t.Error("probeFilesystemMount should return Supported=true when fallback path exists") + t.Error("probeFilesystemPresent should return Supported=true when fallback path exists") } }) @@ -201,9 +201,9 @@ func TestProbeFilesystemMount(t *testing.T) { if err := os.WriteFile(path, []byte("data"), 0644); err != nil { t.Fatal(err) } - result := probeFilesystemMount(path) + result := probeFilesystemPresent(path) if result.Supported { - t.Error("probeFilesystemMount should return Supported=false for regular file") + t.Error("probeFilesystemPresent should return Supported=false for regular file") } }) }