diff --git a/common/dbg/experiments.go b/common/dbg/experiments.go index 23d63dca492..6bdca9fa15f 100644 --- a/common/dbg/experiments.go +++ b/common/dbg/experiments.go @@ -137,7 +137,12 @@ var ( UseCodeStore = EnvBool("USE_CODE_STORE", true) DisableAdaptivePin = EnvBool("DISABLE_ADAPTIVE_PIN", true) AssertStateCache = EnvBool("ASSERT_STATE_CACHE", false) - ReadAhead = EnvBool("READ_AHEAD", true) + // MmapPoison makes a closed file revoke its mapping instead of unmapping it, + // turning a read through a slice that outlived its file into a fault at that + // read. Without it the address is free for the next file, and the stale read + // silently returns another file's bytes. Leaks address space — diagnostics only. + MmapPoison = EnvBool("MMAP_POISON", false) + ReadAhead = EnvBool("READ_AHEAD", true) // FilesAsyncIO warms cold state .kv pages via io_uring before the mmap read, so // a would-be blocking page fault becomes a non-blocking read that releases the // goroutine's P. Linux + io_uring only; self-disables (reads use ordinary faults) diff --git a/common/mmap/mmap_unix.go b/common/mmap/mmap_unix.go index c5ae2d56c7b..39d7be76690 100644 --- a/common/mmap/mmap_unix.go +++ b/common/mmap/mmap_unix.go @@ -91,3 +91,15 @@ func Munmap(mmapHandle1 []byte, _ *[MaxMapSize]byte) error { err := unix.Munmap(mmapHandle1) return err } + +// Poison revokes access to the range but deliberately leaves it mapped, so the +// address can never be reused. A slice that outlived its file then faults at +// the read that misuses it, instead of silently returning whichever file the +// kernel mapped there next. Diagnostics only: the range leaks for the life of +// the process. +func Poison(mmapHandle1 []byte, _ *[MaxMapSize]byte) error { + if mmapHandle1 == nil { + return nil + } + return unix.Mprotect(mmapHandle1, unix.PROT_NONE) +} diff --git a/common/mmap/mmap_windows.go b/common/mmap/mmap_windows.go index e29d6fc2c4b..b33fef1a434 100644 --- a/common/mmap/mmap_windows.go +++ b/common/mmap/mmap_windows.go @@ -66,3 +66,17 @@ func Munmap(_ []byte, mmapHandle2 *[MaxMapSize]byte) error { } return nil } + +// Poison revokes access to the range but deliberately leaves it mapped, so the +// address can never be reused. A slice that outlived its file then faults at +// the read that misuses it, instead of silently returning whichever file the +// kernel mapped there next. Diagnostics only: the range leaks for the life of +// the process. +func Poison(mmapHandle1 []byte, mmapHandle2 *[MaxMapSize]byte) error { + if mmapHandle2 == nil { + return nil + } + addr := (uintptr)(unsafe.Pointer(&mmapHandle2[0])) + var old uint32 + return windows.VirtualProtect(addr, uintptr(len(mmapHandle1)), windows.PAGE_NOACCESS, &old) +} diff --git a/common/mmap/poison_test.go b/common/mmap/poison_test.go new file mode 100644 index 00000000000..024711e397c --- /dev/null +++ b/common/mmap/poison_test.go @@ -0,0 +1,109 @@ +//go:build !windows + +package mmap + +import ( + "os" + "os/exec" + "path/filepath" + "runtime" + "strings" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/erigontech/erigon/common/dir" +) + +// A read through a poisoned range kills the process, so the read has to happen +// in a child. The parent asserts on how the child died. +const poisonChildEnv = "MMAP_POISON_TEST_CHILD" + +func TestMain(m *testing.M) { + if os.Getenv(poisonChildEnv) == "" { + os.Exit(m.Run()) + } + poisonChild() +} + +func poisonChild() { + f, err := os.CreateTemp("", "poison") + if err != nil { + os.Exit(3) + } + if err := f.Truncate(4096); err != nil { + os.Exit(3) + } + data, handle, err := Mmap(f, 4096) + if err != nil { + os.Exit(3) + } + borrowed := data[:8] + _ = borrowed[0] // readable while the file is open + + // Unlink while mapped, the way a merge disposes of a superseded file. + if err := dir.RemoveFile(f.Name()); err != nil { + os.Exit(3) + } + if err := Poison(data, handle); err != nil { + os.Exit(3) + } + // The file is "closed" now. A stale borrow must not survive this. + if borrowed[0] == 0xFF { + os.Exit(4) + } + os.Exit(5) // reached only if the read did not fault +} + +func TestPoisonFaultsOnStaleRead(t *testing.T) { + exe, err := os.Executable() + require.NoError(t, err) + + cmd := exec.Command(exe, "-test.run", "TestPoisonFaultsOnStaleRead") + cmd.Env = append(os.Environ(), poisonChildEnv+"=1") + out, err := cmd.CombinedOutput() + + require.Error(t, err, "reading a poisoned range must kill the child, got clean exit:\n%s", out) + require.Contains(t, string(out), "fatal error: fault", + "child should die on a memory fault, not exit normally:\n%s", out) + + // Which signal carries the fault is the OS's choice: Linux reports + // SIGSEGV/SEGV_ACCERR, Darwin reports SIGBUS for a protection violation on a + // file-backed mapping. Only Linux runs in production, so pin the code there. + if runtime.GOOS == "linux" { + require.Contains(t, string(out), "SIGSEGV") + require.Contains(t, string(out), "code=0x2", + "the range must still be mapped (SEGV_ACCERR), not unmapped (SEGV_MAPERR):\n%s", out) + } +} + +// TestPoisonKeepsAddressReserved is the property that makes poisoning useful: +// the range stays owned, so no later file can be handed the same address and +// answer a stale read with plausible bytes. +func TestPoisonKeepsAddressReserved(t *testing.T) { + dir := t.TempDir() + first, err := os.Create(filepath.Join(dir, "a.bin")) + require.NoError(t, err) + defer first.Close() + require.NoError(t, first.Truncate(4096)) + + data, handle, err := Mmap(first, 4096) + require.NoError(t, err) + addr := &data[0] + + require.NoError(t, Poison(data, handle)) + + // Map enough further files to make address reuse likely had the range + // been released. + for i := range 32 { + f, err := os.Create(filepath.Join(dir, strings.Repeat("b", i+1)+".bin")) + require.NoError(t, err) + require.NoError(t, f.Truncate(4096)) + next, _, err := Mmap(f, 4096) + require.NoError(t, err) + require.NotSame(t, addr, &next[0], + "a later file was handed the poisoned address — a stale read there would "+ + "return this file's bytes instead of faulting") + f.Close() + } +} diff --git a/db/seg/decompress.go b/db/seg/decompress.go index c1f94c91151..dded5e6a160 100644 --- a/db/seg/decompress.go +++ b/db/seg/decompress.go @@ -578,7 +578,11 @@ func (d *Decompressor) Close() { rb.stop() // join the refresh goroutine before munmap so no mincore touches freed memory d.residency.Store(nil) } - if err := mmap.Munmap(d.mmapHandle1, d.mmapHandle2); err != nil { + unmap := mmap.Munmap + if dbg.MmapPoison { + unmap = mmap.Poison + } + if err := unmap(d.mmapHandle1, d.mmapHandle2); err != nil { log.Log(dbg.FileCloseLogLevel, "unmap", "err", err, "file", d.FileName(), "stack", dbg.Stack()) } if err := d.f.Close(); err != nil { diff --git a/db/seg/poison_wiring_test.go b/db/seg/poison_wiring_test.go new file mode 100644 index 00000000000..a8c3e25024e --- /dev/null +++ b/db/seg/poison_wiring_test.go @@ -0,0 +1,46 @@ +//go:build linux + +package seg + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/stretchr/testify/require" + + "github.com/erigontech/erigon/common/dbg" +) + +// fileStillMapped reports whether this process still holds a mapping of path. +// Asking by file rather than by address matters: a released address is reused +// almost immediately, so "something is mapped here" says nothing about whether +// our file's mapping survived. +func fileStillMapped(t *testing.T, path string) bool { + t.Helper() + maps, err := os.ReadFile("/proc/self/maps") + require.NoError(t, err) + base := filepath.Base(path) + for line := range strings.SplitSeq(string(maps), "\n") { + if strings.HasSuffix(line, base) || strings.HasSuffix(line, base+" (deleted)") { + return true + } + } + return false +} + +// TestPoisonWiring proves the dbg flag reaches Decompressor.Close: with it set +// the closed file keeps its range (revoked, never reusable); without it the +// range is handed back to the kernel. +func TestPoisonWiring(t *testing.T) { + d := prepareLoremDict(t) + path := d.FilePath() + require.True(t, fileStillMapped(t, path), "mapped while open") + + d.Close() + + require.Equal(t, dbg.MmapPoison, fileStillMapped(t, path), + "poison must retain the mapping so the address cannot be handed to another "+ + "file; a plain Close must release it") +}