From 9bf5de8e9297446f68178f566481b9fa188e8545 Mon Sep 17 00:00:00 2001 From: Pavel Lazureykis Date: Thu, 9 Jul 2026 00:44:16 -0400 Subject: [PATCH] plugin/hosts: fix data race between lookups and reload (#8253) LookupStaticHostV4/V6 dereferenced h.hmap and h.inline as call arguments, which are evaluated before lookupStaticHostFamily takes the read lock. The reload path (readHosts) swaps h.hmap under h.Lock() on every reload, so the field read raced the swap for every A/AAAA lookup. This was introduced when wildcard support (#8185) refactored these methods to pass the maps as parameters; LookupStaticAddr still reads the fields inside the lock and was unaffected. Read h.hmap/h.inline inside the RLock by selecting the address family with a bool instead of passing pre-dereferenced maps. Also read h.mtime under the existing RLock in readHosts: it was read without a lock while the reload writes it under h.Lock(), and readHosts runs from both the OnStartup handler and the reload ticker goroutine. Both races are confirmed by go test -race. Signed-off-by: Pavel Lazureykis --- plugin/hosts/hostsfile.go | 21 ++++++++++----- plugin/hosts/hostsfile_test.go | 49 ++++++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 6 deletions(-) diff --git a/plugin/hosts/hostsfile.go b/plugin/hosts/hostsfile.go index fb41b05d6..f3ef2cc75 100644 --- a/plugin/hosts/hostsfile.go +++ b/plugin/hosts/hostsfile.go @@ -134,9 +134,10 @@ func (h *Hostsfile) readHosts() { } h.RLock() size := h.size + mtime := h.mtime h.RUnlock() - if h.mtime.Equal(stat.ModTime()) && size == stat.Size() { + if mtime.Equal(stat.ModTime()) && size == stat.Size() { return } @@ -239,25 +240,33 @@ func (h *Hostsfile) lookupStaticHostLocked(m, wild map[string][]net.IP, host str return nil } -func (h *Hostsfile) lookupStaticHostFamily(host string, m, wild, inlineM, inlineWild map[string][]net.IP) []net.IP { +func (h *Hostsfile) lookupStaticHostFamily(host string, v4 bool) []net.IP { host = strings.ToLower(host) h.RLock() defer h.RUnlock() - ip1 := h.lookupStaticHostLocked(m, wild, host) - ip2 := h.lookupStaticHostLocked(inlineM, inlineWild, host) + // h.hmap and h.inline must be read under the lock: readHosts swaps h.hmap + // under h.Lock() on every reload. + var ip1, ip2 []net.IP + if v4 { + ip1 = h.lookupStaticHostLocked(h.hmap.name4, h.hmap.wildName4, host) + ip2 = h.lookupStaticHostLocked(h.inline.name4, h.inline.wildName4, host) + } else { + ip1 = h.lookupStaticHostLocked(h.hmap.name6, h.hmap.wildName6, host) + ip2 = h.lookupStaticHostLocked(h.inline.name6, h.inline.wildName6, host) + } return append(ip1, ip2...) } // LookupStaticHostV4 looks up the IPv4 addresses for the given host from the hosts file. func (h *Hostsfile) LookupStaticHostV4(host string) []net.IP { - return h.lookupStaticHostFamily(host, h.hmap.name4, h.hmap.wildName4, h.inline.name4, h.inline.wildName4) + return h.lookupStaticHostFamily(host, true) } // LookupStaticHostV6 looks up the IPv6 addresses for the given host from the hosts file. func (h *Hostsfile) LookupStaticHostV6(host string) []net.IP { - return h.lookupStaticHostFamily(host, h.hmap.name6, h.hmap.wildName6, h.inline.name6, h.inline.wildName6) + return h.lookupStaticHostFamily(host, false) } // LookupStaticAddr looks up the hosts for the given address from the hosts file. diff --git a/plugin/hosts/hostsfile_test.go b/plugin/hosts/hostsfile_test.go index fb558ebd2..4d9595220 100644 --- a/plugin/hosts/hostsfile_test.go +++ b/plugin/hosts/hostsfile_test.go @@ -6,8 +6,10 @@ package hosts import ( "net" + "os" "reflect" "strings" + "sync" "testing" "github.com/coredns/coredns/plugin" @@ -241,3 +243,50 @@ func TestHostCacheModification(t *testing.T) { } testStaticAddr(t, entip, h) } + +// TestLookupStaticHostReloadRace exercises concurrent A/AAAA lookups against a +// reload that swaps h.hmap. Before the fix, LookupStaticHostV4/V6 dereferenced +// h.hmap/h.inline as call arguments (outside the RLock), racing the swap that +// readHosts performs under h.Lock(). Run with -race. +func TestLookupStaticHostReloadRace(t *testing.T) { + f, err := os.CreateTemp(t.TempDir(), "hosts") + if err != nil { + t.Fatal(err) + } + if _, err := f.WriteString("127.0.0.1 example.org\n::1 example.org\n"); err != nil { + t.Fatal(err) + } + f.Close() + + h := &Hostsfile{ + Origins: []string{"."}, + hmap: newMap(), + inline: newMap(), + options: newOptions(), + path: f.Name(), + } + + var wg sync.WaitGroup + + // Reloader: force a re-parse + h.hmap swap on every iteration. + wg.Go(func() { + for range 1000 { + h.Lock() + h.size = 0 + h.Unlock() + h.readHosts() + } + }) + + // Readers: concurrent A/AAAA lookups. + for range 4 { + wg.Go(func() { + for range 1000 { + h.LookupStaticHostV4("example.org.") + h.LookupStaticHostV6("example.org.") + } + }) + } + + wg.Wait() +}