From 3b103ae8aa494c843e83c7901f8dfe8f85f744d8 Mon Sep 17 00:00:00 2001 From: Simon Kelley Date: Thu, 22 Dec 2022 23:19:05 +0000 Subject: [PATCH 1/7] If we hit a cache internal error, log the entry we failed to remove. This is code which should never run, but if it does, we now log information useful for debugging. Signed-off-by: DL6ER --- src/dnsmasq/cache.c | 180 ++++++++++++++++++++++++-------------------- 1 file changed, 98 insertions(+), 82 deletions(-) diff --git a/src/dnsmasq/cache.c b/src/dnsmasq/cache.c index 89c40f82e..d423f264d 100644 --- a/src/dnsmasq/cache.c +++ b/src/dnsmasq/cache.c @@ -29,6 +29,7 @@ static int bignames_left, hash_size; static void make_non_terminals(struct crec *source); static struct crec *really_insert(char *name, union all_addr *addr, unsigned short class, time_t now, unsigned long ttl, unsigned int flags); +static void dump_cache_entry(struct crec *cache, time_t now); /* type->string mapping: this is also used by the name-hash function as a mixing table. */ /* taken from https://www.iana.org/assignments/dns-parameters/dns-parameters.xhtml */ @@ -595,7 +596,7 @@ static struct crec *really_insert(char *name, union all_addr *addr, unsigned sho struct crec *new, *target_crec = NULL; union bigname *big_name = NULL; int freed_all = (flags & F_REVERSE); - int free_avail = 0; + struct crec *free_avail = NULL; unsigned int target_uid; /* if previous insertion failed give up now. */ @@ -643,7 +644,7 @@ static struct crec *really_insert(char *name, union all_addr *addr, unsigned sho /* Free entry at end of LRU list, use it. */ if (!(new->flags & (F_FORWARD | F_REVERSE))) - break; + break; /* End of LRU list is still in use: if we didn't scan all the hash chains for expired entries do that now. If we already tried that @@ -659,6 +660,8 @@ static struct crec *really_insert(char *name, union all_addr *addr, unsigned sho if (!warned) { my_syslog(LOG_ERR, _("Internal error in cache.")); + /* Log the entry we tried to delete. */ + dump_cache_entry(free_avail, now); warned = 1; } insert_error = 1; @@ -668,13 +671,13 @@ static struct crec *really_insert(char *name, union all_addr *addr, unsigned sho if (freed_all) { /* For DNSSEC records, uid holds class. */ - free_avail = 1; /* Must be free space now. */ + free_avail = new; /* Must be free space now. */ /* condition valid when stale-caching */ if (difftime(now, new->ttd) < 0) daemon->metrics[METRIC_DNS_CACHE_LIVE_FREED]++; - cache_scan_free(cache_get_name(new), &new->addr, new->uid, now, new->flags, NULL, NULL); + cache_scan_free(cache_get_name(new), &new->addr, new->uid, now, new->flags, NULL, NULL); } else { @@ -1767,6 +1770,95 @@ static char *sanitise(char *name) return name; } +static void dump_cache_entry(struct crec *cache, time_t now) +{ + (void)now; + static char *buff = NULL; + + char *p, *t = " "; + char *a = daemon->addrbuff, *n = cache_get_name(cache); + + /* String length is limited below */ + if (!buff && !(buff = whine_malloc(150))) + return; + + p = buff; + + *a = 0; + if (strlen(n) == 0 && !(cache->flags & F_REVERSE)) + n = ""; + p += sprintf(p, "%-30.30s ", sanitise(n)); + if ((cache->flags & F_CNAME) && !is_outdated_cname_pointer(cache)) + a = sanitise(cache_get_cname_target(cache)); + else if ((cache->flags & F_SRV) && !(cache->flags & F_NEG)) + { + int targetlen = cache->addr.srv.targetlen; + ssize_t len = sprintf(a, "%u %u %u ", cache->addr.srv.priority, + cache->addr.srv.weight, cache->addr.srv.srvport); + + if (targetlen > (40 - len)) + targetlen = 40 - len; + blockdata_retrieve(cache->addr.srv.target, targetlen, a + len); + a[len + targetlen] = 0; + } +#ifdef HAVE_DNSSEC + else if (cache->flags & F_DS) + { + if (!(cache->flags & F_NEG)) + sprintf(a, "%5u %3u %3u", cache->addr.ds.keytag, + cache->addr.ds.algo, cache->addr.ds.digest); + } + else if (cache->flags & F_DNSKEY) + sprintf(a, "%5u %3u %3u", cache->addr.key.keytag, + cache->addr.key.algo, cache->addr.key.flags); +#endif + else if (!(cache->flags & F_NEG) || !(cache->flags & F_FORWARD)) + { + a = daemon->addrbuff; + if (cache->flags & F_IPV4) + inet_ntop(AF_INET, &cache->addr, a, ADDRSTRLEN); + else if (cache->flags & F_IPV6) + inet_ntop(AF_INET6, &cache->addr, a, ADDRSTRLEN); + } + + if (cache->flags & F_IPV4) + t = "4"; + else if (cache->flags & F_IPV6) + t = "6"; + else if (cache->flags & F_CNAME) + t = "C"; + else if (cache->flags & F_SRV) + t = "V"; +#ifdef HAVE_DNSSEC + else if (cache->flags & F_DS) + t = "S"; + else if (cache->flags & F_DNSKEY) + t = "K"; +#endif + else /* non-terminal */ + t = "!"; + + p += sprintf(p, "%-40.40s %s%s%s%s%s%s%s%s%s%s ", a, t, + cache->flags & F_FORWARD ? "F" : " ", + cache->flags & F_REVERSE ? "R" : " ", + cache->flags & F_IMMORTAL ? "I" : " ", + cache->flags & F_DHCP ? "D" : " ", + cache->flags & F_NEG ? "N" : " ", + cache->flags & F_NXDOMAIN ? "X" : " ", + cache->flags & F_HOSTS ? "H" : " ", + cache->flags & F_CONFIG ? "C" : " ", + cache->flags & F_DNSSECOK ? "V" : " "); +#ifdef HAVE_BROKEN_RTC + p += sprintf(p, "%-24lu", cache->flags & F_IMMORTAL ? 0: (unsigned long)(cache->ttd - now)); +#else + p += sprintf(p, "%-24.24s", cache->flags & F_IMMORTAL ? "" : ctime(&(cache->ttd))); +#endif + if(cache->flags & (F_HOSTS | F_CONFIG) && cache->uid > 0) + p += sprintf(p, " %-40.40s", record_source(cache->uid)); + + my_syslog(LOG_INFO, "%s", buff); +} + /***************** Pi-hole modification *****************/ void get_dnsmasq_cache_info(struct cache_info *ci) { @@ -1847,90 +1939,14 @@ void dump_cache(time_t now) if (option_bool(OPT_DEBUG) || option_bool(OPT_LOG)) { - struct crec *cache ; + struct crec *cache; int i; my_syslog(LOG_INFO, "Host Address Flags Expires Source"); my_syslog(LOG_INFO, "------------------------------ ---------------------------------------- ---------- ------------------------ ------------"); for (i=0; ihash_next) - { - char *t = " "; - char *a = daemon->addrbuff, *p = daemon->namebuff, *n = cache_get_name(cache); - *a = 0; - if (strlen(n) == 0 && !(cache->flags & F_REVERSE)) - n = ""; - p += sprintf(p, "%-30.30s ", sanitise(n)); - if ((cache->flags & F_CNAME) && !is_outdated_cname_pointer(cache)) - a = sanitise(cache_get_cname_target(cache)); - else if ((cache->flags & F_SRV) && !(cache->flags & F_NEG)) - { - int targetlen = cache->addr.srv.targetlen; - ssize_t len = sprintf(a, "%u %u %u ", cache->addr.srv.priority, - cache->addr.srv.weight, cache->addr.srv.srvport); - - if (targetlen > (40 - len)) - targetlen = 40 - len; - blockdata_retrieve(cache->addr.srv.target, targetlen, a + len); - a[len + targetlen] = 0; - } -#ifdef HAVE_DNSSEC - else if (cache->flags & F_DS) - { - if (!(cache->flags & F_NEG)) - sprintf(a, "%5u %3u %3u", cache->addr.ds.keytag, - cache->addr.ds.algo, cache->addr.ds.digest); - } - else if (cache->flags & F_DNSKEY) - sprintf(a, "%5u %3u %3u", cache->addr.key.keytag, - cache->addr.key.algo, cache->addr.key.flags); -#endif - else if (!(cache->flags & F_NEG) || !(cache->flags & F_FORWARD)) - { - a = daemon->addrbuff; - if (cache->flags & F_IPV4) - inet_ntop(AF_INET, &cache->addr, a, ADDRSTRLEN); - else if (cache->flags & F_IPV6) - inet_ntop(AF_INET6, &cache->addr, a, ADDRSTRLEN); - } - - if (cache->flags & F_IPV4) - t = "4"; - else if (cache->flags & F_IPV6) - t = "6"; - else if (cache->flags & F_CNAME) - t = "C"; - else if (cache->flags & F_SRV) - t = "V"; -#ifdef HAVE_DNSSEC - else if (cache->flags & F_DS) - t = "S"; - else if (cache->flags & F_DNSKEY) - t = "K"; -#endif - else /* non-terminal */ - t = "!"; - - p += sprintf(p, "%-40.40s %s%s%s%s%s%s%s%s%s%s ", a, t, - cache->flags & F_FORWARD ? "F" : " ", - cache->flags & F_REVERSE ? "R" : " ", - cache->flags & F_IMMORTAL ? "I" : " ", - cache->flags & F_DHCP ? "D" : " ", - cache->flags & F_NEG ? "N" : " ", - cache->flags & F_NXDOMAIN ? "X" : " ", - cache->flags & F_HOSTS ? "H" : " ", - cache->flags & F_CONFIG ? "C" : " ", - cache->flags & F_DNSSECOK ? "V" : " "); -#ifdef HAVE_BROKEN_RTC - p += sprintf(p, "%-24lu", cache->flags & F_IMMORTAL ? 0: (unsigned long)(cache->ttd - now)); -#else - p += sprintf(p, "%-24.24s", cache->flags & F_IMMORTAL ? "" : ctime(&(cache->ttd))); -#endif - if(cache->flags & (F_HOSTS | F_CONFIG) && cache->uid > 0) - p += sprintf(p, " %s", record_source(cache->uid)); - - my_syslog(LOG_INFO, "%s", daemon->namebuff); - } + dump_cache_entry(cache, now); } } From 21cec0c01dbefcd0cacf6e8b4be7f486112ca0c4 Mon Sep 17 00:00:00 2001 From: Simon Kelley Date: Mon, 2 Jan 2023 22:17:57 +0000 Subject: [PATCH 2/7] Log all cache internal errors. Signed-off-by: DL6ER --- src/dnsmasq/cache.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/src/dnsmasq/cache.c b/src/dnsmasq/cache.c index d423f264d..1c8b8f0ba 100644 --- a/src/dnsmasq/cache.c +++ b/src/dnsmasq/cache.c @@ -656,14 +656,9 @@ static struct crec *really_insert(char *name, union all_addr *addr, unsigned sho insert. Once in this state, all inserts will probably fail. */ if (free_avail) { - static int warned = 0; - if (!warned) - { - my_syslog(LOG_ERR, _("Internal error in cache.")); - /* Log the entry we tried to delete. */ - dump_cache_entry(free_avail, now); - warned = 1; - } + my_syslog(LOG_ERR, _("Internal error in cache.")); + /* Log the entry we tried to delete. */ + dump_cache_entry(free_avail, now); insert_error = 1; return NULL; } From 45a760b3f8e5485fe04c88da978404d1f3b3cdb9 Mon Sep 17 00:00:00 2001 From: Simon Kelley Date: Wed, 4 Jan 2023 23:10:07 +0000 Subject: [PATCH 3/7] Fix cosmetic big in dump_cache_entry() Signed-off-by: DL6ER --- src/dnsmasq/cache.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/dnsmasq/cache.c b/src/dnsmasq/cache.c index 1c8b8f0ba..5ad39a2b2 100644 --- a/src/dnsmasq/cache.c +++ b/src/dnsmasq/cache.c @@ -1079,7 +1079,6 @@ void add_hosts_entry(struct crec *cache, union all_addr *addr, int addrlen, the array rhash, hashed on address. Note that rhash and the values in ->next are only valid whilst reading hosts files: the buckets are then freed, and the ->next pointer used for other things. - Only insert each unique address once into this hashing structure. This complexity avoids O(n^2) divergent CPU use whilst reading @@ -1830,7 +1829,7 @@ static void dump_cache_entry(struct crec *cache, time_t now) else if (cache->flags & F_DNSKEY) t = "K"; #endif - else /* non-terminal */ + else if (!(cache->flags & F_NXDOMAIN)) /* non-terminal */ t = "!"; p += sprintf(p, "%-40.40s %s%s%s%s%s%s%s%s%s%s ", a, t, From 33c04059f4452236fd07f8023dd72f2228dc3ad1 Mon Sep 17 00:00:00 2001 From: Simon Kelley Date: Wed, 11 Jan 2023 23:23:40 +0000 Subject: [PATCH 4/7] 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. Signed-off-by: DL6ER --- src/dnsmasq/cache.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/dnsmasq/cache.c b/src/dnsmasq/cache.c index 5ad39a2b2..0816cb545 100644 --- a/src/dnsmasq/cache.c +++ b/src/dnsmasq/cache.c @@ -237,19 +237,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; From 1b62122a8c588e54d829a80f124c3ebaabe49bed Mon Sep 17 00:00:00 2001 From: Dominik Derigs Date: Mon, 23 Jan 2023 22:48:01 +0000 Subject: [PATCH 5/7] Add --no-ident option. Signed-off-by: DL6ER --- src/dnsmasq/dnsmasq.h | 3 ++- src/dnsmasq/option.c | 49 ++++++++++++++++++++++++------------------- 2 files changed, 30 insertions(+), 22 deletions(-) diff --git a/src/dnsmasq/dnsmasq.h b/src/dnsmasq/dnsmasq.h index 9c9e825fe..2883b8d67 100644 --- a/src/dnsmasq/dnsmasq.h +++ b/src/dnsmasq/dnsmasq.h @@ -281,7 +281,8 @@ struct event_desc { #define OPT_STRIP_ECS 69 #define OPT_STRIP_MAC 70 #define OPT_NORR 71 -#define OPT_LAST 72 +#define OPT_NO_IDENT 72 +#define OPT_LAST 73 #define OPTION_BITS (sizeof(unsigned int)*8) #define OPTION_SIZE ( (OPT_LAST/OPTION_BITS)+((OPT_LAST%OPTION_BITS)!=0) ) diff --git a/src/dnsmasq/option.c b/src/dnsmasq/option.c index 0c61a22f6..96c094c61 100644 --- a/src/dnsmasq/option.c +++ b/src/dnsmasq/option.c @@ -189,6 +189,7 @@ struct myoption { #define LOPT_FAST_RETRY 376 #define LOPT_STALE_CACHE 377 #define LOPT_NORR 378 +#define LOPT_NO_IDENT 379 #ifdef HAVE_GETOPT_LONG static const struct option opts[] = @@ -378,6 +379,7 @@ static const struct myoption opts[] = { "port-limit", 1, 0, LOPT_RANDPORT_LIM }, { "fast-dns-retry", 2, 0, LOPT_FAST_RETRY }, { "use-stale-cache", 2, 0 , LOPT_STALE_CACHE }, + { "no-ident", 0, 0, LOPT_NO_IDENT }, { NULL, 0, 0, 0 } }; @@ -574,6 +576,7 @@ static struct { { LOPT_UMBRELLA, ARG_ONE, "[=]", gettext_noop("Send Cisco Umbrella identifiers including remote IP."), NULL }, { LOPT_QUIET_TFTP, OPT_QUIET_TFTP, NULL, gettext_noop("Do not log routine TFTP."), NULL }, { LOPT_NORR, OPT_NORR, NULL, gettext_noop("Suppress round-robin ordering of DNS records."), NULL }, + { LOPT_NO_IDENT, OPT_NO_IDENT, NULL, gettext_noop("Do not add CHAOS TXT records."), NULL }, { 0, 0, NULL, NULL, NULL } }; @@ -5761,27 +5764,6 @@ void read_opts(int argc, char **argv, char *compile_opts) daemon->randport_limit = 1; daemon->host_index = SRC_AH; -#ifndef NO_ID - add_txt("version.bind", "dnsmasq-" VERSION, 0 ); - add_txt("authors.bind", "Simon Kelley", 0); - add_txt("copyright.bind", COPYRIGHT, 0); - add_txt("cachesize.bind", NULL, TXT_STAT_CACHESIZE); - add_txt("insertions.bind", NULL, TXT_STAT_INSERTS); - add_txt("evictions.bind", NULL, TXT_STAT_EVICTIONS); - add_txt("misses.bind", NULL, TXT_STAT_MISSES); - add_txt("hits.bind", NULL, TXT_STAT_HITS); -#ifdef HAVE_AUTH - add_txt("auth.bind", NULL, TXT_STAT_AUTH); -#endif - add_txt("servers.bind", NULL, TXT_STAT_SERVERS); - /* Pi-hole modification */ - add_txt("privacylevel.pihole", NULL, TXT_PRIVACYLEVEL); - /************************/ -#endif - /******** Pi-hole modification ********/ - add_txt("version.FTL", (char*)get_FTL_version(), 0 ); - /**************************************/ - /* See comment above make_servers(). Optimises server-read code. */ mark_servers(0); @@ -5879,6 +5861,31 @@ void read_opts(int argc, char **argv, char *compile_opts) else one_file(CONFFILE, LOPT_CONF_OPT); + /* Add TXT records if wanted */ +#ifndef NO_ID + if (!option_bool(OPT_NO_IDENT)) + { + add_txt("version.bind", "dnsmasq-" VERSION, 0 ); + add_txt("authors.bind", "Simon Kelley", 0); + add_txt("copyright.bind", COPYRIGHT, 0); + add_txt("cachesize.bind", NULL, TXT_STAT_CACHESIZE); + add_txt("insertions.bind", NULL, TXT_STAT_INSERTS); + add_txt("evictions.bind", NULL, TXT_STAT_EVICTIONS); + add_txt("misses.bind", NULL, TXT_STAT_MISSES); + add_txt("hits.bind", NULL, TXT_STAT_HITS); +#ifdef HAVE_AUTH + add_txt("auth.bind", NULL, TXT_STAT_AUTH); +#endif + add_txt("servers.bind", NULL, TXT_STAT_SERVERS); + /* Pi-hole modification */ + add_txt("privacylevel.pihole", NULL, TXT_PRIVACYLEVEL); + /************************/ + } +#endif + /******** Pi-hole modification ********/ + add_txt("version.FTL", (char*)get_FTL_version(), 0 ); + /**************************************/ + /* port might not be known when the address is parsed - fill in here */ if (daemon->servers) { From f493af3e430a0589536c728dacffdb7fb98ff226 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Fri, 27 Jan 2023 16:38:24 +0100 Subject: [PATCH 6/7] Update dnsmasq version to 2.89rc1 Signed-off-by: DL6ER --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 76f7c68fc..1d049915c 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -11,6 +11,6 @@ cmake_minimum_required(VERSION 2.8.12) project(PIHOLE_FTL C) -set(DNSMASQ_VERSION pi-hole-v2.88) +set(DNSMASQ_VERSION pi-hole-v2.89rc1) add_subdirectory(src) From 6edb071f5407ec74b405eb3e6ddf8a0180c68398 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Tue, 7 Feb 2023 19:12:00 +0100 Subject: [PATCH 7/7] Update dnsmasq version to 2.89 Signed-off-by: DL6ER --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 1d049915c..d468618a4 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -11,6 +11,6 @@ cmake_minimum_required(VERSION 2.8.12) project(PIHOLE_FTL C) -set(DNSMASQ_VERSION pi-hole-v2.89rc1) +set(DNSMASQ_VERSION pi-hole-v2.89) add_subdirectory(src)