diff options
author | Simon Kelley <simon@thekelleys.org.uk> | 2023-01-11 23:23:40 +0000 |
---|---|---|
committer | Simon Kelley <simon@thekelleys.org.uk> | 2023-01-13 21:12:53 +0000 |
commit | f172fdbb77c422e27d3b7530f3fe95b98d1608e7 (patch) | |
tree | f9403c1cb03f10c9a8634d543cb862671429bc98 /src/cache.c | |
parent | 3822825e54d758bbbfd16ee1e6db2206029387a6 (diff) | |
download | dnsmasq-f172fdbb77c422e27d3b7530f3fe95b98d1608e7.tar.gz |
Fix bug which can break the invariants on the order of a hash chain.
If there are multiple cache records with the same name but different
F_REVERSE and/or F_IMMORTAL flags, the code added in fe9a134b could
concievable break the REVERSE-FORWARD-IMMORTAL order invariant.
Reproducing this is damn near impossible, but it is responsible
for rare and otherwise inexplicable reversion between 2.87 and 2.88
which manifests itself as a cache internal error. All observed
cases have depended on DNSSEC being enabled, but the bug could in
theory manifest itself without DNSSEC
Thanks to Timo van Roermund for reporting the bug and huge
efforts to isolate it.
Diffstat (limited to 'src/cache.c')
-rw-r--r-- | src/cache.c | 14 |
1 files changed, 9 insertions, 5 deletions
diff --git a/src/cache.c b/src/cache.c index 42283bc..0a5fd14 100644 --- a/src/cache.c +++ b/src/cache.c @@ -236,19 +236,23 @@ static void cache_hash(struct crec *crecp) char *name = cache_get_name(crecp); struct crec **up = hash_bucket(name); - - if (!(crecp->flags & F_REVERSE)) + unsigned int flags = crecp->flags & (F_IMMORTAL | F_REVERSE); + + if (!(flags & F_REVERSE)) { while (*up && ((*up)->flags & F_REVERSE)) up = &((*up)->hash_next); - if (crecp->flags & F_IMMORTAL) + if (flags & F_IMMORTAL) while (*up && !((*up)->flags & F_IMMORTAL)) up = &((*up)->hash_next); } - /* Preserve order when inserting the same name multiple times. */ - while (*up && hostname_isequal(cache_get_name(*up), name)) + /* Preserve order when inserting the same name multiple times. + Do not mess up the flag invariants. */ + while (*up && + hostname_isequal(cache_get_name(*up), name) && + flags == ((*up)->flags & (F_IMMORTAL | F_REVERSE))) up = &((*up)->hash_next); crecp->hash_next = *up; |