From 7db5bbacb2f67f3e2218f6fcca1d90a76b521ea4 Mon Sep 17 00:00:00 2001 From: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com> Date: Wed, 29 Jul 2026 19:15:48 -0700 Subject: [PATCH] collector/filesystem: clamp negative free/avail block counts to zero Motivation: Issue #1672 reports node_filesystem_avail_bytes (and, less often, node_filesystem_free_bytes) occasionally reporting a value vastly larger than node_filesystem_size_bytes, while the size metric stays correct. The reporter's df output at the time showed a total size of 879510155264 bytes with an absurdly larger avail value. This matches a known, recurring class of kernel bug: statfs(2)'s f_bavail is computed by the kernel as f_bfree minus reserved blocks (e.g. ext3/ext4 reserves ~5% of blocks for root). When reserved blocks exceed free blocks - which happens as a filesystem fills up - that subtraction goes negative, but f_bavail/f_bfree are unsigned 64-bit kernel fields, so the negative result wraps via two's complement to a value near 2^64. This exact wraparound pattern has been documented independently for ext3/ext4 reserved-block exhaustion, CephFS, and AFS quota handling. golang.org/x/sys/unix.Statfs_t.Bavail/.Bfree are typed uint64 on linux/amd64, so node_exporter faithfully converts the wrapped kernel value into an enormous, misleading byte count - it is not miscomputing anything itself, it is relaying a bogus value as-is. Because the original report was never root-caused by the reporter or maintainer (see the issue thread), this change addresses the known, independently-documented wraparound class rather than a confirmed repro of this specific report; it is offered as a likely fix. Approach: Add a blocksToBytes helper in collector/filesystem_linux.go that treats a block count whose sign bit is set (i.e. int64(blocks) < 0) as a wrapped negative value and returns 0 instead of an enormous byte count. Apply it when computing free (buf.Bfree) and avail (buf.Bavail). buf.Blocks (size) is left unchanged since total block count is not derived from a subtraction that can go negative in the kernel. Zero is not merely a safe fallback here: when reserved blocks exceed free blocks, zero blocks truly are available to unprivileged users, so clamping avail to zero is the semantically correct value, not an approximation. Validation: - go build ./... and GOOS=linux GOARCH=amd64 go build ./... both pass. - GOOS=linux GOARCH=amd64 go vet ./collector/... passes. - GOOS=linux GOARCH=amd64 go test -c ./collector/ compiles cleanly (this file is Linux-only and the dev host is Darwin/arm64 with no Linux runtime available, so the compiled test binary could not be executed directly). - Added TestBlocksToBytes covering a normal value, zero, the smallest and largest block counts with/without the sign bit set, and math.MaxUint64 (wrapped -1). The underlying arithmetic was also manually verified against the original report's numbers: blocksToBytes(858896636, 1024) == 879510155264, matching the exact size value from the issue's df output. Fixes #1672 Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com> --- collector/filesystem_linux.go | 17 +++++++++-- collector/filesystem_linux_test.go | 49 ++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 2 deletions(-) diff --git a/collector/filesystem_linux.go b/collector/filesystem_linux.go index 038ff7970b..5cb7aec617 100644 --- a/collector/filesystem_linux.go +++ b/collector/filesystem_linux.go @@ -144,14 +144,27 @@ func (c *filesystemCollector) processStat(labels filesystemLabels) filesystemSta return filesystemStats{ labels: labels, size: float64(buf.Blocks) * float64(buf.Bsize), - free: float64(buf.Bfree) * float64(buf.Bsize), - avail: float64(buf.Bavail) * float64(buf.Bsize), + free: blocksToBytes(buf.Bfree, buf.Bsize), + avail: blocksToBytes(buf.Bavail, buf.Bsize), files: float64(buf.Files), filesFree: float64(buf.Ffree), ro: ro, } } +// blocksToBytes converts a block count reported by statfs(2) to bytes. +// Some filesystems (e.g. ext3/ext4 when reserved blocks are exhausted) can +// report a negative free/available block count that overflows the kernel's +// unsigned f_bfree/f_bavail fields, wrapping to a value near 2^64. Treat any +// block count whose sign bit is set as a wrapped negative value and report +// zero instead of an enormous, misleading byte count. +func blocksToBytes(blocks uint64, bsize int64) float64 { + if int64(blocks) < 0 { + return 0 + } + return float64(blocks) * float64(bsize) +} + // stuckMountWatcher listens on the given success channel and if the channel closes // then the watcher does nothing. If instead the timeout is reached, the // mount point that is being watched is marked as stuck. diff --git a/collector/filesystem_linux_test.go b/collector/filesystem_linux_test.go index 9e8869016a..d44134fd8a 100644 --- a/collector/filesystem_linux_test.go +++ b/collector/filesystem_linux_test.go @@ -18,6 +18,7 @@ package collector import ( "io" "log/slog" + "math" "sort" "strings" "testing" @@ -288,6 +289,54 @@ func TestMountOptionsStringReadOnlyDetection(t *testing.T) { } } +func TestBlocksToBytes(t *testing.T) { + tests := []struct { + name string + blocks uint64 + bsize int64 + want float64 + }{ + { + name: "normal value", + blocks: 1000, + bsize: 4096, + want: 1000 * 4096, + }, + { + name: "zero blocks", + blocks: 0, + bsize: 4096, + want: 0, + }, + { + name: "negative block count wrapped into uint64 clamps to zero", + blocks: math.MaxUint64, // two's complement of -1, as reported when reserved blocks exceed free blocks + bsize: 4096, + want: 0, + }, + { + name: "smallest block count with sign bit set clamps to zero", + blocks: 1 << 63, + bsize: 4096, + want: 0, + }, + { + name: "largest block count without sign bit set is not clamped", + blocks: 1<<63 - 1, + bsize: 4096, + want: float64(1<<63-1) * 4096, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := blocksToBytes(tt.blocks, tt.bsize); got != tt.want { + t.Errorf("blocksToBytes(%d, %d) = %v, want %v", tt.blocks, tt.bsize, got, tt.want) + } + }) + } +} + func TestPathRootfs(t *testing.T) { if _, err := kingpin.CommandLine.Parse([]string{"--path.procfs", "./fixtures_bindmount/proc", "--path.rootfs", "/host"}); err != nil { t.Fatal(err)