Merge pull request #2232 from pi-hole/development

Pi-hole FTL v6.0.2

CMakeLists.txt

@@ -16,6 +16,6 @@ set(CMAKE_C_STANDARD 17)
 
 project(PIHOLE_FTL C)
 
-set(DNSMASQ_VERSION pi-hole-v2.91rc4)
+set(DNSMASQ_VERSION pi-hole-v2.91rc5)
 
 add_subdirectory(src)

src/config/dnsmasq_config.c

@@ -14,8 +14,6 @@
 #include "log.h"
 // get_blocking_mode_str()
 #include "datastructure.h"
-// flock(), LOCK_SH
-#include <sys/file.h>
 // struct config
 #include "config/config.h"
 // JSON array functions
@@ -116,11 +114,21 @@ static bool test_dnsmasq_config(char errbuf[ERRBUF_SIZE])
 			// We can ignore EINTR as it just means that the wait
 			// was interrupted, so we just try again. All other
 			// errors are fatal and we break out of the loop
-			if(err != EINTR)
+			if(err != EINTR && err != EAGAIN && err != ECHILD)
 			{
 				log_err("Cannot wait for dnsmasq test: %s", strerror(err));
 				break;
 			}
+
+			// Check if the child exited too quickly for waitpid to
+			// catch it. We cannot get the return code in this case
+			// and have to check the pipe content instead
+			if(errno == ECHILD)
+			{
+				log_debug(DEBUG_CONFIG, "dnsmasq test exited too quickly for waitpid");
+				code = strstr(errbuf, "syntax check OK") != NULL ? EXIT_SUCCESS : EXIT_FAILURE;
+				goto check_return;
+			}
 		}
 
 		// Get return code if child exited normally
@@ -136,11 +144,12 @@ static bool test_dnsmasq_config(char errbuf[ERRBUF_SIZE])
 			        WCOREDUMP(status) ? "(core dumped)" : "");
 		}
 
+check_return:
 		// Check if the error message contains a line number. If so, we
 		// can append the offending line to the error message
 		if(code != EXIT_SUCCESS)
 		{
-			int lineno = get_lineno_from_string(errbuf);
+			const int lineno = get_lineno_from_string(errbuf);
 			if(lineno > 0)
 			{
 				const size_t errbuf_size = strlen(errbuf);
@@ -314,12 +323,7 @@ bool __attribute__((const)) write_dnsmasq_config(struct config *conf, bool test_
 	}
 
 	// Lock file, may block if the file is currently opened
-	if(flock(fileno(pihole_conf), LOCK_EX) != 0)
-	{
-		log_err("Cannot open "DNSMASQ_TEMP_CONF" in exclusive mode: %s", strerror(errno));
-		fclose(pihole_conf);
-		return false;
-	}
+	const bool locked = lock_file(pihole_conf, DNSMASQ_TEMP_CONF);
 
 	write_config_header(pihole_conf, "Dnsmasq config for Pi-hole's FTLDNS");
 	fputs("hostsdir="DNSMASQ_HOSTSDIR"\n", pihole_conf);
@@ -500,9 +504,9 @@ bool __attribute__((const)) write_dnsmasq_config(struct config *conf, bool test_
 			continue;
 		}
 
-		if(active == NULL || cidr == NULL || target == NULL || domain == NULL)
+		if(active == NULL || cidr == NULL || target == NULL)
 		{
-			log_err("Skipped invalid dns.revServers[%u]: %s", i, revServer->valuestring);
+			log_err("Skipped invalid dns.revServers[%u]: %s (not fully defined)", i, revServer->valuestring);
 			free(copy);
 			continue;
 		}
@@ -513,7 +517,7 @@ bool __attribute__((const)) write_dnsmasq_config(struct config *conf, bool test_
 
 		// If we have a reverse domain, we forward all queries to this domain to
 		// the same destination
-		if(strlen(domain) > 0)
+		if(domain != NULL && strlen(domain) > 0)
 		{
 			fprintf(pihole_conf, "server=/%s/%s\n", domain, target);
 
@@ -757,12 +761,8 @@ bool __attribute__((const)) write_dnsmasq_config(struct config *conf, bool test_
 	fflush(pihole_conf);
 
 	// Unlock file
-	if(flock(fileno(pihole_conf), LOCK_UN) != 0)
-	{
-		log_err("Cannot release lock on dnsmasq config file: %s", strerror(errno));
-		fclose(pihole_conf);
-		return false;
-	}
+	if(locked)
+		unlock_file(pihole_conf, DNSMASQ_TEMP_CONF);
 
 	// Close file
 	if(fclose(pihole_conf) != 0)
@@ -1026,12 +1026,7 @@ bool write_custom_list(void)
 	}
 
 	// Lock file, may block if the file is currently opened
-	if(flock(fileno(custom_list), LOCK_EX) != 0)
-	{
-		log_err("Cannot open "DNSMASQ_CUSTOM_LIST_LEGACY".tmp in exclusive mode: %s", strerror(errno));
-		fclose(custom_list);
-		return false;
-	}
+	const bool locked = lock_file(custom_list, DNSMASQ_CUSTOM_LIST_LEGACY".tmp");
 
 	write_config_header(custom_list, "Custom DNS entries (HOSTS file)");
 	fputc('\n', custom_list);
@@ -1056,12 +1051,8 @@ bool write_custom_list(void)
 		fputs("\n# There are currently no entries in this file\n", custom_list);
 
 	// Unlock file
-	if(flock(fileno(custom_list), LOCK_UN) != 0)
-	{
-		log_err("Cannot release lock on custom.list: %s", strerror(errno));
-		fclose(custom_list);
-		return false;
-	}
+	if(locked)
+		unlock_file(custom_list, DNSMASQ_CUSTOM_LIST_LEGACY".tmp");
 
 	// Close file
 	if(fclose(custom_list) != 0)

src/config/env.c

@@ -633,8 +633,9 @@ cJSON *read_forced_vars(const unsigned int version)
 	cJSON *env_vars = cJSON_CreateArray();
 
 	// Try to open default config file. Use fallback if not found
-	FILE *fp;
-	if((fp = openFTLtoml("r", version)) == NULL)
+	bool locked = false;
+	FILE *fp = openFTLtoml("r", version, &locked);
+	if(fp == NULL)
 	{
 		// Return empty cJSON array
 		return env_vars;
@@ -672,7 +673,7 @@ cJSON *read_forced_vars(const unsigned int version)
 	}
 
 	// Close file and release exclusive lock
-	closeFTLtoml(fp);
+	closeFTLtoml(fp, locked);
 
 	// Return cJSON array
 	return env_vars;

src/config/toml_helper.c

@@ -13,8 +13,6 @@
 #include "config/config.h"
 // get_refresh_hostnames_str()
 #include "datastructure.h"
-// flock(), LOCK_SH
-#include <sys/file.h>
 // fcntl(), O_ACCMODE, O_RDONLY
 #include <fcntl.h>
 // rotate_files()
@@ -29,7 +27,7 @@
 #include "files.h"
 
 // Open the TOML file for reading or writing
-FILE * __attribute((malloc)) __attribute((nonnull(1))) openFTLtoml(const char *mode, const unsigned int version)
+FILE * __attribute((malloc)) __attribute((nonnull(1))) openFTLtoml(const char *mode, const unsigned int version, bool *locked)
 {
 	// This should not happen, install a safeguard anyway to unveil
 	// possible future coding issues early on
@@ -69,15 +67,7 @@ FILE * __attribute((malloc)) __attribute((nonnull(1))) openFTLtoml(const char *m
 	}
 
 	// Lock file, may block if the file is currently opened
-	if(flock(fileno(fp), LOCK_EX) != 0)
-	{
-		const int _e = errno;
-		log_err("Cannot open config file %s in exclusive mode (%s): %s",
-		        filename, mode, strerror(errno));
-		fclose(fp);
-		errno = _e;
-		return NULL;
-	}
+	*locked = lock_file(fp, filename);
 
 	// Log if we are using a backup file
 	if(version > 0)
@@ -88,14 +78,14 @@ FILE * __attribute((malloc)) __attribute((nonnull(1))) openFTLtoml(const char *m
 }
 
 // Open the TOML file for reading or writing
-void closeFTLtoml(FILE *fp)
+void closeFTLtoml(FILE *fp, const bool locked)
 {
 	// Release file lock
-	const int fn = fileno(fp);
-	if(flock(fn, LOCK_UN) != 0)
-		log_err("Cannot release lock on FTL's config file: %s", strerror(errno));
+	if(locked)
+		unlock_file(fp, NULL);
 
 	// Get access mode
+	const int fn = fileno(fp);
 	const int mode = fcntl(fn, F_GETFL);
 	if (mode == -1)
 		log_err("Cannot get access mode for FTL's config file: %s", strerror(errno));

src/config/toml_helper.h

@@ -17,8 +17,8 @@
 #include "tomlc99/toml.h"
 
 void indentTOML(FILE *fp, const unsigned int indent);
-FILE *openFTLtoml(const char *mode, const unsigned int version) __attribute((malloc)) __attribute((nonnull(1)));
-void closeFTLtoml(FILE *fp);
+FILE *openFTLtoml(const char *mode, const unsigned int version, bool *locked) __attribute((malloc)) __attribute((nonnull(1)));
+void closeFTLtoml(FILE *fp, const bool locked);
 void print_comment(FILE *fp, const char *str, const char *intro, const unsigned int width, const unsigned int indent);
 void print_toml_allowed_values(cJSON *allowed_values, FILE *fp, const unsigned int width, const unsigned int indent);
 void writeTOMLvalue(FILE * fp, const int indent, const enum conf_type t, union conf_value *v);

src/config/toml_reader.c

@@ -228,16 +228,17 @@ bool readFTLtoml(struct config *oldconf, struct config *newconf,
 static toml_table_t *parseTOML(const unsigned int version)
 {
 	// Try to open default config file. Use fallback if not found
-	FILE *fp;
-	if((fp = openFTLtoml("r", version)) == NULL)
+	bool locked = false;
+	FILE *fp = openFTLtoml("r", version, &locked);
+	if(fp == NULL)
 		return NULL;
 
 	// Parse lines in the config file
 	char errbuf[200];
 	toml_table_t *conf = toml_parse_file(fp, errbuf, sizeof(errbuf));
 
 	// Close file and release exclusive lock
-	closeFTLtoml(fp);
+	closeFTLtoml(fp, locked);
 
 	// Check for errors
 	if(conf == NULL)

src/config/toml_writer.c

@@ -41,8 +41,9 @@ bool writeFTLtoml(const bool verbose)
 	}
 
 	// Try to open a temporary config file for writing
-	FILE *fp;
-	if((fp = openFTLtoml("w", 0)) == NULL)
+	bool locked = false;
+	FILE *fp = openFTLtoml("w", 0, &locked);
+	if(fp == NULL)
 		return false;
 
 	// Write header
@@ -163,7 +164,7 @@ bool writeFTLtoml(const bool verbose)
 	cJSON_Delete(env_vars);
 
 	// Close file and release exclusive lock
-	closeFTLtoml(fp);
+	closeFTLtoml(fp, locked);
 
 	// Move temporary file to the final location if it is different
 	// We skip the first 8 lines as they contain the header and will always

src/dnsmasq/forward.c

@@ -301,8 +301,9 @@ static void forward_query(int udpfd, union mysockaddr *udpaddr,
 
 	      if (gobig && !bitvector)
 		{
-		  casediff = (i/BITS_IN_INT) + 1; /* length of array */
-		  if ((bitvector = whine_malloc(casediff)))
+		  casediff = ((i - 1)/BITS_IN_INT) + 1; /* length of array */
+		  /* whine_malloc() zeros memory */
+		  if ((bitvector = whine_malloc(casediff * sizeof(unsigned int))))
 		    goto big_redo;
 		}
 	    }
@@ -402,6 +403,7 @@ static void forward_query(int udpfd, union mysockaddr *udpaddr,
       header->id = ntohs(forward->new_id);
 
       forward->frec_src.encode_bitmap = option_bool(OPT_NO_0x20) ? 0 : rand32();
+      forward->frec_src.encode_bigmap = NULL;
       p = (unsigned char *)(header+1);
       if (!extract_name(header, plen, &p, (char *)&forward->frec_src.encode_bitmap, EXTR_NAME_FLIP, 1))
 	goto reply;
@@ -721,7 +723,6 @@ static size_t process_reply(struct dns_header *header, time_t now, struct server
   unsigned int rcode = RCODE(header);
   size_t plen; 
   /******** Pi-hole modification ********/
-  unsigned char *pheader_copy = NULL;
   unsigned char ede_data[MAX_EDE_DATA] = { 0 };
   size_t ede_len = 0;
   /**************************************/
@@ -874,16 +875,6 @@ static size_t process_reply(struct dns_header *header, time_t now, struct server
 	if(rc == 99)
 	    {
 	      cache_secure = 0;
-	      // Make a private copy of the pheader to ensure
-	      // we are not accidentially rewriting what is in
-	      // the pheader when we're creating a crafted reply
-	      // further below (when a query is to be blocked)
-	      if (pheader)
-	      {
-	       pheader_copy = calloc(1, plen);
-		memcpy(pheader_copy, pheader, plen);
-	      }
-
 	      // Generate DNS packet for reply, a possibly existing pseudo header
 	      // will be restored later inside resize_packet()
 	      n = FTL_make_answer(header, ((char *) header) + 65536, n, ede_data, &ede_len);
@@ -919,22 +910,17 @@ static size_t process_reply(struct dns_header *header, time_t now, struct server
 
   /* the code above can elide sections of the packet. Find the new length here 
      and put back pseudoheader if it was removed. */
-  n = resize_packet(header, n, pheader_copy ? pheader_copy : pheader, plen);
+  n = resize_packet(header, n, pheader, plen);
   /******** Pi-hole modification ********/
-  // The line above was modified to use
-  // pheader_copy instead of pheader
-  if(pheader_copy)
-    free(pheader_copy);
-
   if (pheader && (ede != EDE_UNSET || ede_len > 0))
     {
       if (ede_len > 0)
-	  n = add_pseudoheader(header, n, limit, EDNS0_OPTION_EDE, ede_data, ede_len, do_bit, 1);
-	else
-	  {
-	    u16 swap = htons((u16)ede);
-	    n = add_pseudoheader(header, n, limit, EDNS0_OPTION_EDE, (unsigned char *)&swap, 2, do_bit, 1);
-	  }
+	n = add_pseudoheader(header, n, limit, EDNS0_OPTION_EDE, ede_data, ede_len, do_bit, 1);
+      else
+        {
+	  u16 swap = htons((u16)ede);
+	  n = add_pseudoheader(header, n, limit, EDNS0_OPTION_EDE, (unsigned char *)&swap, 2, do_bit, 1);
+	}
     }
   /**************************************/
 
@@ -1100,6 +1086,7 @@ static void dnssec_validate(struct frec *forward, struct dns_header *header,
 		  new->flags |= flags;
 		  new->forwardall = 0;
 		  new->frec_src.encode_bitmap = 0;
+		  new->frec_src.encode_bigmap = NULL;
 
 		  forward->next_dependent = NULL;
 		  new->dependent = forward; /* to find query awaiting new one. */
@@ -1555,13 +1542,13 @@ void return_reply(time_t now, struct frec *forward, struct dns_header *header, s
 	  int first_ID = -1;
 
 	  /* This gets the name back to the state it was in when we started. */
-	  flip_queryname(header, nn, prev, &forward->frec_src);
+	  flip_queryname(header, new, prev, &forward->frec_src);
 
 	  for (src = &forward->frec_src, prev = NULL; src; prev = src, src = src->next)
 	    {
 	      /* If you didn't undertand this above, you won't understand it here either. */
 	      if (prev)
-		flip_queryname(header, nn, prev, src);
+		flip_queryname(header, new, prev, src);
 
 	      if (src->fd != -1 && nn > src->udp_pkt_size)
 		{
@@ -3200,7 +3187,7 @@ static void free_frec(struct frec *f)
   struct frec_src *last;
 
   /* add back to freelist if not the record builtin to every frec,
-     also free any bigmaps they's been decorated with. */
+     also free any bigmaps they've been decorated with. */
   for (last = f->frec_src.next; last && last->next; last = last->next)
     if (last->encode_bigmap)
       {
@@ -3210,6 +3197,12 @@ static void free_frec(struct frec *f)
 
   if (last)
     {
+      /* final link in the chain loses bigmap too. */
+      if (last->encode_bigmap)
+	{
+	  free(last->encode_bigmap);
+	  last->encode_bigmap = NULL;
+	}
       last->next = daemon->free_frec_src;
       daemon->free_frec_src = f->frec_src.next;
     }