Return an empty block from blockdata_alloc() for zero length.

738c049 turned the allocation loop in blockdata_alloc_real() from
do/while into for, so a zero length now returns NULL. find_soa() and
extract_addresses() start the chain for an RR with
blockdata_alloc(NULL, 0) and fill it with blockdata_expand(), so they
now fail on every SOA and every RR that is not an address, e.g., MX,
SRV, or SVCB, and those answers are no longer cached. A zero-length
RR also no longer survives cache_recv_insert().

Allocate one empty block for zero length, as before.

Signed-off-by: DL6ER <dl6er@dl6er.de>

src/blockdata.c

@@ -16,7 +16,9 @@
 
 #include "dnsmasq.h"
 
-static struct blockdata *keyblock_free;
+static struct blockdata *new_block(void);
+
+static struct blockdata *keyblock_free, *fallback;
 static unsigned int blockdata_count, blockdata_hwm, blockdata_alloced;
 
 static void add_blocks(int n)
@@ -48,7 +50,11 @@ void blockdata_init(void)
   /* Note that daemon->cachesize is enforced to have non-zero size if OPT_DNSSEC_VALID is set */  
   if (option_bool(OPT_DNSSEC_VALID))
     add_blocks(daemon->cachesize);
-}
+
+  /* make sure we have one block to fall back on. */
+  if (!(fallback = new_block()))
+    die(_("Blockdata init: no memory"), NULL, EC_MISC);
+ }
 
 void blockdata_report(void)
 {
@@ -84,36 +90,52 @@ static struct blockdata *blockdata_alloc_real(int fd, char *data, size_t len)
   struct blockdata *block, *ret = NULL;
   struct blockdata **prev = &ret;
   size_t blen;
+  int fail;
 
-  do
+  /* Zero length still gives one empty block, blockdata_alloc(NULL, 0)
-    {
+     is how callers start a chain to add to with blockdata_expand(). */
-      if (!(block = new_block()))
+  if (len == 0)
-	{
+    return new_block();
-	  /* failed to alloc, free partial chain */
-	  blockdata_free(ret);
-	  return NULL;
-	}
-
-      if ((blen = len > KEYBLOCK_LEN ? KEYBLOCK_LEN : len) > 0)
-	{
-	  if (data)
-	    {
-	      memcpy(block->key, data, blen);
-	      data += blen;
-	    }
-	  else if (!read_write(fd, block->key, blen, RW_READ))
-	    {
-	      /* failed read free partial chain */
-	      blockdata_free(ret);
-	      return NULL;
-	    }
-	}
-      
-      len -= blen;
-      *prev = block;
-      prev = &block->next;
-    } while (len != 0);
   
+  for (fail = 0; len != 0; len -= blen)
+    {
+       blen = len > KEYBLOCK_LEN ? KEYBLOCK_LEN : len;
+
+       if (!fail)
+	{
+	  if ((block = new_block()))
+	    {
+	      *prev = block;
+	      prev = &block->next;
+	    }
+	  else
+	    {
+	      /* Failed to alloc, use the pre-allocated fall-back. 
+		 This allows us to always read the specified amount of data from the fd. */
+	      fail = 1;
+	      block = fallback;
+	    }
+	}
+
+       if (data)
+	 {
+	   memcpy(block->key, data, blen);
+	   data += blen;
+	 }
+       else if (!read_write(fd, block->key, blen, RW_READ))
+	 {
+	   fail = 1;
+	   break;
+	 }
+    }
+  
+  if (fail)
+    {
+      /* failed, free partial chain */
+      blockdata_free(ret);
+      return NULL;
+    }
+
   return ret;
 }
 
@@ -235,6 +257,13 @@ void blockdata_write(struct blockdata *block, size_t len, int fd)
     }
 }
 
+/* NOTE: this can fail (NULL return) for two possible reasons.
+   1) end of stream. 
+   2) malloc failure when allocating blocks.
+
+   Unless EOF is encountered, len octets will always be read from the stream,
+   even in memory allocation fails.
+*/
 struct blockdata *blockdata_read(int fd, size_t len)
 {
   return blockdata_alloc_real(fd, NULL, len);

src/cache.c

@@ -1039,9 +1039,18 @@ int cache_recv_insert(time_t now, int fd)
 		unsigned short class = C_IN;
 		struct blockdata *block = NULL;
 
-		if ((flags & F_RR) && !(flags & F_NEG) && (flags & F_KEYTAG)
+		/* Note that blockdata_read() can return NULL either
-		    && !(block = addr.rrblock.rrdata = blockdata_read(fd, addr.rrblock.datalen)))
+		   because the stream ended, OR memory allocation failed.
-		  continue;
+		   We continue here rather than returning 0 to finish draining 
+		   the pipe in the second case. If the stream ended, the read
+		   of the next length at the top of the loop will fail
+		   and we'll return 0 there. */
+		if (flags & F_RR)
+		  {
+		    if (!(flags & F_NEG) && (flags & F_KEYTAG) &&
+			!(block = addr.rrblock.rrdata = blockdata_read(fd, addr.rrblock.datalen)))
+		      continue;
+		  }
 #ifdef HAVE_DNSSEC
 		else if (flags & F_DNSKEY)
 		  {

src/dnsmasq.c

@@ -140,6 +140,9 @@ int main (int argc, char **argv)
       /* one char flag per possible RR in answer section (may get extended). */
       daemon->rr_status_sz = 64;
       daemon->rr_status = safe_malloc(sizeof(*daemon->rr_status) * daemon->rr_status_sz);
+      /* Increase TCP process slots by half, and reserve the extra for validation-to-tcp use only. */
+      daemon->dnssec_tcp_slot_reserved = daemon->max_procs/2;
+      daemon->max_procs += daemon->dnssec_tcp_slot_reserved;
     }
 #endif
   
@@ -427,7 +430,8 @@ int main (int argc, char **argv)
 
       daemon->tcp_pids = safe_malloc(daemon->max_procs*sizeof(pid_t));
       daemon->tcp_pipes = safe_malloc(daemon->max_procs*sizeof(int));
-
+      daemon->tcp_type = safe_malloc(daemon->max_procs);
+      
       for (i = 0; i < daemon->max_procs; i++)
 	daemon->tcp_pipes[i] = -1;
     }
@@ -1652,7 +1656,11 @@ static void async_event(int pipe, time_t now)
 		  
 		  /* tcp_pipes == -1 && tcp_pids == 0 required to free slot */
 		  if (daemon->tcp_pipes[i] == -1)
-		    daemon->metrics[METRIC_TCP_CONNECTIONS]--;
+		    {
+		      if (daemon->tcp_type[i])
+			 daemon->dnssec_tcp_slot_reserved++;
+		      daemon->metrics[METRIC_TCP_CONNECTIONS]--;
+		    }
 		}
 	break;
 	
@@ -1888,7 +1896,7 @@ static void set_dns_listeners(void)
   struct serverfd *serverfdp;
   struct listener *listener;
   struct randfd_list *rfl;
-  int i;
+  int i, j;
   
   for (serverfdp = daemon->sfds; serverfdp; serverfdp = serverfdp->next)
     poll_listen(serverfdp->fd, POLLIN);
@@ -1902,8 +1910,8 @@ static void set_dns_listeners(void)
     poll_listen(rfl->rfd->fd, POLLIN);
   
   /* check to see if we have free tcp process slots. */
-  for (i = daemon->max_procs - 1; i >= 0; i--)
+  for (j = 0, i = daemon->max_procs - 1; i >= 0; i--)
-    if (daemon->tcp_pids[i] == 0 && daemon->tcp_pipes[i] == -1)
+    if (daemon->tcp_pids[i] == 0 && daemon->tcp_pipes[i] == -1 && ++j > daemon->dnssec_tcp_slot_reserved)
       break;
 
   for (listener = daemon->listeners; listener; listener = listener->next)
@@ -1930,7 +1938,7 @@ static void check_dns_listeners(time_t now)
   struct serverfd *serverfdp;
   struct listener *listener;
   struct randfd_list *rfl;
-  int i;
+  int i, j;
   
   /* Note that handling events here can create or destroy fds and
      render the result of the last poll() call invalid. Once
@@ -1955,7 +1963,11 @@ static void check_dns_listeners(time_t now)
 	      daemon->tcp_pipes[i] = -1;	
 	      /* tcp_pipes == -1 && tcp_pids == 0 required to free slot */
 	      if (daemon->tcp_pids[i] == 0)
-		daemon->metrics[METRIC_TCP_CONNECTIONS]--;
+		{
+		  daemon->metrics[METRIC_TCP_CONNECTIONS]--;
+		  if (daemon->tcp_type[i])
+		    daemon->dnssec_tcp_slot_reserved++;
+		}
 	    }
 	  return;
 	}
@@ -1995,8 +2007,8 @@ static void check_dns_listeners(time_t now)
      at least one a poll() time, that we still do.
      There may be more waiting connections after
      poll() returns then free process slots. */
-  for (i = daemon->max_procs - 1; i >= 0; i--)
+  for (j = 0,i = daemon->max_procs - 1; i >= 0; i--)
-    if (daemon->tcp_pids[i] == 0 && daemon->tcp_pipes[i] == -1)
+    if (daemon->tcp_pids[i] == 0 && daemon->tcp_pipes[i] == -1 && ++j > daemon->dnssec_tcp_slot_reserved)
       break;
 
   if (i >= 0)
@@ -2165,6 +2177,7 @@ static void do_tcp_connection(struct listener *listener, time_t now, int slot)
 	  
 	  daemon->tcp_pids[slot] = p;
 	  daemon->tcp_pipes[slot] = pipefd[0];
+	  daemon->tcp_type[slot] = 0;
 	  daemon->metrics[METRIC_TCP_CONNECTIONS]++;
 	  if (daemon->metrics[METRIC_TCP_CONNECTIONS] > daemon->max_procs_used)
 	    daemon->max_procs_used = daemon->metrics[METRIC_TCP_CONNECTIONS];
@@ -2306,6 +2319,8 @@ int swap_to_tcp(struct frec *forward, time_t now, int status, struct dns_header
 	  /* i holds index of free slot */
 	  daemon->tcp_pids[i] = p;
 	  daemon->tcp_pipes[i] = pipefd[0];
+	  daemon->tcp_type[i] = 1;
+	  daemon->dnssec_tcp_slot_reserved--;
 	  daemon->metrics[METRIC_TCP_CONNECTIONS]++;
 	  if (daemon->metrics[METRIC_TCP_CONNECTIONS] > daemon->max_procs_used)
 	    daemon->max_procs_used = daemon->metrics[METRIC_TCP_CONNECTIONS];

src/dnsmasq.h

@@ -1296,6 +1296,7 @@ extern struct daemon {
   int    fd_save;          /*      "        "        */
   pid_t *tcp_pids;
   int *tcp_pipes;
+  char *tcp_type;
   int pipe_to_parent;
   int numrrand;
   struct randfd *randomsocks;
@@ -1355,7 +1356,7 @@ extern struct daemon {
   /* file for packet dumps. */
   int dumpfd;
 #endif
-  int max_procs;
+  int max_procs, dnssec_tcp_slot_reserved;
   uint max_procs_used;
 } *daemon;
 

src/dnssec.c

@@ -486,7 +486,6 @@ static int validate_rrset(time_t now, struct dns_header *header, size_t plen, in
       int i, wire_len, sig_len;
       const struct nettle_hash *hash;
       void *ctx;
-      char *name_start;
       u32 nsigttl, ttl, orig_ttl;
 
       failflags &= ~DNSSEC_FAIL_NOSIG;
@@ -576,13 +575,13 @@ static int validate_rrset(time_t now, struct dns_header *header, size_t plen, in
 	  if (!extract_name(header, plen, &p, name, EXTR_NAME_EXTRACT, 10)) 
 	    return STAT_BOGUS;
 
-	  name_start = name;
-	  
 	  /* if more labels than in RRsig name, hash *.<no labels in rrsig labels field>  4035 5.3.2 */
 	  /* If the name is already the wildcard, we're not going to change it. */
 	  if (labels < name_labels &&
-	      !(name_labels - labels == 1 && name_start[0] == '*' && name_start[1] == '.'))
+	      !(name_labels - labels == 1 && name[0] == '*' && name[1] == '.'))
 	    {
+	      char *name_start = name;
+
 	      for (j = name_labels - labels; j != 0; j--)
 		{
 		  while (*name_start != '.' && *name_start != 0)
@@ -595,11 +594,13 @@ static int validate_rrset(time_t now, struct dns_header *header, size_t plen, in
 		*wildcard_offset_out = name_start - name + 1;
 	      
 	      name_start--;
-	      *name_start = '*';	      
+	      *name_start = '*';
+	      /* move our name to the start of the buffer to avoid pathalogical cases over-running the end in to_wire(). */
+	      memmove(name, name_start, strlen(name_start) + 1);	      
 	    }
-	  
+
-	  wire_len = to_wire(name_start);
+	  wire_len = to_wire(name);
-	  hash->update(ctx, (unsigned int)wire_len, (unsigned char *)name_start);
+	  hash->update(ctx, (unsigned int)wire_len, (unsigned char *)name);
 	  hash->update(ctx, 4, p); /* class and type */
 	  hash->update(ctx, 4, (unsigned char *)&nsigttl);
 
@@ -1643,7 +1644,7 @@ static int prove_non_existence_nsec3(struct dns_header *header, size_t plen, uns
       if (*closest_encloser == '.')
 	closest_encloser++;
 
-      if (wild_offset != 0 && name - closest_encloser == wild_offset)
+      if (wild_offset != 0 && closest_encloser - name == wild_offset)
 	break;
       
       if (dec_counter(validate_counter, NULL))
@@ -1694,11 +1695,13 @@ static int prove_non_existence_nsec3(struct dns_header *header, size_t plen, uns
       
       wildcard--;
       *wildcard = '*';
+      /* move our name to the start of the buffer to avoid pathalogical cases over-running the end in to_wire(). */
+      memmove(name, wildcard, strlen(wildcard) + 1);
       
       if (dec_counter(validate_counter, NULL))
 	return DNSSEC_FAIL_WORK;
       
-      if ((digest_len = hash_name(wildcard, &digest, hash, salt, salt_len, iterations)) == 0)
+      if ((digest_len = hash_name(name, &digest, hash, salt, salt_len, iterations)) == 0)
 	return DNSSEC_FAIL_NONSEC;
       
       if (!check_nsec3_coverage(header, plen, digest_len, digest, type, workspace1, workspace2, nsecs, nsec_count, NULL, 1))

src/forward.c

@@ -2406,7 +2406,7 @@ void tcp_request(int confd, time_t now, struct iovec *bigbuff,
   unsigned char *pheader;
   unsigned int mark = 0;
   int have_mark = 0;
-  int first, last, filtered, do_stale = 0;
+  int first, last, filtered, do_stale = 0, closed = 0;
   struct iovec out_iov[2];
   
   bigbuff->iov_base = NULL;
@@ -2742,8 +2742,16 @@ void tcp_request(int confd, time_t now, struct iovec *bigbuff,
 	    }
 	}
 
+      /* Fresh data is in the cache now. Go on with the queries the client
+	 had already sent, if the connection is still open. */
       if (do_stale)
-	break;
+	{
+	  if (closed)
+	    break;
+	  do_stale = 0;
+	  daemon->log_source_addr = &peer_addr;
+	  continue;
+	}
       
       /* In case of local answer or no connections made. */
       if (m == 0)
@@ -2792,19 +2800,27 @@ void tcp_request(int confd, time_t now, struct iovec *bigbuff,
       /* If we answered with stale data, this process will now try and get fresh data into
 	 the cache and cannot therefore accept new queries. Close the incoming
 	 connection to signal that to the client. Then set do_stale and loop round
-	 once more to try and get fresh data, after which we exit. */
+	 once more to try and get fresh data, after which we exit. A client which
+	 has already sent its next query is waiting for the answer and would lose
+	 it, so keep the connection open then and answer it after the refresh. */
       if (stale)
 	{
-	  shutdown(confd, SHUT_RDWR);
+	  char c;
-	  close(confd);
+	  
+	  if (recv(confd, &c, 1, MSG_PEEK | MSG_DONTWAIT) != 1)
+	    {
+	      shutdown(confd, SHUT_RDWR);
+	      close(confd);
+	      closed = 1;
+	    }
 	  do_stale = 1;
 	  /* Don't mark the query with the source when refreshing stale data. */
 	  daemon->log_source_addr = NULL;
 	}
     }
   
-/* If we ran once to get fresh data, confd is already closed. */
+/* If we ran once to get fresh data, confd may be closed already. */
-  if (!do_stale)
+  if (!closed)
     {
       shutdown(confd, SHUT_RDWR);
       close(confd);

src/option.c

@@ -5046,7 +5046,7 @@ err:
 	    new->target = target;
 	    new->ttl = ttl;
 
-	    for (arg += arglen+1; *arg && isspace((unsigned char)*arg); arg++);
+	    for (arg += arglen+1; arg != last && (!*arg || isspace((unsigned char)*arg)); arg++);
 	  }
       
 	break;

src/rfc1035.c

@@ -560,7 +560,7 @@ static int find_soa(struct dns_header *header, size_t qlen, char *name, int *sub
 	  soa_len = strlen(daemon->workspacename);
 
 	  /* SOA must be for the name we're interested in. */
-	  if (soa_len <= name_len && memcmp(daemon->workspacename, name + name_len - soa_len, soa_len) == 0)
+	  if (hostname_issubdomain(daemon->workspacename, name) != 0)
 	    {
 	      int prefix = name_len - soa_len;
 	      

src/rfc3315.c

@@ -502,7 +502,7 @@ static int dhcp6_no_relay(struct state *state, int msg_type, unsigned char *inbu
  
        state->fqdn_flags &= ~0x04;
 
-       if (len != 0 && len < 255)
+       if (len >= 1 && len < 255)
 	 {
 	   unsigned char *pp, *op = opt6_ptr(opt, 1);
 	   char *pq = daemon->dhcp_buff;

src/tftp.c

@@ -545,10 +545,17 @@ static struct tftp_file *check_tftp_fileperm(char *packet, ssize_t *len, char *p
   /* trick to ban moving out of the subtree */
   if (prefix)
     {
-      char *suspect = strstr(namebuff, "/..");
+      char *suspect = namebuff;
 
-      if (suspect && (suspect[3] == '/' || suspect[3] == 0))
+      /* Every occurrence has to be checked: a harmless leading match
-        goto perm;
+         such as "/..data" otherwise hides a later "/../" from here. */
+      while ((suspect = strstr(suspect, "/..")))
+        {
+          if (suspect[3] == '/' || suspect[3] == 0)
+            goto perm;
+
+          suspect += 3;
+        }
     }
   
   if ((fd = open(namebuff, O_RDONLY)) == -1)
@@ -836,6 +843,12 @@ static void free_transfer(struct tftp_transfer *transfer)
       close(transfer->file->fd);
       free(transfer->file);
     }
+
+  /* freeing this transfer and then allocating a new one
+     may result in the same memory, but the contents of
+     the transfer is no longer valid to cache. */
+  if (daemon->srv_save == transfer)
+    daemon->srv_save = NULL;
   
   free(transfer);
 }