nihilist/tor
1
0
Fork 0
mirror of https://gitlab.torproject.org/tpo/core/tor.git synced 2024-09-20 13:06:20 +02:00
Code Issues Packages Projects Releases Wiki Activity

Refactor config free logic to use a single path.

Browse Source 
The right way to free a config object is now to wrap config_free(),
always.  Instead of creating an alternative free function, objects
should provide an alternative clear callback to free any fields that
the configuration manager doesn't manage.

This lets us simplify our code a little, and lets us extend the
confparse.c code to manage additional fields in config_free.
This commit is contained in:
Nick Mathewson 2019-07-23 12:04:59 -04:00
parent 3d1f9f583a
commit 47654d3249
12 changed files with 112 additions and 99 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

29
src/app/config/config.c
View File

@ -843,9 +843,9 @@ static void config_maybe_load_geoip_files_(const or_options_t *options,
static int options_validate_cb(void *old_options, void *options,
void *default_options,
int from_setconf, char **msg);
static void options_free_cb(const config_mgr_t *, void *options);
static void cleanup_protocol_warning_severity_level(void);
static void set_protocol_warning_severity_level(int warning_severity);
static void options_clear_cb(const config_mgr_t *mgr, void *opts);
/** Magic value for or_options_t. */
#define OR_OPTIONS_MAGIC 9090909
@ -862,8 +862,8 @@ static const config_format_t options_format = {
option_deprecation_notes_,
option_vars_,
options_validate_cb,
options_free_cb,
NULL
options_clear_cb,
NULL,
};
/*
@ -1011,11 +1011,11 @@ set_options(or_options_t *new_val, char **msg)
/** Release additional memory allocated in options
*/
STATIC void
or_options_free_(or_options_t *options)
static void
options_clear_cb(const config_mgr_t *mgr, void *opts)
{
if (!options)
return;
(void)mgr;
or_options_t *options = opts;
routerset_free(options->ExcludeExitNodesUnion_);
if (options->NodeFamilySets) {
@ -1038,6 +1038,13 @@ or_options_free_(or_options_t *options)
tor_free(options->command_arg);
tor_free(options->master_key_fname);
config_free_lines(options->MyFamily);
}
/** Release all memory allocated in options
*/
STATIC void
or_options_free_(or_options_t *options)
{
config_free(get_options_mgr(), options);
}
@ -3164,14 +3171,6 @@ options_validate_cb(void *old_options, void *options, void *default_options,
return rv;
}
/** Callback to free an or_options_t */
static void
options_free_cb(const config_mgr_t *mgr, void *options)
{
(void)mgr;
or_options_free_(options);
}
#define REJECT(arg) \
STMT_BEGIN *msg = tor_strdup(arg); return -1; STMT_END
#if defined(__GNUC__) && __GNUC__ <= 3

8
src/app/config/confparse.c
View File

@ -789,6 +789,10 @@ config_free_(const config_mgr_t *mgr, void *options)
tor_assert(fmt);
if (mgr->toplevel->clear_fn) {
mgr->toplevel->clear_fn(mgr, options);
}
SMARTLIST_FOREACH_BEGIN(mgr->all_vars, const managed_var_t *, mv) {
config_clear(mgr, options, mv);
} SMARTLIST_FOREACH_END(mv);
@ -977,9 +981,7 @@ config_dump(const config_mgr_t *mgr, const void *default_options,
result = smartlist_join_strings(elements, "", 0, NULL);
SMARTLIST_FOREACH(elements, char *, cp, tor_free(cp));
smartlist_free(elements);
if (defaults_tmp) {
fmt->free_fn(mgr, defaults_tmp);
}
config_free(mgr, defaults_tmp);
return result;
}

6
src/app/config/confparse.h
View File

@ -41,8 +41,8 @@ typedef int (*validate_fn_t)(void*,void*,void*,int,char**);
struct config_mgr_t;
/** Callback to free a configuration object. */
typedef void (*free_cfg_fn_t)(const struct config_mgr_t *mgr, void*);
/** Callback to clear all non-managed fields of a configuration object. */
typedef void (*clear_cfg_fn_t)(const struct config_mgr_t *mgr, void*);
/** Information on the keys, value types, key-to-struct-member mappings,
* variable descriptions, validation functions, and abbreviations for a
@ -57,7 +57,7 @@ typedef struct config_format_t {
* values, and where we stick them in the
* structure. */
validate_fn_t validate_fn; /**< Function to validate config. */
free_cfg_fn_t free_fn; /**< Function to free the configuration. */
clear_cfg_fn_t clear_fn; /**< Function to clear the configuration. */
/** If present, extra denotes a LINELIST variable for unrecognized
* lines. Otherwise, unrecognized lines are an error. */
const struct_member_t *extra;

11
src/app/config/statefile.c
View File

@ -145,8 +145,6 @@ static int or_state_validate_cb(void *old_options, void *options,
void *default_options,
int from_setconf, char **msg);
static void or_state_free_cb(const config_mgr_t *mgr, void *state);
/** Magic value for or_state_t. */
#define OR_STATE_MAGIC 0x57A73f57
@ -170,7 +168,7 @@ static const config_format_t state_format = {
NULL,
state_vars_,
or_state_validate_cb,
or_state_free_cb,
NULL,
&state_extra_var,
};
@ -282,13 +280,6 @@ or_state_validate_cb(void *old_state, void *state, void *default_state,
return or_state_validate(state, msg);
}
static void
or_state_free_cb(const config_mgr_t *mgr, void *state)
{
(void)mgr;
or_state_free_(state);
}
/** Return 0 if every setting in <b>state</b> is reasonable, and a
* permissible transition from <b>old_state</b>. Else warn and return -1.
* Should have no side effects, except for normalizing the contents of

10
src/feature/dirauth/shared_random_state.c
View File

@ -62,7 +62,6 @@ DUMMY_TYPECHECK_INSTANCE(sr_disk_state_t);
static int
disk_state_validate_cb(void *old_state, void *state, void *default_state,
int from_setconf, char **msg);
static void disk_state_free_cb(const config_mgr_t *mgr, void *);
/* Array of variables that are saved to disk as a persistent state. */
static const config_var_t state_vars[] = {
@ -99,7 +98,7 @@ static const config_format_t state_format = {
NULL,
state_vars,
disk_state_validate_cb,
disk_state_free_cb,
NULL,
&state_extra_var,
};
@ -361,13 +360,6 @@ disk_state_validate_cb(void *old_state, void *state, void *default_state,
return 0;
}
static void
disk_state_free_cb(const config_mgr_t *mgr, void *state)
{
(void)mgr;
disk_state_free_(state);
}
/* Parse the Commit line(s) in the disk state and translate them to the
* the memory state. Return 0 on success else -1 on error. */
static int

3
src/test/fuzz/fuzzing_common.c
View File

@ -1,6 +1,7 @@
/* Copyright (c) 2016-2019, The Tor Project, Inc. */
/* See LICENSE for licensing information */
#define CRYPTO_ED25519_PRIVATE
#define CONFIG_PRIVATE
#include "orconfig.h"
#include "core/or/or.h"
#include "app/main/subsysmgr.h"
@ -189,7 +190,7 @@ main(int argc, char **argv)
if (fuzz_cleanup() < 0)
abort();
tor_free(mock_options);
or_options_free(mock_options);
UNMOCK(get_options);
return 0;
}

84
src/test/test_config.c
View File

@ -1755,6 +1755,18 @@ add_default_fallback_dir_servers_known_default(void)
n_add_default_fallback_dir_servers_known_default++;
}
/* Helper for test_config_adding_dir_servers(), which should be
* refactored: clear the fields in the options which the options object
* does not really own. */
static void
ads_clear_helper(or_options_t *options)
{
options->DirAuthorities = NULL;
options->AlternateBridgeAuthority = NULL;
options->AlternateDirAuthority = NULL;
options->FallbackDir = NULL;
}
/* Test all the different combinations of adding dir servers */
static void
test_config_adding_dir_servers(void *arg)
@ -1885,7 +1897,9 @@ test_config_adding_dir_servers(void *arg)
n_add_default_fallback_dir_servers_known_default = 0;
/* clear options*/
memset(options, 0, sizeof(or_options_t));
ads_clear_helper(options);
or_options_free(options);
options = options_new();
/* clear any previous dir servers:
consider_adding_dir_servers() should do this anyway */
@ -1967,7 +1981,9 @@ test_config_adding_dir_servers(void *arg)
n_add_default_fallback_dir_servers_known_default = 0;
/* clear options*/
memset(options, 0, sizeof(or_options_t));
ads_clear_helper(options);
or_options_free(options);
options = options_new();
/* clear any previous dir servers:
consider_adding_dir_servers() should do this anyway */
@ -2108,7 +2124,9 @@ test_config_adding_dir_servers(void *arg)
n_add_default_fallback_dir_servers_known_default = 0;
/* clear options*/
memset(options, 0, sizeof(or_options_t));
ads_clear_helper(options);
or_options_free(options);
options = options_new();
/* clear any previous dir servers:
consider_adding_dir_servers() should do this anyway */
@ -2249,7 +2267,9 @@ test_config_adding_dir_servers(void *arg)
n_add_default_fallback_dir_servers_known_default = 0;
/* clear options*/
memset(options, 0, sizeof(or_options_t));
ads_clear_helper(options);
or_options_free(options);
options = options_new();
/* clear any previous dir servers:
consider_adding_dir_servers() should do this anyway */
@ -2391,7 +2411,9 @@ test_config_adding_dir_servers(void *arg)
n_add_default_fallback_dir_servers_known_default = 0;
/* clear options*/
memset(options, 0, sizeof(or_options_t));
ads_clear_helper(options);
or_options_free(options);
options = options_new();
/* clear any previous dir servers:
consider_adding_dir_servers() should do this anyway */
@ -2543,7 +2565,9 @@ test_config_adding_dir_servers(void *arg)
n_add_default_fallback_dir_servers_known_default = 0;
/* clear options*/
memset(options, 0, sizeof(or_options_t));
ads_clear_helper(options);
or_options_free(options);
options = options_new();
/* clear any previous dir servers:
consider_adding_dir_servers() should do this anyway */
@ -2697,7 +2721,9 @@ test_config_adding_dir_servers(void *arg)
n_add_default_fallback_dir_servers_known_default = 0;
/* clear options*/
memset(options, 0, sizeof(or_options_t));
ads_clear_helper(options);
or_options_free(options);
options = options_new();
/* clear any previous dir servers:
consider_adding_dir_servers() should do this anyway */
@ -2860,7 +2886,9 @@ test_config_adding_dir_servers(void *arg)
n_add_default_fallback_dir_servers_known_default = 0;
/* clear options*/
memset(options, 0, sizeof(or_options_t));
ads_clear_helper(options);
or_options_free(options);
options = options_new();
/* clear any previous dir servers:
consider_adding_dir_servers() should do this anyway */
@ -3017,7 +3045,9 @@ test_config_adding_dir_servers(void *arg)
n_add_default_fallback_dir_servers_known_default = 0;
/* clear options*/
memset(options, 0, sizeof(or_options_t));
ads_clear_helper(options);
or_options_free(options);
options = options_new();
/* clear any previous dir servers:
consider_adding_dir_servers() should do this anyway */
@ -3183,7 +3213,9 @@ test_config_adding_dir_servers(void *arg)
n_add_default_fallback_dir_servers_known_default = 0;
/* clear options*/
memset(options, 0, sizeof(or_options_t));
ads_clear_helper(options);
or_options_free(options);
options = options_new();
/* clear any previous dir servers:
consider_adding_dir_servers() should do this anyway */
@ -3346,7 +3378,9 @@ test_config_adding_dir_servers(void *arg)
n_add_default_fallback_dir_servers_known_default = 0;
/* clear options*/
memset(options, 0, sizeof(or_options_t));
ads_clear_helper(options);
or_options_free(options);
options = options_new();
/* clear any previous dir servers:
consider_adding_dir_servers() should do this anyway */
@ -3515,10 +3549,7 @@ test_config_adding_dir_servers(void *arg)
tor_free(test_fallback_directory->value);
tor_free(test_fallback_directory);
options->DirAuthorities = NULL;
options->AlternateBridgeAuthority = NULL;
options->AlternateDirAuthority = NULL;
options->FallbackDir = NULL;
ads_clear_helper(options);
or_options_free(options);
UNMOCK(add_default_fallback_dir_servers);
@ -3619,9 +3650,10 @@ test_config_directory_fetch(void *arg)
mock_router_my_exit_policy_is_reject_star);
MOCK(advertised_server_mode, mock_advertised_server_mode);
MOCK(router_get_my_routerinfo, mock_router_get_my_routerinfo);
or_options_free(options);
options = options_new();
/* Clients can use multiple directory mirrors for bootstrap */
memset(options, 0, sizeof(or_options_t));
options->ClientOnly = 1;
tt_assert(server_mode(options) == 0);
tt_assert(public_server_mode(options) == 0);
@ -3630,7 +3662,8 @@ test_config_directory_fetch(void *arg)
OP_EQ, 1);
/* Bridge Clients can use multiple directory mirrors for bootstrap */
memset(options, 0, sizeof(or_options_t));
or_options_free(options);
options = options_new();
options->UseBridges = 1;
tt_assert(server_mode(options) == 0);
tt_assert(public_server_mode(options) == 0);
@ -3640,7 +3673,8 @@ test_config_directory_fetch(void *arg)
/* Bridge Relays (Bridges) must act like clients, and use multiple
* directory mirrors for bootstrap */
memset(options, 0, sizeof(or_options_t));
or_options_free(options);
options = options_new();
options->BridgeRelay = 1;
options->ORPort_set = 1;
tt_assert(server_mode(options) == 1);
@ -3651,7 +3685,8 @@ test_config_directory_fetch(void *arg)
/* Clients set to FetchDirInfoEarly must fetch it from the authorities,
* but can use multiple authorities for bootstrap */
memset(options, 0, sizeof(or_options_t));
or_options_free(options);
options = options_new();
options->FetchDirInfoEarly = 1;
tt_assert(server_mode(options) == 0);
tt_assert(public_server_mode(options) == 0);
@ -3662,7 +3697,8 @@ test_config_directory_fetch(void *arg)
/* OR servers only fetch the consensus from the authorities when they don't
* know their own address, but never use multiple directories for bootstrap
*/
memset(options, 0, sizeof(or_options_t));
or_options_free(options);
options = options_new();
options->ORPort_set = 1;
mock_router_pick_published_address_result = -1;
@ -3682,7 +3718,8 @@ test_config_directory_fetch(void *arg)
/* Exit OR servers only fetch the consensus from the authorities when they
* refuse unknown exits, but never use multiple directories for bootstrap
*/
memset(options, 0, sizeof(or_options_t));
or_options_free(options);
options = options_new();
options->ORPort_set = 1;
options->ExitRelay = 1;
mock_router_pick_published_address_result = 0;
@ -3712,7 +3749,8 @@ test_config_directory_fetch(void *arg)
* advertising their dirport, and never use multiple directories for
* bootstrap. This only applies if they are also OR servers.
* (We don't care much about the behaviour of non-OR directory servers.) */
memset(options, 0, sizeof(or_options_t));
or_options_free(options);
options = options_new();
options->DirPort_set = 1;
options->ORPort_set = 1;
options->DirCache = 1;
@ -3766,7 +3804,7 @@ test_config_directory_fetch(void *arg)
OP_EQ, 0);
done:
tor_free(options);
or_options_free(options);
UNMOCK(router_pick_published_address);
UNMOCK(router_get_my_routerinfo);
UNMOCK(advertised_server_mode);

15
src/test/test_confparse.c
View File

@ -119,8 +119,6 @@ test_validate_cb(void *old_options, void *options, void *default_options,
return 0;
}
static void test_free_cb(const config_mgr_t *mgr, void *options);
#define TEST_MAGIC 0x1337
static const config_format_t test_fmt = {
@ -134,19 +132,10 @@ static const config_format_t test_fmt = {
test_deprecation_notes,
test_vars,
test_validate_cb,
test_free_cb,
NULL,
NULL,
};
static void
test_free_cb(const config_mgr_t *mgr, void *options)
{
if (!options)
return;
config_free(mgr, options);
}
/* Make sure that config_init sets everything to the right defaults. */
static void
test_confparse_init(void *arg)
@ -824,7 +813,7 @@ static config_format_t etest_fmt = {
test_deprecation_notes,
test_vars,
test_validate_cb,
test_free_cb,
NULL,
&extra,
};

3
src/test/test_dir_handle_get.c
View File

@ -479,8 +479,7 @@ static or_options_t *mock_options = NULL;
static void
init_mock_options(void)
{
mock_options = tor_malloc(sizeof(or_options_t));
memset(mock_options, 0, sizeof(or_options_t));
mock_options = options_new();
mock_options->TestingTorNetwork = 1;
mock_options->DataDirectory = tor_strdup(get_fname_rnd("datadir_tmp"));
mock_options->CacheDirectory = tor_strdup(mock_options->DataDirectory);

30
src/test/test_entrynodes.c
View File

@ -5,6 +5,7 @@
#define CIRCUITLIST_PRIVATE
#define CIRCUITBUILD_PRIVATE
#define CONFIG_PRIVATE
#define STATEFILE_PRIVATE
#define ENTRYNODES_PRIVATE
#define ROUTERLIST_PRIVATE
@ -201,7 +202,7 @@ big_fake_network_setup(const struct testcase_t *testcase)
smartlist_add(big_fake_net_nodes, n);
}
dummy_state = tor_malloc_zero(sizeof(or_state_t));
dummy_state = or_state_new();
dummy_consensus = tor_malloc_zero(sizeof(networkstatus_t));
if (reasonably_future_consensus) {
/* Make the dummy consensus valid in 6 hours, and expiring in 7 hours. */
@ -235,12 +236,12 @@ mock_randomize_time_no_randomization(time_t a, time_t b)
return a;
}
static or_options_t mocked_options;
static or_options_t *mocked_options;
static const or_options_t *
mock_get_options(void)
{
return &mocked_options;
return mocked_options;
}
#define TEST_IPV4_ADDR "123.45.67.89"
@ -259,7 +260,7 @@ test_node_preferred_orport(void *arg)
tor_addr_port_t ap;
/* Setup options */
memset(&mocked_options, 0, sizeof(mocked_options));
mocked_options = options_new();
/* We don't test ClientPreferIPv6ORPort here, because it's used in
* nodelist_set_consensus to setup node.ipv6_preferred, which we set
* directly. */
@ -282,8 +283,8 @@ test_node_preferred_orport(void *arg)
/* Check the preferred address is IPv4 if we're only using IPv4, regardless
* of whether we prefer it or not */
mocked_options.ClientUseIPv4 = 1;
mocked_options.ClientUseIPv6 = 0;
mocked_options->ClientUseIPv4 = 1;
mocked_options->ClientUseIPv6 = 0;
node.ipv6_preferred = 0;
node_get_pref_orport(&node, &ap);
tt_assert(tor_addr_eq(&ap.addr, &ipv4_addr));
@ -296,8 +297,8 @@ test_node_preferred_orport(void *arg)
/* Check the preferred address is IPv4 if we're using IPv4 and IPv6, but
* don't prefer the IPv6 address */
mocked_options.ClientUseIPv4 = 1;
mocked_options.ClientUseIPv6 = 1;
mocked_options->ClientUseIPv4 = 1;
mocked_options->ClientUseIPv6 = 1;
node.ipv6_preferred = 0;
node_get_pref_orport(&node, &ap);
tt_assert(tor_addr_eq(&ap.addr, &ipv4_addr));
@ -305,28 +306,29 @@ test_node_preferred_orport(void *arg)
/* Check the preferred address is IPv6 if we prefer it and
* ClientUseIPv6 is 1, regardless of ClientUseIPv4 */
mocked_options.ClientUseIPv4 = 1;
mocked_options.ClientUseIPv6 = 1;
mocked_options->ClientUseIPv4 = 1;
mocked_options->ClientUseIPv6 = 1;
node.ipv6_preferred = 1;
node_get_pref_orport(&node, &ap);
tt_assert(tor_addr_eq(&ap.addr, &ipv6_addr));
tt_assert(ap.port == ipv6_port);
mocked_options.ClientUseIPv4 = 0;
mocked_options->ClientUseIPv4 = 0;
node_get_pref_orport(&node, &ap);
tt_assert(tor_addr_eq(&ap.addr, &ipv6_addr));
tt_assert(ap.port == ipv6_port);
/* Check the preferred address is IPv6 if we don't prefer it, but
* ClientUseIPv4 is 0 */
mocked_options.ClientUseIPv4 = 0;
mocked_options.ClientUseIPv6 = 1;
node.ipv6_preferred = fascist_firewall_prefer_ipv6_orport(&mocked_options);
mocked_options->ClientUseIPv4 = 0;
mocked_options->ClientUseIPv6 = 1;
node.ipv6_preferred = fascist_firewall_prefer_ipv6_orport(mocked_options);
node_get_pref_orport(&node, &ap);
tt_assert(tor_addr_eq(&ap.addr, &ipv6_addr));
tt_assert(ap.port == ipv6_port);
done:
or_options_free(mocked_options);
UNMOCK(get_options);
}

10
src/test/test_hs_service.c
View File

@ -1181,7 +1181,7 @@ test_introduce2(void *arg)
MOCK(get_or_state,
get_or_state_replacement);
dummy_state = tor_malloc_zero(sizeof(or_state_t));
dummy_state = or_state_new();
circ = helper_create_origin_circuit(CIRCUIT_PURPOSE_S_INTRO, flags);
tt_assert(circ);
@ -1345,7 +1345,7 @@ test_rotate_descriptors(void *arg)
(void) arg;
dummy_state = tor_malloc_zero(sizeof(or_state_t));
dummy_state = or_state_new();
hs_init();
MOCK(get_or_state, get_or_state_replacement);
@ -1462,7 +1462,7 @@ test_build_update_descriptors(void *arg)
MOCK(networkstatus_get_live_consensus,
mock_networkstatus_get_live_consensus);
dummy_state = tor_malloc_zero(sizeof(or_state_t));
dummy_state = or_state_new();
ret = parse_rfc1123_time("Sat, 26 Oct 1985 03:00:00 UTC",
&mock_ns.valid_after);
@ -1693,7 +1693,7 @@ test_build_descriptors(void *arg)
MOCK(networkstatus_get_live_consensus,
mock_networkstatus_get_live_consensus);
dummy_state = tor_malloc_zero(sizeof(or_state_t));
dummy_state = or_state_new();
ret = parse_rfc1123_time("Sat, 26 Oct 1985 03:00:00 UTC",
&mock_ns.valid_after);
@ -1794,7 +1794,7 @@ test_upload_descriptors(void *arg)
MOCK(networkstatus_get_live_consensus,
mock_networkstatus_get_live_consensus);
dummy_state = tor_malloc_zero(sizeof(or_state_t));
dummy_state = or_state_new();
ret = parse_rfc1123_time("Sat, 26 Oct 1985 13:00:00 UTC",
&mock_ns.valid_after);

2
src/test/test_pt.c
View File

@ -352,7 +352,7 @@ test_pt_configure_proxy(void *arg)
managed_proxy_t *mp = NULL;
(void) arg;
dummy_state = tor_malloc_zero(sizeof(or_state_t));
dummy_state = or_state_new();
MOCK(process_read_stdout, process_read_stdout_replacement);
MOCK(get_or_state,