diff --git a/build.sh b/build.sh index bebdc636..15d09587 100755 --- a/build.sh +++ b/build.sh @@ -181,7 +181,6 @@ fi # If we are asked to run tests, we do this here if [[ -n "${test}" ]]; then cd .. - bash test/arch_test.sh bash test/run.sh fi diff --git a/src/api/config.c b/src/api/config.c index f279f469..9e0dc4e7 100644 --- a/src/api/config.c +++ b/src/api/config.c @@ -718,7 +718,7 @@ static int api_config_patch(struct ftl_conn *api) if(new_item->f & FLAG_READ_ONLY && cJSON_IsBool(elem) && elem->valueint == 1) { char *key = strdup(new_item->k); - free_config(&newconf); + free_config(&newconf, false); return send_json_error_free(api, 400, "bad_request", "This config option can only be set in pihole.toml, not via the API", @@ -740,7 +740,7 @@ static int api_config_patch(struct ftl_conn *api) char *hint = calloc(strlen(new_item->k) + strlen(response) + 3, sizeof(char)); if(hint == NULL) { - free_config(&newconf); + free_config(&newconf, false); return send_json_error(api, 500, "internal_error", "Failed to allocate memory for hint", @@ -749,7 +749,7 @@ static int api_config_patch(struct ftl_conn *api) strcpy(hint, new_item->k); strcat(hint, ": "); strcat(hint, response); - free_config(&newconf); + free_config(&newconf, false); return send_json_error_free(api, 400, "bad_request", "Config item is invalid", @@ -764,7 +764,7 @@ static int api_config_patch(struct ftl_conn *api) if(new_item->f & FLAG_ENV_VAR && !compare_config_item(conf_item->t, &new_item->v, &conf_item->v)) { char *key = strdup(new_item->k); - free_config(&newconf); + free_config(&newconf, false); return send_json_error_free(api, 400, "bad_request", "Config items set via environment variables cannot be changed via the API", @@ -789,7 +789,7 @@ static int api_config_patch(struct ftl_conn *api) char errbuf[VALIDATOR_ERRBUF_LEN] = { 0 }; if(!conf_item->c(&new_item->v, new_item->k, errbuf)) { - free_config(&newconf); + free_config(&newconf, false); return send_json_error(api, 400, "bad_request", "Config item validation failed", @@ -832,7 +832,7 @@ static int api_config_patch(struct ftl_conn *api) } else { - free_config(&newconf); + free_config(&newconf, false); return send_json_error(api, 400, "bad_request", "Invalid configuration", @@ -856,7 +856,7 @@ static int api_config_patch(struct ftl_conn *api) else { // Nothing changed, merely release copied config memory - free_config(&newconf); + free_config(&newconf, false); log_info("No config changes detected"); } @@ -932,7 +932,7 @@ static int api_config_put_delete(struct ftl_conn *api) if(new_item->f & FLAG_ENV_VAR) { char *key = strdup(new_item->k); - free_config(&newconf); + free_config(&newconf, false); free_config_path(requested_path); return send_json_error_free(api, 400, "bad_request", @@ -992,7 +992,7 @@ static int api_config_put_delete(struct ftl_conn *api) char errbuf[VALIDATOR_ERRBUF_LEN] = { 0 }; if(!new_item->c(&new_item->v, new_item->k, errbuf)) { - free_config(&newconf); + free_config(&newconf, false); free_config_path(requested_path); return send_json_error(api, 400, "bad_request", @@ -1018,7 +1018,7 @@ static int api_config_put_delete(struct ftl_conn *api) // Error 404 if config element not found if(!found) { - free_config(&newconf); + free_config(&newconf, false); cJSON *json = JSON_NEW_OBJECT(); JSON_SEND_OBJECT_CODE(json, 404); } @@ -1026,7 +1026,7 @@ static int api_config_put_delete(struct ftl_conn *api) // Error 400 if unique item already present if(message != NULL) { - free_config(&newconf); + free_config(&newconf, false); return send_json_error(api, 400, "bad_request", message, diff --git a/src/api/network.c b/src/api/network.c index 4130d6e9..5effdaf0 100644 --- a/src/api/network.c +++ b/src/api/network.c @@ -186,7 +186,7 @@ static int api_network_devices_GET(struct ftl_conn *api) { networkTable_readDevicesFinalize(device_stmt); dbclose(&db); - + // Add SQL message (may be NULL = not available) return send_json_error(api, 500, "database_error", diff --git a/src/api/padd.c b/src/api/padd.c index aa6e53cf..beaa2de3 100644 --- a/src/api/padd.c +++ b/src/api/padd.c @@ -22,8 +22,6 @@ #include "config/config.h" // uname() #include -// nlroutes(), nladdrs(), nllinks() -#include "tools/netlink.h" // struct proc_mem, getProcessMemory() #include "procps.h" // getcpu_percentage() diff --git a/src/config/cli.c b/src/config/cli.c index 933f5f5a..487c20fe 100644 --- a/src/config/cli.c +++ b/src/config/cli.c @@ -427,7 +427,7 @@ int set_config_from_CLI(const char *key, const char *value) if(item->f & FLAG_ENV_VAR) { log_err("Config option %s is read-only (set via environmental variable)", key); - free_config(&newconf); + free_config(&newconf, false); return ENV_VAR_FORCED; } @@ -435,7 +435,7 @@ int set_config_from_CLI(const char *key, const char *value) if(item->f & FLAG_READ_ONLY) { log_err("Config option %s can only be set in pihole.toml, not via the CLI", key); - free_config(&newconf); + free_config(&newconf, false); return EXIT_FAILURE; } @@ -459,14 +459,14 @@ int set_config_from_CLI(const char *key, const char *value) log_err(" - %s", matches[i]); free(matches); - free_config(&newconf); + free_config(&newconf, false); return KEY_UNKNOWN; } // Parse value if(!readStringValue(new_item, value, &newconf)) { - free_config(&newconf); + free_config(&newconf, false); return VALUE_INVALID; } @@ -484,7 +484,7 @@ int set_config_from_CLI(const char *key, const char *value) char errbuf[VALIDATOR_ERRBUF_LEN] = { 0 }; if(!new_item->c(&new_item->v, new_item->k, errbuf)) { - free_config(&newconf); + free_config(&newconf, false); log_err("Invalid value: %s", errbuf); return 3; } @@ -498,7 +498,7 @@ int set_config_from_CLI(const char *key, const char *value) { // Test failed log_debug(DEBUG_CONFIG, "Config item %s: dnsmasq config test failed", conf_item->k); - free_config(&newconf); + free_config(&newconf, false); return DNSMASQ_TEST_FAILED; } } @@ -519,7 +519,7 @@ int set_config_from_CLI(const char *key, const char *value) { // No change log_debug(DEBUG_CONFIG, "Config item %s: Unchanged", conf_item->k); - free_config(&newconf); + free_config(&newconf, false); // Print value writeTOMLvalue(stdout, -1, conf_item->t, &conf_item->v); diff --git a/src/config/config.c b/src/config/config.c index be10f3ea..2ba8e804 100644 --- a/src/config/config.c +++ b/src/config/config.c @@ -334,14 +334,14 @@ bool compare_config_item(const enum conf_type t, const union conf_value *val1, c } -void free_config(struct config *conf) +void free_config(struct config *conf, const bool terminating) { // Post-processing: // Initialize and verify config data for(unsigned int i = 0; i < CONFIG_ELEMENTS; i++) { - // Get pointer to memory location of this conf_item (copy) - struct conf_item *copy_item = get_conf_item(conf, i); + // Get pointer to memory location of this conf_item + struct conf_item *conf_item = get_conf_item(conf, i); // Free allowed values (if defined) // Note: This is no necessary as we simply leave the allowed values @@ -350,7 +350,7 @@ void free_config(struct config *conf) // if(conf->a != NULL) cJSON_Delete(conf->a); // Make a type-dependent copy of the value - switch(copy_item->t) + switch(conf_item->t) { case CONF_BOOL: case CONF_INT: @@ -375,12 +375,19 @@ void free_config(struct config *conf) // Nothing to do break; case CONF_STRING_ALLOCATED: - free(copy_item->v.s); - copy_item->v.s = NULL; - copy_item->t = CONF_STRING; // not allocated anymore + free(conf_item->v.s); + conf_item->v.s = NULL; + conf_item->t = CONF_STRING; // not allocated anymore break; case CONF_JSON_STRING_ARRAY: - cJSON_Delete(copy_item->v.json); + // Delete default JSON only when terminating. + // During config replacements, it is simply + // handed over from the old to the new config + // structure to avoid unnecessary memory + // duplications + if(terminating) + cJSON_Delete(conf_item->d.json); + cJSON_Delete(conf_item->v.json); break; } } @@ -1043,12 +1050,12 @@ void initConfig(struct config *conf) conf->webserver.headers.t = CONF_JSON_STRING_ARRAY; conf->webserver.headers.f = FLAG_RESTART_FTL; conf->webserver.headers.d.json = cJSON_CreateArray(); - cJSON_AddItemReferenceToArray(conf->webserver.headers.d.json, cJSON_CreateStringReference("X-DNS-Prefetch-Control: off")); - cJSON_AddItemReferenceToArray(conf->webserver.headers.d.json, cJSON_CreateStringReference("Content-Security-Policy: default-src 'self'; style-src 'self' 'unsafe-inline'; img-src 'self' data:;")); - cJSON_AddItemReferenceToArray(conf->webserver.headers.d.json, cJSON_CreateStringReference("X-Frame-Options: DENY")); - cJSON_AddItemReferenceToArray(conf->webserver.headers.d.json, cJSON_CreateStringReference("X-XSS-Protection: 0")); - cJSON_AddItemReferenceToArray(conf->webserver.headers.d.json, cJSON_CreateStringReference("X-Content-Type-Options: nosniff")); - cJSON_AddItemReferenceToArray(conf->webserver.headers.d.json, cJSON_CreateStringReference("Referrer-Policy: strict-origin-when-cross-origin")); + cJSON_AddItemToArray(conf->webserver.headers.d.json, cJSON_CreateStringReference("X-DNS-Prefetch-Control: off")); + cJSON_AddItemToArray(conf->webserver.headers.d.json, cJSON_CreateStringReference("Content-Security-Policy: default-src 'self'; style-src 'self' 'unsafe-inline'; img-src 'self' data:;")); + cJSON_AddItemToArray(conf->webserver.headers.d.json, cJSON_CreateStringReference("X-Frame-Options: DENY")); + cJSON_AddItemToArray(conf->webserver.headers.d.json, cJSON_CreateStringReference("X-XSS-Protection: 0")); + cJSON_AddItemToArray(conf->webserver.headers.d.json, cJSON_CreateStringReference("X-Content-Type-Options: nosniff")); + cJSON_AddItemToArray(conf->webserver.headers.d.json, cJSON_CreateStringReference("Referrer-Policy: strict-origin-when-cross-origin")); conf->webserver.headers.c = validate_stub; // Only type-based checking conf->webserver.serve_all.k = "webserver.serve_all"; @@ -1067,7 +1074,7 @@ void initConfig(struct config *conf) conf->webserver.session.restore.k = "webserver.session.restore"; conf->webserver.session.restore.h = "Should Pi-hole backup and restore sessions from the database? This is useful if you want to keep your sessions after a restart of the web interface."; - conf->webserver.session.restore.a = cJSON_CreateStringReference("true or false"); + conf->webserver.session.restore.a = cJSON_CreateStringReference("true or false"); conf->webserver.session.restore.t = CONF_BOOL; conf->webserver.session.restore.d.b = true; conf->webserver.session.restore.c = validate_stub; // Only type-based checking @@ -1665,7 +1672,6 @@ void initConfig(struct config *conf) { conf_item->a = cJSON_CreateStringReference("true or false"); } - } } @@ -1956,7 +1962,7 @@ void replace_config(struct config *newconf) memcpy(&config, newconf, sizeof(struct config)); // Free old backup struct - free_config(&old_conf); + free_config(&old_conf, false); // Unlock shared memory unlock_shm(); @@ -2016,7 +2022,7 @@ void reread_config(void) { // New configuration is invalid, restore old one log_debug(DEBUG_CONFIG, "Modified config file is invalid, discarding and overwriting with current configuration"); - free_config(&conf_copy); + free_config(&conf_copy, false); } // Write the config file back to disk to ensure that all options and diff --git a/src/config/config.h b/src/config/config.h index 47b84c2b..e5dc91a8 100644 --- a/src/config/config.h +++ b/src/config/config.h @@ -371,7 +371,7 @@ struct conf_item *get_conf_item(struct config *conf, const unsigned int n); struct conf_item *get_debug_item(struct config *conf, const enum debug_flag debug); unsigned int config_path_depth(char **paths) __attribute__ ((pure)); void duplicate_config(struct config *dst, struct config *src); -void free_config(struct config *conf); +void free_config(struct config *conf, const bool terminating); bool compare_config_item(const enum conf_type t, const union conf_value *val1, const union conf_value *val2); char **gen_config_path(const char *pathin, const char delim); void free_config_path(char **paths); diff --git a/src/daemon.c b/src/daemon.c index 45e399fb..d92ab343 100644 --- a/src/daemon.c +++ b/src/daemon.c @@ -65,6 +65,9 @@ void go_daemon(void) if (process_id > 0) { printf("FTL started!\n"); + // Free config to silence meaningless (but still loud) memcheck + // warnings about lost memory concerning the parsed config + free_config(&config, true); // Return success in exit status exit(EXIT_SUCCESS); } @@ -423,18 +426,21 @@ void cleanup(const int ret) // Close memory database close_memory_database(); - // Remove shared memory objects - // Important: This invalidated all objects such as - // counters-> ... etc. - // This should be the last action when c - destroy_shmem(); - // De-initialize the random number generator and entropy collector destroy_entropy(); // Free environment variables freeEnvVars(); + // Free config + free_config(&config, true); + + // Remove shared memory objects + // Important: This invalidated all objects such as + // counters-> ... etc. + // This should be the last action when cleaning up + destroy_shmem(); + char buffer[42] = { 0 }; format_time(buffer, 0, timer_elapsed_msec(EXIT_TIMER)); if(ret == RESTART_FTL_CODE) diff --git a/src/database/message-table.c b/src/database/message-table.c index 4fbf8c04..d7cf4904 100644 --- a/src/database/message-table.c +++ b/src/database/message-table.c @@ -359,9 +359,9 @@ static int _add_message(const enum message_type type, return -1; } - sqlite3 *db; + sqlite3 *db = dbopen(false, false); // Open database connection - if((db = dbopen(false, false)) == NULL) + if(db == NULL) // Reason for failure is logged in dbopen() return -1; @@ -380,8 +380,6 @@ static int _add_message(const enum message_type type, { log_err("add_message(type=%u, message=%s) - Failed to bind type DELETE: %s", type, message, sqlite3_errstr(rc)); - sqlite3_reset(stmt); - sqlite3_finalize(stmt); goto end_of_add_message; } @@ -390,8 +388,6 @@ static int _add_message(const enum message_type type, { log_err("add_message(type=%u, message=%s) - Failed to bind message DELETE: %s", type, message, sqlite3_errstr(rc)); - sqlite3_reset(stmt); - sqlite3_finalize(stmt); goto end_of_add_message; } @@ -423,8 +419,6 @@ static int _add_message(const enum message_type type, { log_err("add_message(type=%u, message=%s) - Failed to bind type: %s", type, message, sqlite3_errstr(rc)); - sqlite3_reset(stmt); - sqlite3_finalize(stmt); goto end_of_add_message; } @@ -433,8 +427,6 @@ static int _add_message(const enum message_type type, { log_err("add_message(type=%u, message=%s) - Failed to bind message: %s", type, message, sqlite3_errstr(rc)); - sqlite3_reset(stmt); - sqlite3_finalize(stmt); goto end_of_add_message; } @@ -489,15 +481,19 @@ static int _add_message(const enum message_type type, goto end_of_add_message; } - // Final database handling - sqlite3_clear_bindings(stmt); - sqlite3_reset(stmt); - sqlite3_finalize(stmt); - - // Get row ID of the newly added message - rowid = sqlite3_last_insert_rowid(db); - end_of_add_message: // Close database connection + + // Final database handling + if(stmt != NULL) + { + sqlite3_clear_bindings(stmt); + sqlite3_reset(stmt); + sqlite3_finalize(stmt); + + // Get row ID of the newly added message + rowid = sqlite3_last_insert_rowid(db); + } + dbclose(&db); return rowid; diff --git a/src/database/network-table.c b/src/database/network-table.c index 22c59a3a..10a1df9d 100644 --- a/src/database/network-table.c +++ b/src/database/network-table.c @@ -1069,6 +1069,7 @@ static bool add_local_interfaces_to_network_table(sqlite3 *db, time_t now, unsig if(!nllinks(links, false)) { log_err("Failed to get links, cannot update network table"); + cJSON_Delete(links); return false; } log_debug(DEBUG_ARP, "Network table: Successfully read links with %i entries", @@ -1166,6 +1167,9 @@ static bool add_local_interfaces_to_network_table(sqlite3 *db, time_t now, unsig } } + // Free allocated memory + cJSON_Delete(link); + return true; } @@ -1544,6 +1548,9 @@ void parse_neighbor_cache(sqlite3 *db) entries++; } + // Free allocated JSON array + cJSON_Delete(json); + log_debug(DEBUG_ARP, "Network table: Finished parsing ARP cache with %u entries", entries); if(rc != SQLITE_OK) diff --git a/src/ntp/client.c b/src/ntp/client.c index 69300fb8..79b80ec4 100644 --- a/src/ntp/client.c +++ b/src/ntp/client.c @@ -478,7 +478,7 @@ bool ntp_client(const char *server, const bool settime, const bool print) { // Resolve server address int eai; - struct addrinfo *saddr; + struct addrinfo *saddr = NULL; // Resolve server address, port 123 is used for NTP if((eai = getaddrinfo(server, "123", NULL, &saddr)) != 0) { @@ -494,6 +494,7 @@ bool ntp_client(const char *server, const bool settime, const bool print) } errbuf[sizeof(errbuf) - 1] = '\0'; log_ntp_message(true, false, errbuf); + freeaddrinfo(saddr); return false; } @@ -502,6 +503,7 @@ bool ntp_client(const char *server, const bool settime, const bool print) if(ntp == NULL) { log_err("Cannot allocate memory for NTP client"); + freeaddrinfo(saddr); return false; } @@ -542,6 +544,7 @@ bool ntp_client(const char *server, const bool settime, const bool print) // Free allocated memory freeaddrinfo(saddr); + saddr = NULL; // Compute average and standard deviation unsigned int valid = 0; diff --git a/src/webserver/webserver.c b/src/webserver/webserver.c index e0999d23..0e3cce53 100644 --- a/src/webserver/webserver.c +++ b/src/webserver/webserver.c @@ -38,6 +38,7 @@ static char *prefix_webhome = NULL; static char *api_uri = NULL; static char *admin_api_uri = NULL; static char *login_uri = NULL; +static char *webheaders = NULL; // Private prototypes static char *append_to_path(char *path, const char *append); @@ -577,7 +578,7 @@ void http_init(void) } // Construct additional headers - char *webheaders = strdup(""); + webheaders = strdup(""); if (webheaders == NULL) { log_err("Failed to allocate memory for webheaders!"); return; @@ -595,11 +596,14 @@ void http_init(void) const char *h = cJSON_GetStringValue(header); // Allocate memory for the new header - webheaders = realloc(webheaders, strlen(webheaders) + strlen(h) + 3); - if (webheaders == NULL) { - log_err("Failed to allocate memory for webheaders!"); + char *new_webheaders = realloc(webheaders, strlen(webheaders) + strlen(h) + 3); + if (new_webheaders == NULL) { + log_err("Failed to (re)allocate memory for webheaders!"); + free(webheaders); + webheaders = NULL; return; } + webheaders = new_webheaders; strcat(webheaders, h); strcat(webheaders, "\r\n"); } @@ -866,8 +870,11 @@ void http_terminate(void) // Free admin_api_uri path if(admin_api_uri != NULL) free(admin_api_uri); - + // Free login_uri path if(login_uri != NULL) free(login_uri); + + if(webheaders != NULL) + free(webheaders); }