Skip to content

Race condition in plugin_cloak.go cache update #3210

Description

@hklcf

Bug Description

The Eval method in plugin_cloak.go has a race condition when two concurrent queries need to resolve the same cloaked hostname. The "last writer wins" cache update pattern means that a concurrent resolution can overwrite another goroutine's results.

Affected Code

plugin_cloak.go:300-334:

plugin.RUnlock()  // line 300 - release read lock
// ... DNS resolution (takes time, no lock held)
plugin.Lock()     // line 319
cloakedName.ipv4 = foundIPs[:n]  // line 325 - write
plugin.Unlock()   // line 331

Impact

Goroutine A resolves "example.com" to IPs [1.2.3.4, 5.6.7.8] and stores them. Goroutine B (concurrently) resolves the same name to [9.10.11.12, 13.14.15.16] and overwrites A's result. The cached IPs for the cloaked name can flap between two different resolution results depending on timing. This can cause inconsistent DNS responses for clients.

Fix

Use a per-name mutex or a compare-and-swap-like pattern to serialize DNS resolution per cloaked name. Alternatively, skip the cache update if another goroutine has already updated it:

plugin.Lock()
// Only update if another goroutine hasn't already done so
if cloakedName.lastUpdate4 == nil || now.After(*cloakedName.lastUpdate4) {
    cloakedName.lastUpdate4 = &now
    cloakedName.ipv4 = foundIPs[:n]
}
plugin.Unlock()

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions