carlos / platform Public

geoip: the verifier child runs under a 512 MiB private-memory cap; retained recovery generations are cleaned when a later sync lands and destroyed at their GeoLite2 expiry (Astra code review R10)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

claude 2 weeks ago 98c0139bd6a1dbe6b4e1d94a57ffbbcdda0c3cac parent 0042860

9 files changed, +107 −11

internal/console/geoipfollower_test.go +15 −0
internal/console/geoippublish_test.go +1 −1
internal/console/geoipverify.go +8 −0
internal/console/geoipverify_linux.go +14 −0
internal/console/geoipverify_other.go +6 −0
internal/edge/geoipmirror.go +26 −9
internal/edge/geoipmirror_test.go +34 −1
internal/geo/testdata/README +3 −0
internal/geo/testdata/pathological-bigmap.mmdb binary

CI passed — run details

diff --git a/internal/console/geoipfollower_test.go b/internal/console/geoipfollower_test.go

index 9370e8fc9..68b139370 100644

--- a/internal/console/geoipfollower_test.go

+++ b/internal/console/geoipfollower_test.go

@@ -1,6 +1,7 @@

package console

import (

+ "context"

"net"

"os"

"path/filepath"

@@ -130,3 +131,17 @@ func TestRecordingLocatorNamesTheAnsweringReader(t *testing.T) {

t.Fatalf("recorded = %+v", rec)

}

+// TestRealDatabaseVerifiesInTheChild runs only with GEOIP_REAL_MMDB set:

+// a real database must fit the verifier child's memory cap.

+func TestRealDatabaseVerifiesInTheChild(t *testing.T) {

+ p := os.Getenv("GEOIP_REAL_MMDB")

+ if p == "" {

+ t.Skip("set GEOIP_REAL_MMDB to a real .mmdb")

+ }

+ res, err := verifyInChild(context.Background(), p, []string{"8.8.8.8", "1.1.1.1"})

+ if err != nil || res.Countries != 2 {

+ t.Fatalf("real database in the child = (%+v, %v)", res, err)

+ }

+ t.Logf("%s, %d nodes", res.Source, res.NodeCount)

diff --git a/internal/console/geoippublish_test.go b/internal/console/geoippublish_test.go

index 473432029..1f2082dc0 100644

--- a/internal/console/geoippublish_test.go

+++ b/internal/console/geoippublish_test.go

@@ -495,7 +495,7 @@ func TestPublishSurvivesDatabasesThatHangOrPanicTheParser(t *testing.T) {

if err != nil {

t.Fatal(err)

}

- for _, f := range []string{"pathological-empty-dag.mmdb", "pathological-metadata.mmdb"} {

+ for _, f := range []string{"pathological-empty-dag.mmdb", "pathological-metadata.mmdb", "pathological-bigmap.mmdb"} {

start := time.Now()

if _, err := p.Publish(ctx, bytes.NewReader(fixture(t, f)), geoipPublishOpts{Origin: "manual", Force: true}); !errors.Is(err, errGeoipRefused) {

t.Fatalf("%s: err = %v", f, err)

diff --git a/internal/console/geoipverify.go b/internal/console/geoipverify.go

index fce816964..496abaa90 100644

--- a/internal/console/geoipverify.go

+++ b/internal/console/geoipverify.go

@@ -41,6 +41,11 @@ func RunGeoipVerify(args []string, w io.Writer) int {

if len(args) < 1 {

return 2

}

+ // Measured: DB-IP City Lite (127 MB) verifies within 256 MiB of

+ // private memory; 512 MiB leaves room for GeoLite2 and growth.

+ if err := limitVerifierMemory(geoipVerifyDataLimit); err != nil {

+ return 2 // without the bound this process must not parse anything

+ }

db, err := geo.OpenMMDBVerified(context.Background(), args[0])

if errors.Is(err, geo.ErrSearchTreeTooLarge) {

return geoipVerifyExitTree

@@ -61,6 +66,9 @@ func RunGeoipVerify(args []string, w io.Writer) int {

return 0

}

+// geoipVerifyDataLimit is the child's private-memory cap (code review R10-1).

+const geoipVerifyDataLimit = 512 << 20

// geoipVerifyTimeout bounds the child; a var so a test need not wait.

var geoipVerifyTimeout = 5 * time.Minute

diff --git a/internal/console/geoipverify_linux.go b/internal/console/geoipverify_linux.go

new file mode 100644

index 000000000..230d94350

--- /dev/null

+++ b/internal/console/geoipverify_linux.go

@@ -0,0 +1,14 @@

+//go:build linux

+package console

+import "syscall"

+// limitVerifierMemory caps the verifier child's private writable memory

+// (RLIMIT_DATA, which Linux applies to anonymous mappings since 4.7) so a

+// tiny database declaring a 16-million-entry map dies in the child, not by

+// exhausting the console's cgroup (code review R10-1). The database file's

+// own read-only mapping does not count against it.

+func limitVerifierMemory(bytes uint64) error {

+ return syscall.Setrlimit(syscall.RLIMIT_DATA, &syscall.Rlimit{Cur: bytes, Max: bytes})

diff --git a/internal/console/geoipverify_other.go b/internal/console/geoipverify_other.go

new file mode 100644

index 000000000..70edc6e70

--- /dev/null

+++ b/internal/console/geoipverify_other.go

@@ -0,0 +1,6 @@

+//go:build !linux

+package console

+// limitVerifierMemory is Linux-only; the console runs on Linux boxes.

+func limitVerifierMemory(uint64) error { return nil }

diff --git a/internal/edge/geoipmirror.go b/internal/edge/geoipmirror.go

index 3420aff1e..947b73a38 100644

--- a/internal/edge/geoipmirror.go

+++ b/internal/edge/geoipmirror.go

@@ -52,10 +52,10 @@ type geoMirror struct {

loadedSHA string

loadedType string

loadedBuild time.Time

- skipSHA map[string]string // sha -> why (rejected | expired); cleared whenever the observed pointer changes

- pointerSHA string // the sha the last readable pointer named; "" for none

- syncPending bool // the last `current` rename is not known durable

- unsynced map[string]bool // every generation a crash could bring back while syncPending; kept until a sync succeeds

+ skipSHA map[string]string // sha -> why (rejected | expired); cleared whenever the observed pointer changes

+ pointerSHA string // the sha the last readable pointer named; "" for none

+ syncPending bool // the last `current` rename is not known durable

+ unsynced map[string]geoCurrent // every generation a crash could bring back while syncPending; kept until a sync succeeds or it expires

// The alert's inputs (geoip spec §10.4). The pointer's metadata is

// kept after the reader is dropped, so an expired pointer still reads

@@ -311,6 +311,15 @@ func (m *geoMirror) poll(ctx context.Context, recheck bool) error {

}

if p.SHA256 == m.loadedSHA {

m.lastErr = ""

+ if m.syncPending {

+ // The selection was never made durable: retry the sync, and once

+ // it lands the retained recovery copies can go (code review

+ // R10-2).

+ if err := syncDirFn(m.dir); err == nil {

+ m.syncPending, m.unsynced = false, nil

+ m.cleanLocked(m.loadedSHA, false)

+ }

m.genMu.Unlock()

return nil

}

@@ -439,7 +448,7 @@ func (m *geoMirror) install(tmp string, p geoip.Pointer) error {

m.removeGenerationLocked(p.SHA256)

return m.failLocked("install failed", err)

}

- prev := m.loadedSHA

+ prev := geoCurrent{SHA256: m.loadedSHA, DatabaseType: m.loadedType, BuildEpoch: m.loadedBuild.Unix()}

m.sw.Swap(db)

m.loadedSHA, m.loadedType, m.loadedBuild = p.SHA256, db.Source(), db.BuildEpoch()

if m.syncPending {

@@ -447,10 +456,10 @@ func (m *geoMirror) install(tmp string, p geoip.Pointer) error {

// could come back after a crash: keep every one of them (code

// review R2 on R1-4).

if m.unsynced == nil {

- m.unsynced = map[string]bool{}

+ m.unsynced = map[string]geoCurrent{}

}

- if prev != "" {

- m.unsynced[prev] = true

+ if prev.SHA256 != "" {

+ m.unsynced[prev.SHA256] = prev

}

keep := []string{p.SHA256}

for sha := range m.unsynced {

@@ -470,7 +479,7 @@ func (m *geoMirror) install(tmp string, p geoip.Pointer) error {

// fail): republishing a retained A must never delete A on an error path

// (code review R3 on R1-4).

func (m *geoMirror) removeGenerationLocked(sha string) {

- if sha == m.loadedSHA || m.unsynced[sha] {

+ if _, retained := m.unsynced[sha]; sha == m.loadedSHA || retained {

return

}

os.Remove(m.path(sha))

@@ -481,6 +490,14 @@ func (m *geoMirror) removeGenerationLocked(sha string) {

func (m *geoMirror) ExpireTick() {

m.genMu.Lock()

defer m.genMu.Unlock()

+ // Retained recovery copies obey D1 too (code review R10-2): an expired

+ // GeoLite2 kept only in case a crash brings it back is destroyed.

+ for sha, g := range m.unsynced {

+ if geoip.Expired(g.DatabaseType, time.Unix(g.BuildEpoch, 0), m.now()) {

+ os.Remove(m.path(sha))

+ delete(m.unsynced, sha)

+ }

if m.loadedSHA == "" || !geoip.Expired(m.loadedType, m.loadedBuild, m.now()) {

return

}

diff --git a/internal/edge/geoipmirror_test.go b/internal/edge/geoipmirror_test.go

index 7f3b385e4..3ff1b26d4 100644

--- a/internal/edge/geoipmirror_test.go

+++ b/internal/edge/geoipmirror_test.go

@@ -507,7 +507,7 @@ func TestRemoveGenerationKeepsRetainedOnes(t *testing.T) {

for _, sha := range []string{a, b, c} {

os.WriteFile(filepath.Join(dir, sha+".mmdb"), []byte("x"), 0o644)

}

- m.loadedSHA, m.unsynced = a, map[string]bool{b: true}

+ m.loadedSHA, m.unsynced = a, map[string]geoCurrent{b: {SHA256: b}}

for _, sha := range []string{a, b, c} {

m.removeGenerationLocked(sha)

}

@@ -560,3 +560,36 @@ func TestMirrorExpiryReportedWithAnUnreadableFeed(t *testing.T) {

t.Fatalf("evaluator = %v", ev)

}

+// Code review R10-2: one failed sync during a GeoLite2 -> DB-IP move; the

+// retained GeoLite2 copy goes once a later sync lands, or at its expiry.

+func TestMirrorRetainedCopyIsCleanedOrExpired(t *testing.T) {

+ for _, mode := range []string{"sync recovers", "expiry"} {

+ now := time.Date(2026, 10, 6, 0, 0, 0, 0, time.UTC)

+ m, s, dir := newTestMirror(t)

+ m.now = func() time.Time { return now }

+ a := publishFixture(t, s, geoFixture(t, "GeoIP2-City-Test.mmdb"), "GeoIP2-City", now)

+ m.Poll(context.Background())

+ m.genMu.Lock()

+ m.loadedType, m.loadedBuild = "GeoLite2-City", now.Add(-10*24*time.Hour) // A is a GeoLite2, 10 days old

+ m.genMu.Unlock()

+ syncDirFn = func(string) error { return errors.New("injected") }

+ publishFixture(t, s, geoFixture(t, "GeoIP2-Country-Test.mmdb"), "GeoIP2-Country", now)

+ m.Poll(context.Background())

+ if _, err := os.Stat(filepath.Join(dir, a.SHA256+".mmdb")); err != nil {

+ t.Fatalf("%s: A not retained", mode)

+ }

+ if mode == "sync recovers" {

+ syncDirFn = syncDir

+ m.Poll(context.Background()) // unchanged pointer

+ } else {

+ now = now.Add(21 * 24 * time.Hour) // A past its 30 days

+ m.ExpireTick()

+ syncDirFn = syncDir

+ }

+ if _, err := os.Stat(filepath.Join(dir, a.SHA256+".mmdb")); !os.IsNotExist(err) {

+ t.Fatalf("%s: A survived (%v)", mode, err)

+ }

+ syncDirFn = syncDir

diff --git a/internal/geo/testdata/README b/internal/geo/testdata/README

index 3a2460922..74832d7b7 100644

--- a/internal/geo/testdata/README

+++ b/internal/geo/testdata/README

@@ -11,3 +11,6 @@ steps inside one call. `pathological-metadata.mmdb` declares node_count

2^63, which overflows the library's own arithmetic and panics it. Both are

for the geoip code review R9 regressions: only the console's verifier

child may ever open them.

+`pathological-bigmap.mmdb` has one data record declaring a map of

+16,843,036 entries: maxminddb allocates it before noticing the entries are

+missing. The verifier child's memory cap must contain it.

diff --git a/internal/geo/testdata/pathological-bigmap.mmdb b/internal/geo/testdata/pathological-bigmap.mmdb

new file mode 100644

index 000000000..0d40669b5

Binary files /dev/null and b/internal/geo/testdata/pathological-bigmap.mmdb differ