I am quite sure also --rebind-domain-ok is broken. It allocates struct
serv_local, but does not set any flags if I saw it correctly. Therefore
it would be later threated as struct server and checked in wrong places.

That is still unfixed even after commit
de372d6914ae20a1f9997815f258efbf3b14c39b.

Made that change separate commit, rebased the previous change with small
changes.

On 9/23/21 12:28, Simon Kelley wrote:
>
> On 22/09/2021 23:07, Petr Menšík wrote:
>> Good catch. A new bug #2006367 [1] were also reported on Fedora. It
>> seems to point to related structures and memory corruption in them. I
>> have no coredump to check it (yet), so mostly guessing.
>>
>> Juggling with type unsafe structures with few common members is quite
>> bad idea. I think those structures should contain common server_local
>> struct member at the start. They could pass pointer to it on every place
>> which needs working just with those common parts.
>>
>> On domain-match.c:677 is also suspicious memset. Its flags are not
>> directly related to allocated size. I think there might be a case, when
>> it overwrites more memory than allocated for the pointer. On line 696 it
>> may overwrite interface target even with flags SERV_4ADDR | SERV_6ADDR.
>> Both allow rewriting uid member when HAVE_LOOP is set, which is a
>> default. 
> SERV_IS_LOCAL is defined at the top of the file, and provides a simple
> check. If that's NOT true then the record is a complete server struct.
> If it is set then it's a struct serv_local, possibly with added IPv4 or
> IPV6 address, controlled by the SERV_4ADDR and SERV_6ADDR flags bits.
>
> /* If the server is USE_RESOLV or LITERAL_ADDRESS, it lives on the
> local_domains chain. */
> #define SERV_IS_LOCAL (SERV_USE_RESOLV | SERV_LITERAL_ADDRESS)
>
> With that information, it's fairly easy to see that the code is correct.
>
> The problem here was nothing to do with this code, the domain search
> code assumed that --server=/example.com/# would not be set along with
> --server=/example.com/#/<address> and when that was done, it wrongly
> returned BOTH server records, each if which has different lengths. The
> consumer of that set of records  assumed (as it was entitled to do) they
> were of the same type and length, hence the dereferencing of fields
> outside allocated memory.
>
>> I see many tricky corners without simple and readable checks
>> ensuring it always does what it should. I think char type enum would
>> definitely not hurt in common structure instead of this juggling with
>> flags. It would be much more clear what members are available. I think
>> default struct should be the smallest one and only retyped to bigger
>> struct, if some flag clearly indicated it is there. Preferred would be
>> separate type member.
> A major goal of this rewrite was to avoid wasting memory when there are
> 100,000 --address=/adserver.com/ lines in the configuration. Doing what
> you suggest would break that.
>> At first it should be fixed by minimal fix. I think constant sized
>> structure with some unused members would be far more safe. I think union
>> would be good candidate here. Its a pity we did not notice those issues
>> before release. I should spend some time on basic automated tests again.
>> I think dnsmasq it small, but needs more regular testing.
>>
>
> Agree entirely. I still have you test system in a git branch, and would
> like to progress it. It would have been nice to find the regression
> (removal) of --address=/#/..... before release.
>
>
> Cheers,
>
> Simon.
>
>
>
>> Cheers,
>>
>> Petr
>>
>> 1. https://bugzilla.redhat.com/show_bug.cgi?id=2006367
>>
>> On 9/16/21 16:31, Dominik DL6ER wrote:
>>> Addendum: Depending on the configuration, it can happen that the
>>> query is sent to another server that is configured to be used for
>>> an altogether different domain, e.g.
>>>
>>>> server=127.0.0.1#5353
>>>> server=::1#5353
>>>> rev-server=192.168.0.1/24,192.168.0.1
>>>> server=/fritz.box/192.168.0.1
>>>> server=/bo.net/#
>>>> address=/bo.net/#/
>>> resulting in "A bo.net" being sent to 192.168.0.1
>>>
>>> Something is definitely fishy here.
>>>
>>> Best,
>>> Dominik

-- 
Petr Menšík
Software Engineer
Red Hat, http://www.redhat.com/
email: pemen...@redhat.com
PGP: DFCF908DB7C87E8E529925BC4931CA5B6C9FC5CB
From f3819fc908d34c7f8bd15be21ec7e96457d26c16 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Petr=20Men=C5=A1=C3=ADk?= <pemen...@redhat.com>
Date: Thu, 23 Sep 2021 11:21:59 +0200
Subject: [PATCH 3/3] Separate limited serv_local from full struct server
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Make sure distinction from separate serv_local and server structure is
obvious.

Finish serv_local separation, make serv_addr4 and serv_addr6 have it as a member.
Adds more checks into different places, ensuring members used are used
only when flags indicate it were allocated.

Signed-off-by: Petr Menšík <pemen...@redhat.com>
---
 src/dnsmasq.h      |  24 ++++-----
 src/domain-match.c | 121 ++++++++++++++++++++++++++-------------------
 src/forward.c      |  65 ++++++++++++++----------
 src/network.c      |  13 ++---
 src/option.c       |   6 +--
 5 files changed, 131 insertions(+), 98 deletions(-)

diff --git a/src/dnsmasq.h b/src/dnsmasq.h
index 639c568..e7a6b6d 100644
--- a/src/dnsmasq.h
+++ b/src/dnsmasq.h
@@ -548,6 +548,9 @@ union mysockaddr {
 #define SERV_DO_DNSSEC      16384  /* Validate DNSSEC when using this server */
 #define SERV_GOT_TCP        32768  /* Got some data from the TCP connection */
 
+/* If the server is USE_RESOLV or LITERAL_ADDRES, it lives on the local_domains chain. */
+#define SERV_IS_LOCAL (SERV_USE_RESOLV | SERV_LITERAL_ADDRESS)
+
 struct serverfd {
   int fd;
   union mysockaddr source_addr;
@@ -589,26 +592,22 @@ struct server {
 };
 
 /* First four fields must match struct server in next three definitions.. */
-struct serv_addr4 {
+struct serv_local {
   u16 flags, domain_len;
   char *domain;
-  struct server *next;
+  struct serv_local *next;
+};
+
+struct serv_addr4 {
+  struct serv_local local;
   struct in_addr addr;
 };
 
 struct serv_addr6 {
-  u16 flags, domain_len;
-  char *domain;
-  struct server *next;
+  struct serv_local local;
   struct in6_addr addr;
 };
 
-struct serv_local {
-  u16 flags, domain_len;
-  char *domain;
-  struct server *next;
-};
-
 struct ipsets {
   char **sets;
   char *domain;
@@ -1105,7 +1104,8 @@ extern struct daemon {
   char *lease_change_command;
   struct iname *if_names, *if_addrs, *if_except, *dhcp_except, *auth_peers, *tftp_interfaces;
   struct bogus_addr *bogus_addr, *ignore_addr;
-  struct server *servers, *servers_tail, *local_domains, **serverarray, *no_rebind;
+  struct server *servers, *servers_tail;
+  struct serv_local *local_domains, **serverarray, *no_rebind;
   int server_has_wildcard;
   int serverarraysz, serverarrayhwm;
   struct ipsets *ipsets;
diff --git a/src/domain-match.c b/src/domain-match.c
index 3f1cc74..605867c 100644
--- a/src/domain-match.c
+++ b/src/domain-match.c
@@ -16,16 +16,22 @@
 
 #include "dnsmasq.h"
 
-static int order(char *qdomain, size_t qlen, struct server *serv);
+static int order(char *qdomain, size_t qlen, struct serv_local *serv);
 static int order_qsort(const void *a, const void *b);
-static int order_servers(struct server *s, struct server *s2);
+static int order_servers(struct serv_local *s, struct serv_local *s2);
 
-/* If the server is USE_RESOLV or LITERAL_ADDRES, it lives on the local_domains chain. */
-#define SERV_IS_LOCAL (SERV_USE_RESOLV | SERV_LITERAL_ADDRESS)
+/* copy of function from forward.c, static to be inlined. */
+static struct server *server_from_local(struct serv_local *sl)
+{
+  if (!(sl->flags & SERV_IS_LOCAL))
+    return (struct server *) sl;
+  return NULL;
+}
 
 void build_server_array(void)
 {
   struct server *serv;
+  struct serv_local *sl;
   int count = 0;
   
   for (serv = daemon->servers; serv; serv = serv->next)
@@ -38,10 +44,10 @@ void build_server_array(void)
 	  daemon->server_has_wildcard = 1;
       }
   
-  for (serv = daemon->local_domains; serv; serv = serv->next)
+  for (sl = daemon->local_domains; sl; sl = sl->next)
     {
       count++;
-      if (serv->flags & SERV_WILDCARD)
+      if (sl->flags & SERV_WILDCARD)
 	daemon->server_has_wildcard = 1;
     }
   
@@ -49,12 +55,13 @@ void build_server_array(void)
 
   if (count > daemon->serverarrayhwm)
     {
-      struct server **new;
+      struct serv_local **new;
 
       count += 10; /* A few extra without re-allocating. */
 
       if ((new = whine_malloc(count * sizeof(struct server *))))
 	{
+	  // TODO: use realloc here!
 	  if (daemon->serverarray)
 	    free(daemon->serverarray);
 	  
@@ -70,22 +77,22 @@ void build_server_array(void)
     if (!(serv->flags & SERV_LOOP))
 #endif
       {
-	daemon->serverarray[count] = serv;
+	daemon->serverarray[count] = (struct serv_local *) serv;
 	serv->serial = count;
 	serv->last_server = -1;
 	count++;
       }
   
-  for (serv = daemon->local_domains; serv; serv = serv->next, count++)
-    daemon->serverarray[count] = serv;
+  for (sl = daemon->local_domains; sl; sl = sl->next, count++)
+    daemon->serverarray[count] = sl;
   
-  qsort(daemon->serverarray, daemon->serverarraysz, sizeof(struct server *), order_qsort);
+  qsort(daemon->serverarray, daemon->serverarraysz, sizeof(struct serv_local *), order_qsort);
   
   /* servers need the location in the array to find all the whole
      set of equivalent servers from a pointer to a single one. */
   for (count = 0; count < daemon->serverarraysz; count++)
-    if (!(daemon->serverarray[count]->flags & SERV_IS_LOCAL))
-      daemon->serverarray[count]->arrayposn = count;
+    if ((serv = server_from_local(daemon->serverarray[count])))
+      serv->arrayposn = count;
 }
 
 /* we're looking for the server whose domain is the longest exact match
@@ -258,7 +265,7 @@ int lookup_domain(char *domain, int flags, int *lowout, int *highout)
 /* Return first server in group of equivalent servers; this is the "master" record. */
 int server_samegroup(struct server *a, struct server *b)
 {
-  return order_servers(a, b) == 0;
+  return order_servers((struct serv_local *)a, (struct serv_local *)b) == 0;
 }
 
 int filter_servers(int seed, int flags, int *lowout, int *highout)
@@ -411,12 +418,13 @@ size_t make_local_answer(int flags, int gotname, size_t size, struct dns_header
   if (flags & gotname & F_IPV4)
     for (start = first; start != last; start++)
       {
-	struct serv_addr4 *srv = (struct serv_addr4 *)daemon->serverarray[start];
-
-	if (srv->flags & SERV_ALL_ZEROS)
+	struct serv_local *serv = daemon->serverarray[start];
+	if (serv->flags & SERV_4ADDR)
+	  addr.addr4 = ((struct serv_addr4 *)serv)->addr;
+	else if (serv->flags & SERV_ALL_ZEROS)
 	  memset(&addr, 0, sizeof(addr));
 	else
-	  addr.addr4 = srv->addr;
+	  continue;
 	
 	header->ancount = htons(ntohs(header->ancount) + 1);
 	if (!add_resource_record(header, limit, &trunc, sizeof(struct dns_header), &p, daemon->local_ttl, NULL, T_A, C_IN, "4", &addr))
@@ -427,13 +435,14 @@ size_t make_local_answer(int flags, int gotname, size_t size, struct dns_header
   if (flags & gotname & F_IPV6)
     for (start = first; start != last; start++)
       {
-	struct serv_addr6 *srv = (struct serv_addr6 *)daemon->serverarray[start];
-
-	if (srv->flags & SERV_ALL_ZEROS)
+	struct serv_local *serv = daemon->serverarray[start];
+	if (serv->flags & SERV_6ADDR)
+	  addr.addr6 = ((struct serv_addr6 *)serv)->addr;
+	else if (serv->flags & SERV_ALL_ZEROS)
 	  memset(&addr, 0, sizeof(addr));
 	else
-	  addr.addr6 = srv->addr;
-	
+	  continue;
+
 	header->ancount = htons(ntohs(header->ancount) + 1);
 	add_resource_record(header, limit, &trunc, sizeof(struct dns_header), &p, daemon->local_ttl, NULL, T_AAAA, C_IN, "6", &addr);
 	log_query((flags | F_CONFIG | F_FORWARD) & ~F_IPV4, name, (union all_addr *)&addr, NULL, 0);
@@ -449,6 +458,7 @@ size_t make_local_answer(int flags, int gotname, size_t size, struct dns_header
 int dnssec_server(struct server *server, char *keyname, int *firstp, int *lastp)
 {
   int first, last, index;
+  struct server *first_serv;
 
   /* Find server to send DNSSEC query to. This will normally be the 
      same as for the original query, but may be another if
@@ -457,14 +467,18 @@ int dnssec_server(struct server *server, char *keyname, int *firstp, int *lastp)
     return -1;
 
   for (index = first; index != last; index++)
-    if (daemon->serverarray[index] == server)
+    if (daemon->serverarray[index] == (struct serv_local *)server)
       break;
 	      
   /* No match to server used for original query.
      Use newly looked up set. */
-  if (index == last)
-    index =  daemon->serverarray[first]->last_server == -1 ?
-      first : daemon->serverarray[first]->last_server;
+  // FIXME: How is ensured first is always !SERV_IS_LOCAL?
+  if (index == last &&
+      (first_serv=server_from_local(daemon->serverarray[first])) )
+    {
+      index =  first_serv->last_server == -1 ?
+	first : first_serv->last_server;
+    }
 
   if (firstp)
     *firstp = first;
@@ -477,7 +491,7 @@ int dnssec_server(struct server *server, char *keyname, int *firstp, int *lastp)
 #endif
 
 /* order by size, then by dictionary order */
-static int order(char *qdomain, size_t qlen, struct server *serv)
+static int order(char *qdomain, size_t qlen, struct serv_local *serv)
 {
   size_t dlen = 0;
     
@@ -497,7 +511,7 @@ static int order(char *qdomain, size_t qlen, struct server *serv)
   return strcmp(qdomain, serv->domain);
 }
 
-static int order_servers(struct server *s1, struct server *s2)
+static int order_servers(struct serv_local *s1, struct serv_local *s2)
 {
   int rc;
 
@@ -521,8 +535,8 @@ static int order_qsort(const void *a, const void *b)
 {
   int rc;
   
-  struct server *s1 = *((struct server **)a);
-  struct server *s2 = *((struct server **)b);
+  struct serv_local *s1 = *((struct serv_local **)a);
+  struct serv_local *s2 = *((struct serv_local **)b);
   
   rc = order_servers(s1, s2);
 
@@ -537,7 +551,7 @@ static int order_qsort(const void *a, const void *b)
   /* Finally, order by appearance in /etc/resolv.conf etc, for --strict-order */
   if (rc == 0)
     if (!(s1->flags & SERV_LITERAL_ADDRESS))
-      rc = s1->serial - s2->serial;
+      rc = ((struct server *)s1)->serial - ((struct server *)s2)->serial;
 
   return rc;
 }
@@ -545,6 +559,7 @@ static int order_qsort(const void *a, const void *b)
 void mark_servers(int flag)
 {
   struct server *serv;
+  struct serv_local *sl;
 
   /* mark everything with argument flag */
   for (serv = daemon->servers; serv; serv = serv->next)
@@ -553,16 +568,17 @@ void mark_servers(int flag)
     else
       serv->flags &= ~SERV_MARK;
 
-  for (serv = daemon->local_domains; serv; serv = serv->next)
-    if (serv->flags & flag)
-      serv->flags |= SERV_MARK;
+  for (sl = daemon->local_domains; sl; sl = sl->next)
+    if (sl->flags & flag)
+      sl->flags |= SERV_MARK;
     else
-      serv->flags &= ~SERV_MARK;
+      sl->flags &= ~SERV_MARK;
 }
 
 void cleanup_servers(void)
 {
   struct server *serv, *tmp, **up;
+  struct serv_local *sl, *tmpl, **upl;
 
   /* unlink and free anything still marked. */
   for (serv = daemon->servers, up = &daemon->servers; serv; serv = tmp) 
@@ -582,17 +598,17 @@ void cleanup_servers(void)
 	}
     }
   
- for (serv = daemon->local_domains, up = &daemon->local_domains; serv; serv = tmp) 
+ for (sl = daemon->local_domains, upl = &daemon->local_domains; sl; sl = tmpl)
    {
-     tmp = serv->next;
-      if (serv->flags & SERV_MARK)
+     tmpl = sl->next;
+      if (sl->flags & SERV_MARK)
        {
-	 *up = serv->next;
-	 free(serv->domain);
-	 free(serv);
+	 *upl = sl->next;
+	 free(sl->domain);
+	 free(sl);
        }
       else 
-	up = &serv->next;
+	upl = &sl->next;
    }
 }
 
@@ -604,6 +620,7 @@ int add_update_server(int flags,
 		      union all_addr *local_addr)
 {
   struct server *serv = NULL;
+  struct serv_local *sl = NULL;
   char *alloc_domain;
   
   if (!domain)
@@ -640,6 +657,7 @@ int add_update_server(int flags,
     {
       free(alloc_domain);
       alloc_domain = serv->domain;
+      sl = (struct serv_local *) serv;
     }
   else
     {
@@ -657,7 +675,7 @@ int add_update_server(int flags,
       else
 	size = sizeof(struct server);
       
-      if (!(serv = whine_malloc(size)))
+      if (!(sl = whine_malloc(size)))
 	{
 	  free(alloc_domain);
 	  return 0;
@@ -665,17 +683,18 @@ int add_update_server(int flags,
       
       if (flags & SERV_IS_LOCAL)
 	{
-	  serv->next = daemon->local_domains;
-	  daemon->local_domains = serv;
+	  sl->next = daemon->local_domains;
+	  daemon->local_domains = sl;
 
 	  if (flags & SERV_4ADDR)
-	    ((struct serv_addr4*)serv)->addr = local_addr->addr4;
+	    ((struct serv_addr4*)sl)->addr = local_addr->addr4;
 	  
 	  if (flags & SERV_6ADDR)
-	    ((struct serv_addr6*)serv)->addr = local_addr->addr6;
+	    ((struct serv_addr6*)sl)->addr = local_addr->addr6;
 	}
       else
 	{
+	  serv = (struct server *) sl;
 	  memset(serv, 0, sizeof(struct server));
 	  
 	  /* Add to the end of the chain, for order */
@@ -698,9 +717,9 @@ int add_update_server(int flags,
 	}
     }
   
-  serv->flags = flags;
-  serv->domain = alloc_domain;
-  serv->domain_len = strlen(alloc_domain);
+  sl->flags = flags;
+  sl->domain = alloc_domain;
+  sl->domain_len = strlen(alloc_domain);
   
   return 1;
 }
diff --git a/src/forward.c b/src/forward.c
index dd8a848..aa3dfea 100644
--- a/src/forward.c
+++ b/src/forward.c
@@ -148,9 +148,9 @@ static void server_send_log(struct server *server, int fd,
 }
 #endif
 
-static int domain_no_rebind(char *domain)
+static int domain_no_rebind(const char *domain)
 {
-  struct server *serv;
+  struct serv_local *serv;
   int dlen = (int)strlen(domain);
   
   for (serv = daemon->no_rebind; serv; serv = serv->next)
@@ -160,6 +160,13 @@ static int domain_no_rebind(char *domain)
   return 0;
 }
 
+static struct server *server_from_local(struct serv_local *sl)
+{
+  if (!(sl->flags & SERV_IS_LOCAL))
+    return (struct server *) sl;
+  return NULL;
+}
+
 static int forward_query(int udpfd, union mysockaddr *udpaddr,
 			 union all_addr *dst_addr, unsigned int dst_iface,
 			 struct dns_header *header, size_t plen,  char *limit, time_t now, 
@@ -276,9 +283,9 @@ static int forward_query(int udpfd, union mysockaddr *udpaddr,
       if (flags || ede == EDE_NOT_READY)
 	goto reply;
       
-      master = daemon->serverarray[first];
+      master = server_from_local(daemon->serverarray[first]);
       
-      if (!(forward = get_new_frec(now, master, 0)))
+      if (!master || !(forward = get_new_frec(now, master, 0)))
 	goto reply;
       /* table full - flags == 0, return REFUSED */
       
@@ -361,7 +368,9 @@ static int forward_query(int udpfd, union mysockaddr *udpaddr,
 	  if (!filter_servers(forward->sentto->arrayposn, F_SERVER, &first, &last))
 	    goto reply;
 	  
-	  master = daemon->serverarray[first];
+	  master = server_from_local(daemon->serverarray[first]);
+	  if (!master)
+	    goto reply;
 	  
 	  /* Forward to all available servers on retry of query from same host. */
 	  if (!option_bool(OPT_ORDER) && old_src)
@@ -443,12 +452,12 @@ static int forward_query(int udpfd, union mysockaddr *udpaddr,
      if we fail to send to all nameservers, send back an error
      packet straight away (helps modem users when offline)  */
 
-  while (1)
+  for (; start != last; ++start)
     { 
       int fd;
-      struct server *srv = daemon->serverarray[start];
+      struct server *srv = server_from_local(daemon->serverarray[start]);
       
-      if ((fd = allocate_rfd(&forward->rfds, srv)) != -1)
+      if (srv && (fd = allocate_rfd(&forward->rfds, srv)) != -1)
 	{
 	  
 #ifdef HAVE_CONNTRACK
@@ -510,9 +519,6 @@ static int forward_query(int udpfd, union mysockaddr *udpaddr,
 	      forward->forwardall++;
 	    }
 	}
-      
-      if (++start == last)
-	break;
     }
   
   if (forwarded || is_dnssec)
@@ -803,12 +809,13 @@ static void dnssec_validate(struct frec *forward, struct dns_header *header,
       struct frec *new = NULL;
       int serverind;
       struct blockdata *stash;
+      struct server *server;
       
       /* Now save reply pending receipt of key data */
       if ((serverind = dnssec_server(forward->sentto, daemon->keyname, NULL, NULL)) != -1 &&
-	  (stash = blockdata_alloc((char *)header, plen)))
+	  (stash = blockdata_alloc((char *)header, plen)) &&
+	  (server = server_from_local(daemon->serverarray[serverind])))
 	{
-	  struct server *server = daemon->serverarray[serverind];
 	  struct frec *orig;
 	  unsigned int flags;
 	  void *hash;
@@ -934,7 +941,7 @@ void reply_query(int fd, time_t now)
   struct frec *forward;
   socklen_t addrlen = sizeof(serveraddr);
   ssize_t n = recvfrom(fd, daemon->packet, daemon->packet_buff_sz, 0, &serveraddr.sa, &addrlen);
-  struct server *server;
+  struct server *server, *first_server;
   void *hash;
   int first, last, c;
     
@@ -959,18 +966,18 @@ void reply_query(int fd, time_t now)
      we may have sent the same query to multiple servers from
      the same local socket, and would like to know which one has answered. */
   for (c = first; c != last; c++)
-    if (sockaddr_isequal(&daemon->serverarray[c]->addr, &serveraddr))
+    if ((server = server_from_local(daemon->serverarray[c])) &&
+	sockaddr_isequal(&server->addr, &serveraddr))
       break;
   
   if (c == last)
     return;
 
-  server = daemon->serverarray[c];
-
+  first_server = server_from_local(daemon->serverarray[first]);
   if (RCODE(header) != REFUSED)
-    daemon->serverarray[first]->last_server = c;
-  else if (daemon->serverarray[first]->last_server == c)
-    daemon->serverarray[first]->last_server = -1;
+    first_server->last_server = c;
+  else if (first_server->last_server == c)
+    first_server->last_server = -1;
 
   /* If sufficient time has elapsed, try and expand UDP buffer size again. */
   if (difftime(now, server->pktsz_reduced) > UDP_TEST_TIME)
@@ -1675,13 +1682,17 @@ static ssize_t tcp_talk(int first, int last, int start, unsigned char *packet,
 	    break;
 	}
       
-      serv = daemon->serverarray[start];
+      serv = server_from_local(daemon->serverarray[start]);
+      if (!serv)
+	continue;
       
     retry:
       *length = htons(qsize);
       
       if (serv->tcpfd == -1)
 	{
+	  struct server *serv_first;
+
 	  if ((serv->tcpfd = socket(serv->addr.sa.sa_family, SOCK_STREAM, 0)) == -1)
 	    continue;
 	  
@@ -1712,7 +1723,9 @@ static ssize_t tcp_talk(int first, int last, int start, unsigned char *packet,
 	      continue;
 	    }
 	  
-	  daemon->serverarray[first]->last_server = start;
+	  serv_first = server_from_local(daemon->serverarray[first]);
+	  if (serv_first)
+	    serv_first->last_server = start;
 	  serv->flags &= ~SERV_GOT_TCP;
 	}
       
@@ -2050,9 +2063,9 @@ unsigned char *tcp_request(int confd, time_t now,
 		
 	      if (!flags && ede != EDE_NOT_READY)
 		{
-		  master = daemon->serverarray[first];
+		  master = server_from_local(daemon->serverarray[first]);
 		  
-		  if (option_bool(OPT_ORDER) || master->last_server == -1)
+		  if (option_bool(OPT_ORDER) || !master || master->last_server == -1)
 		    start = first;
 		  else
 		    start = master->last_server;
@@ -2518,8 +2531,8 @@ static struct frec *lookup_frec(unsigned short id, int fd, void *hash, int *firs
 	   have different bound sockets. */
 	for (first = *firstp, last = *lastp; first != last; first++)
 	  {
-	    s = daemon->serverarray[first];
-	    if (s->sfd && s->sfd->fd == fd)
+	    s = server_from_local(daemon->serverarray[first]);
+	    if (s && s->sfd && s->sfd->fd == fd)
 	      return f;
 	  }
       }
diff --git a/src/network.c b/src/network.c
index 3c1c176..a7ebd77 100644
--- a/src/network.c
+++ b/src/network.c
@@ -1498,6 +1498,7 @@ void check_servers(int no_loop_check)
 {
   struct irec *iface;
   struct server *serv;
+  struct serv_local *sl;
   struct serverfd *sfd, *tmp, **up;
   int port = 0, count;
   int locals = 0;
@@ -1620,21 +1621,21 @@ void check_servers(int no_loop_check)
 
     }
   
-  for (count = 0, serv = daemon->local_domains; serv; serv = serv->next)
+  for (count = 0, sl = daemon->local_domains; sl; sl = sl->next)
     {
        if (++count > SERVERS_LOGGED)
 	 continue;
        
-       if ((serv->flags & SERV_LITERAL_ADDRESS) &&
-	   !(serv->flags & (SERV_6ADDR | SERV_4ADDR | SERV_ALL_ZEROS)) &&
+       if ((sl->flags & SERV_LITERAL_ADDRESS) &&
+	   !(sl->flags & (SERV_6ADDR | SERV_4ADDR | SERV_ALL_ZEROS)) &&
 	   strlen(serv->domain))
 	 {
 	   count--;
 	   if (++locals <= LOCALS_LOGGED)
-	     my_syslog(LOG_INFO, _("using only locally-known addresses for %s"), serv->domain);
+	     my_syslog(LOG_INFO, _("using only locally-known addresses for %s"), sl->domain);
 	 }
-       else if (serv->flags & SERV_USE_RESOLV)
-	 my_syslog(LOG_INFO, _("using standard nameservers for %s"), serv->domain);
+       else if (sl->flags & SERV_USE_RESOLV)
+	 my_syslog(LOG_INFO, _("using standard nameservers for %s"), sl->domain);
     }
   
   if (locals > LOCALS_LOGGED)
diff --git a/src/option.c b/src/option.c
index 9a2db5f..a63cbe6 100644
--- a/src/option.c
+++ b/src/option.c
@@ -2715,7 +2715,7 @@ static int one_opt(int option, char *arg, char *errstr, char *gen_err, int comma
       
     case LOPT_NO_REBIND: /*  --rebind-domain-ok */
       {
-	struct server *new;
+	struct serv_local *new;
 
 	unhide_metas(arg);
 
@@ -2742,10 +2742,9 @@ static int one_opt(int option, char *arg, char *errstr, char *gen_err, int comma
       {
 	char *lastdomain = NULL, *domain = "";
 	u16 flags = 0;
-	char *err;
 	union all_addr addr;
 	union mysockaddr serv_addr, source_addr;
-	char interface[IF_NAMESIZE+1];
+	char interface[IF_NAMESIZE+1] = { 0, };
 
 	unhide_metas(arg);
 	
@@ -2779,6 +2778,7 @@ static int one_opt(int option, char *arg, char *errstr, char *gen_err, int comma
 	  }
 	else
 	  {
+	    char *err;
 	    if ((err = parse_server(arg, &serv_addr, &source_addr, interface, &flags)))
 	      ret_err(err);
 	  }
-- 
2.31.1

From 759c8b17ed66ffc658f73e00fb46f11655ec52df Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Petr=20Men=C5=A1=C3=ADk?= <pemen...@redhat.com>
Date: Thu, 23 Sep 2021 18:07:21 +0200
Subject: [PATCH 2/3] Ensure correct flag is set on rebind domain entry
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

It contains only limited information, therefore must include flag
indicating other members should not be analyzed.

Signed-off-by: Petr Menšík <pemen...@redhat.com>
---
 src/option.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/option.c b/src/option.c
index 54d89aa..9a2db5f 100644
--- a/src/option.c
+++ b/src/option.c
@@ -2725,6 +2725,7 @@ static int one_opt(int option, char *arg, char *errstr, char *gen_err, int comma
 	do {
 	  comma = split_chr(arg, '/');
 	  new = opt_malloc(sizeof(struct serv_local));
+	  new->flags = SERV_LITERAL_ADDRESS;
 	  new->domain = opt_string_alloc(arg);
 	  new->domain_len = strlen(arg);
 	  new->next = daemon->no_rebind;
-- 
2.31.1

From 34a34bba928a83b9de17ce7ab1b3e66b13801295 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Petr=20Men=C5=A1=C3=ADk?= <pemen...@redhat.com>
Date: Thu, 23 Sep 2021 13:37:13 +0200
Subject: [PATCH 1/3] Initialize send buffers before using them

Valgrind emits warnings about uninitialized bytes used in sendmsg().
Make sure all have defined content.
---
 src/forward.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/src/forward.c b/src/forward.c
index 786b11f..dd8a848 100644
--- a/src/forward.c
+++ b/src/forward.c
@@ -47,9 +47,8 @@ int send_from(int fd, int nowild, char *packet, size_t len,
   iov[0].iov_base = packet;
   iov[0].iov_len = len;
 
-  msg.msg_control = NULL;
-  msg.msg_controllen = 0;
-  msg.msg_flags = 0;
+  memset(&msg, 0, sizeof(msg));
+  memset(&control_u, 0, sizeof(control_u));
   msg.msg_name = to;
   msg.msg_namelen = sa_len(to);
   msg.msg_iov = iov;
-- 
2.31.1

_______________________________________________
Dnsmasq-discuss mailing list
Dnsmasq-discuss@lists.thekelleys.org.uk
https://lists.thekelleys.org.uk/cgi-bin/mailman/listinfo/dnsmasq-discuss

Reply via email to