diff --git a/CMakeLists.txt b/CMakeLists.txt index 7b48373cc..4d1106700 100644 --- a/CMakeLists.txt +++ b/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) diff --git a/src/config/dnsmasq_config.c b/src/config/dnsmasq_config.c index 589954d47..48ec74b73 100644 --- a/src/config/dnsmasq_config.c +++ b/src/config/dnsmasq_config.c @@ -14,8 +14,6 @@ #include "log.h" // get_blocking_mode_str() #include "datastructure.h" -// flock(), LOCK_SH -#include // 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) diff --git a/src/config/env.c b/src/config/env.c index 908099e00..b22e05803 100644 --- a/src/config/env.c +++ b/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; diff --git a/src/config/toml_helper.c b/src/config/toml_helper.c index 27aefdb99..8eed6f928 100644 --- a/src/config/toml_helper.c +++ b/src/config/toml_helper.c @@ -13,8 +13,6 @@ #include "config/config.h" // get_refresh_hostnames_str() #include "datastructure.h" -// flock(), LOCK_SH -#include // fcntl(), O_ACCMODE, O_RDONLY #include // 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)); diff --git a/src/config/toml_helper.h b/src/config/toml_helper.h index 70f4844c6..69227f1f4 100644 --- a/src/config/toml_helper.h +++ b/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); diff --git a/src/config/toml_reader.c b/src/config/toml_reader.c index 6f3f98e36..a42b65e8d 100644 --- a/src/config/toml_reader.c +++ b/src/config/toml_reader.c @@ -228,8 +228,9 @@ 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 @@ -237,7 +238,7 @@ static toml_table_t *parseTOML(const unsigned int version) 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) diff --git a/src/config/toml_writer.c b/src/config/toml_writer.c index 4eab59847..8d3ed4078 100644 --- a/src/config/toml_writer.c +++ b/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 diff --git a/src/dnsmasq/forward.c b/src/dnsmasq/forward.c index 9ac67621e..c30314f28 100644 --- a/src/dnsmasq/forward.c +++ b/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; } diff --git a/src/files.c b/src/files.c index 7489dc77f..c482bfd75 100644 --- a/src/files.c +++ b/src/files.c @@ -35,6 +35,8 @@ #include //basename() #include +// flock(), LOCK_SH +#include // chmod_file() changes the file mode bits of a given file (relative // to the directory file descriptor) according to mode. mode is an @@ -863,3 +865,75 @@ enum verify_result verify_FTL(bool verbose) return success ? VERIFY_OK : VERIFY_FAILED; } +/** + * @brief Locks a file for exclusive access. + * + * This function attempts to acquire an exclusive lock on the file + * associated with the given file pointer. + * + * @param fp A pointer to the FILE object representing the file to be locked. + * @param filename The name of the file to be locked, used for logging purposes. + * @return true if the lock is successfully acquired, false otherwise. + */ + +bool lock_file(FILE *fp, const char *filename) +{ + const int fd = fileno(fp); + if(fd < 0) + { + log_warn("Failed to get file descriptor for \"%s\": %s", + filename, strerror(errno)); + return false; + } + + if(flock(fd, LOCK_EX) != 0) + { + // Backup errno + const int _e = errno; + log_warn("Cannot get exclusive lock for %s: %s", + filename, strerror(errno)); + + // Restore errno + errno = _e; + return false; + } + + return true; +} + +/** + * @brief Unlocks a file by releasing the lock on the file descriptor. + * + * This function attempts to release the lock on the file associated with the + * given file pointer. If the file descriptor cannot be obtained or the lock + * cannot be released, an error message is logged and the function returns false. + * + * @param fp A pointer to the FILE object representing the file to unlock. + * @param filename A string representing the name of the file. This is used for + * logging purposes. If NULL, "" is used in the log message. + * @return true if the lock was successfully released, false otherwise. + */ +bool unlock_file(FILE *fp, const char *filename) +{ + const int fd = fileno(fp); + if(fd < 0) + { + log_warn("Failed to get file descriptor for \"%s\": %s", + filename ? filename : "", strerror(errno)); + return false; + } + + if(flock(fd, LOCK_UN) != 0) + { + // Backup errno + const int _e = errno; + log_warn("Cannot release lock for %s: %s", + filename ? filename : "", strerror(errno)); + + // Restore errno + errno = _e; + return false; + } + + return true; +} diff --git a/src/files.h b/src/files.h index cbdf4b891..4b13d44c4 100644 --- a/src/files.h +++ b/src/files.h @@ -43,4 +43,7 @@ int parse_line(char *line, char **key, char **value); char *get_hwmon_target(const char *path) __attribute__((malloc)); +bool lock_file(FILE *fp, const char *filename); +bool unlock_file(FILE *fp, const char *filename); + #endif //FILE_H