Skip to content

fix: clear dnsmasq[0].server instead of dnsmasq[-1] when hijacking DNS - #5320

Open
HongyuQian wants to merge 1 commit into
vernesong:devfrom
HongyuQian:fix-dnsmasq0-server-clear
Open

HongyuQian wants to merge 1 commit into
vernesong:devfrom
HongyuQian:fix-dnsmasq0-server-clear

Conversation

@HongyuQian

Copy link
Copy Markdown

Problem

Both the DNS hijack setup and the watchdog's "Force Reset DNS Hijack" path clear the server list of the last dnsmasq section but append to the first one:

uci -q del dhcp.@dnsmasq[-1].server
uci -q add_list dhcp.@dnsmasq[0].server=127.0.0.1#"$dns_port"

With a single dnsmasq section — the common case — [-1] and [0] are the same section, so the delete does clear the list that add_list then writes to, and the pair behaves as "reset to exactly one entry".

With more than one config dnsmasq section (a documented OpenWrt feature, used for a second resolver instance on another port), [-1] is some other section. The delete no longer touches [0], and nothing else ever clears it, so add_list only ever appends.

That turns the watchdog's self-healing check into a self-sustaining loop. Its condition fires when dhcp.@dnsmasq[0].server has a second field:

[ ! -z "$(uci -q get dhcp.@dnsmasq[0].server |awk -F ' ' '{print $2}')" ]

Once a duplicate exists, every pass appends another entry, so the condition stays true forever — and each pass also runs /etc/init.d/dnsmasq restart.

Impact

On my router that meant a full dnsmasq restart every ~60–70s. Every instance goes down together for ~8–10s, so the whole LAN loses DNS in a loop. Devices that actively probe connectivity (a TV, in my case) report themselves as offline; laptops and phones mostly ride it out on their local caches, which is why it took a while to notice.

By the time I looked, dhcp.@dnsmasq[0].server had accumulated 6855 identical 127.0.0.1#7874 entries, and logread showed 32 dnsmasq restarts in 20 minutes:

Tue Sep  8 20:12:26 2026 daemon.info dnsmasq[1]: exiting on receipt of SIGTERM
Tue Sep  8 20:12:52 2026 daemon.info dnsmasq[1]: exiting on receipt of SIGTERM
Tue Sep  8 20:14:01 2026 daemon.info dnsmasq[1]: exiting on receipt of SIGTERM
Tue Sep  8 20:15:05 2026 daemon.info dnsmasq[1]: exiting on receipt of SIGTERM
...

Reproduce

  1. Add a second config dnsmasq section in /etc/config/dhcp (e.g. a second instance listening on another port).
  2. Get a duplicate into dhcp.@dnsmasq[0].server — any event that runs change_dnsmasq() while an entry is already there will do.
  3. Watch uci -q get dhcp.@dnsmasq[0].server | tr ' ' '\n' | wc -l grow by one per watchdog pass, and logread | grep dnsmasq show a restart on each pass.

Fix

Address [0] explicitly in all three places.

  • For a single dnsmasq section this is a no-op, since [-1] already resolves to [0] there — so the common configuration is unaffected.
  • For multiple sections it makes the delete hit the list that is actually appended to, so the pair converges on one entry again and the watchdog condition goes false after the first pass.

revert_dnsmasq() has the same asymmetry — it deletes [-1] and then restores the saved upstreams through set_dnsmasq_server(), which does uci add_list dhcp.@dnsmasq[0].server=... — so it is fixed the same way.

Verified

iStoreOS 24.10.8 (OpenWrt 24.10, x86_64), OpenClash 0.47.156, three dnsmasq instances. Before the change the list grew by one entry per watchdog pass. After it: four OpenClash restarts and a dozen dnsmasq restarts later, dhcp.@dnsmasq[0].server stays at exactly one entry and the restart loop is gone.

Possibly related

Some existing reports describe the symptom without a root cause, and may or may not be this bug — I have not reproduced their setups:

Both the DNS hijack setup and the watchdog's "Force Reset DNS Hijack"
path clear the server list of the *last* dnsmasq section but append to
the *first* one:

    uci -q del dhcp.@dnsmasq[-1].server
    uci -q add_list dhcp.@dnsmasq[0].server=127.0.0.1#"$dns_port"

With a single dnsmasq section - the common case - [-1] and [0] are the
same section, so the delete does clear the list the add_list then writes
to, and the pair behaves as "reset to exactly one entry".

With more than one `config dnsmasq` section (a documented OpenWrt
feature, used for a second resolver instance on another port), [-1] is
some other section. The delete no longer touches [0], and nothing else
ever clears it, so add_list only ever appends.

That turns the watchdog's self-healing check into a self-sustaining
loop. Its condition fires when dhcp.@dnsmasq[0].server has a second
field:

    [ ! -z "$(uci -q get dhcp.@dnsmasq[0].server |awk -F ' ' '{print $2}')" ]

Once a duplicate exists, every pass appends another entry, so the
condition stays true forever, and each pass also runs
`/etc/init.d/dnsmasq restart`. On my router that meant a full dnsmasq
restart every ~60-70s - every instance goes down together for ~8-10s, so
the whole LAN loses DNS in a loop. Devices that actively probe
connectivity (a TV, in my case) report themselves as offline; laptops
and phones mostly ride it out on their local caches, which is why it
took a while to notice. By the time I looked, dhcp.@dnsmasq[0].server
had accumulated 6855 identical `127.0.0.1#7874` entries, and logread
showed 32 dnsmasq restarts in 20 minutes.

Address [0] explicitly in all three places. For a single dnsmasq section
this is a no-op, since [-1] already resolves to [0] there. For multiple
sections it makes the delete hit the list that is actually appended to,
so the pair converges on one entry again and the watchdog condition goes
false after the first pass.

`revert_dnsmasq()` has the same asymmetry: it deletes [-1] and then
restores the saved upstreams through set_dnsmasq_server(), which does
`uci add_list dhcp.@dnsmasq[0].server=...`, so it is fixed the same way.

Verified on iStoreOS 24.10.8 (OpenWrt 24.10, x86_64) with three dnsmasq
instances: before the change the list grew by one entry per watchdog
pass; after it, four OpenClash restarts and a dozen dnsmasq restarts
later, it stays at exactly one entry and the restart loop is gone.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant