Fix crash on SERVFAIL when --conntrack in use.

This is an optimisation, not a bugfix. Thanks to Jim Bos for spotting it.

CHANGELOG

@@ -1,3 +1,9 @@
+version 2.67
+	    Fix crash if upstream server returns SERVFAIL when
+	    --conntrack in use. Thanks to Giacomo Tazzari for finding
+	    this and supplying the patch. 
+	
+
 version 2.66
             Add the ability to act as an authoritative DNS
             server. Dnsmasq can now answer queries from the wider 'net

debian/changelog

@@ -1,3 +1,9 @@
+dnsmasq (2.67-1) unstable; urgency=low
+
+   * New upstream.
+
+ -- Simon Kelley <simon@thekelleys.org.uk>  Fri, 19 Apr 2013 10:23:31 +0000
+
 dnsmasq (2.66-1) unstable; urgency=low
 
    * New upstream.

src/dhcp-common.c

@@ -512,7 +512,7 @@ void display_opts6(void)
 }
 #endif
 
-u16 lookup_dhcp_opt(int prot, char *name)
+int lookup_dhcp_opt(int prot, char *name)
 {
   const struct opttab_t *t;
   int i;
@@ -528,10 +528,10 @@ u16 lookup_dhcp_opt(int prot, char *name)
     if (strcasecmp(t[i].name, name) == 0)
       return t[i].val;
   
-  return 0;
+  return -1;
 }
 
-u16 lookup_dhcp_len(int prot, u16 val)
+int lookup_dhcp_len(int prot, int val)
 {
   const struct opttab_t *t;
   int i;

src/dnsmasq.h

@@ -1216,8 +1216,8 @@ void log_tags(struct dhcp_netid *netid, u32 xid);
 int match_bytes(struct dhcp_opt *o, unsigned char *p, int len);
 void dhcp_update_configs(struct dhcp_config *configs);
 void display_opts(void);
-u16 lookup_dhcp_opt(int prot, char *name);
+int lookup_dhcp_opt(int prot, char *name);
-u16 lookup_dhcp_len(int prot, u16 val);
+int lookup_dhcp_len(int prot, int val);
 char *option_string(int prot, unsigned int opt, unsigned char *val, 
 		    int opt_len, char *buf, int buf_len);
 #ifdef HAVE_LINUX_NETWORK

src/forward.c

@@ -328,8 +328,8 @@ static int forward_query(int udpfd, union mysockaddr *udpaddr,
       struct server *firstsentto = start;
       int forwarded = 0;
       
-      if (udpaddr && option_bool(OPT_ADD_MAC))
+      if (option_bool(OPT_ADD_MAC))
-	plen = add_mac(header, plen, ((char *) header) + PACKETSZ, udpaddr);
+	plen = add_mac(header, plen, ((char *) header) + PACKETSZ, &forward->source);
       
       while (1)
 	{ 
@@ -372,7 +372,7 @@ static int forward_query(int udpfd, union mysockaddr *udpaddr,
 		  if (option_bool(OPT_CONNTRACK))
 		    {
 		      unsigned int mark;
-		      if (get_incoming_mark(udpaddr, dst_addr, 0, &mark))
+		      if (get_incoming_mark(&forward->source, &forward->dest, 0, &mark))
 			setsockopt(fd, SOL_SOCKET, SO_MARK, &mark, sizeof(unsigned int));
 		    }
 #endif
@@ -880,9 +880,12 @@ unsigned char *tcp_request(int confd, time_t now,
   unsigned short qtype;
   unsigned int gotname;
   unsigned char c1, c2;
-  /* Max TCP packet + slop */
+  /* Max TCP packet + slop + size */
-  unsigned char *packet = whine_malloc(65536 + MAXDNAME + RRFIXEDSZ);
+  unsigned char *packet = whine_malloc(65536 + MAXDNAME + RRFIXEDSZ + sizeof(u16));
-  struct dns_header *header;
+  unsigned char *payload = &packet[2];
+  /* largest field in header is 16-bits, so this is still sufficiently aligned */
+  struct dns_header *header = (struct dns_header *)payload;
+  u16 *length = (u16 *)packet;
   struct server *last_server;
   struct in_addr dst_addr_4;
   union mysockaddr peer_addr;
@@ -896,14 +899,12 @@ unsigned char *tcp_request(int confd, time_t now,
       if (!packet ||
 	  !read_write(confd, &c1, 1, 1) || !read_write(confd, &c2, 1, 1) ||
 	  !(size = c1 << 8 | c2) ||
-	  !read_write(confd, packet, size, 1))
+	  !read_write(confd, payload, size, 1))
        	return packet; 
   
       if (size < (int)sizeof(struct dns_header))
 	continue;
       
-      header = (struct dns_header *)packet;
-
       /* save state of "cd" flag in query */
       checking_disabled = header->hb4 & HB4_CD;
        
@@ -1020,12 +1021,9 @@ unsigned char *tcp_request(int confd, time_t now,
 #endif	
 			}
 		      
-		      c1 = size >> 8;
+		      *length = htons(size);
-		      c2 = size;
 		      
-		      if (!read_write(last_server->tcpfd, &c1, 1, 0) ||
+		      if (!read_write(last_server->tcpfd, packet, size + sizeof(u16), 0) ||
-			  !read_write(last_server->tcpfd, &c2, 1, 0) ||
-			  !read_write(last_server->tcpfd, packet, size, 0) ||
 			  !read_write(last_server->tcpfd, &c1, 1, 1) ||
 			  !read_write(last_server->tcpfd, &c2, 1, 1))
 			{
@@ -1035,7 +1033,7 @@ unsigned char *tcp_request(int confd, time_t now,
 			} 
 		      
 		      m = (c1 << 8) | c2;
-		      if (!read_write(last_server->tcpfd, packet, m, 1))
+		      if (!read_write(last_server->tcpfd, payload, m, 1))
 			return packet;
 		      
 		      if (!gotname)
@@ -1071,12 +1069,9 @@ unsigned char *tcp_request(int confd, time_t now,
 	  
       check_log_writer(NULL);
       
-      c1 = m>>8;
+      *length = htons(m);
-      c2 = m;
+           
-      if (m == 0 ||
+      if (m == 0 || !read_write(confd, packet, m + sizeof(u16), 0))
-	  !read_write(confd, &c1, 1, 0) ||
-	  !read_write(confd, &c2, 1, 0) || 
-	  !read_write(confd, packet, m, 0))
 	return packet;
     }
 }

src/ipset.c

@@ -110,7 +110,7 @@ static int new_add_to_ipset(const char *setname, const struct all_addr *ipaddr,
       return -1;
     }
   
-  memset(buffer, 0, sizeof(buffer));
+  memset(buffer, 0, BUFF_SZ);
 
   nlh = (struct nlmsghdr *)buffer;
   nlh->nlmsg_len = NL_ALIGN(sizeof(struct nlmsghdr));

src/option.c

@@ -750,6 +750,7 @@ static int parse_dhcp_opt(char *errstr, char *arg, int flags)
   struct dhcp_netid *np = NULL;
   u16 opt_len = 0;
   int is6 = 0;
+  int option_ok = 0;
 
   new->len = 0;
   new->flags = flags;
@@ -769,16 +770,19 @@ static int parse_dhcp_opt(char *errstr, char *arg, int flags)
 	{
 	  new->opt = atoi(arg);
 	  opt_len = 0;
+	  option_ok = 1;
 	  break;
 	}
       
       if (strstr(arg, "option:") == arg)
 	{
-	  new->opt = lookup_dhcp_opt(AF_INET, arg+7);
+	  if ((new->opt = lookup_dhcp_opt(AF_INET, arg+7)) != -1)
+	    {
 	      opt_len = lookup_dhcp_len(AF_INET, new->opt);
 	      /* option:<optname> must follow tag and vendor string. */
-	  if ((opt_len & OT_INTERNAL) && flags != DHOPT_MATCH)
+	      if (!(opt_len & OT_INTERNAL) || flags == DHOPT_MATCH)
-	    new->opt = 0;
+		option_ok = 1;
+	    }
 	  break;
 	}
 #ifdef HAVE_DHCP6
@@ -792,13 +796,16 @@ static int parse_dhcp_opt(char *errstr, char *arg, int flags)
 	    {
 	      new->opt = atoi(arg+8);
 	      opt_len = 0;
+	      option_ok = 1;
 	    }
 	  else
 	    {
-	      new->opt = lookup_dhcp_opt(AF_INET6, arg+8);
+	      if ((new->opt = lookup_dhcp_opt(AF_INET6, arg+8)) != -1)
+		{
 		  opt_len = lookup_dhcp_len(AF_INET6, new->opt);
-	      if ((opt_len & OT_INTERNAL) && flags != DHOPT_MATCH)
+		  if (!(opt_len & OT_INTERNAL) || flags == DHOPT_MATCH)
-		new->opt = 0;
+		    option_ok = 1;
+		}
 	    }
 	  /* option6:<opt>|<optname> must follow tag and vendor string. */
 	  is6 = 1;
@@ -821,7 +828,7 @@ static int parse_dhcp_opt(char *errstr, char *arg, int flags)
 	  new->flags |= DHOPT_RFC3925;
 	  if (flags == DHOPT_MATCH)
 	    {
-	      new->opt = 1; /* avoid error below */
+	      option_ok = 1;
 	      break;
 	    }
 	}
@@ -857,7 +864,7 @@ static int parse_dhcp_opt(char *errstr, char *arg, int flags)
       opt_len = lookup_dhcp_len(AF_INET, new->opt);
   
   /* option may be missing with rfc3925 match */
-  if (new->opt == 0)
+  if (!option_ok)
     ret_err(_("bad dhcp-option"));
   
   if (comma)