nihilist/tor
1
0
Fork 0
mirror of https://gitlab.torproject.org/tpo/core/tor.git synced 2024-11-10 21:23:58 +01:00
Code Issues Packages Projects Releases Wiki Activity

sr: Check for replacing a SRV pointer with the same pointer

Browse Source 
Check if the new pointer is the same as the old one: if it is, it's
probably a bug:
* the caller may have confused current and previous, or
* they may have forgotten to sr_srv_dup().

Putting NULL multiple times is allowed.

Part of 29706.
This commit is contained in:
teor 2019-03-12 10:24:34 +10:00
parent 593fde930f
commit 0cca554110
2 changed files with 230 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

32
src/or/shared_random_state.c
View File

@ -900,14 +900,26 @@ state_query_put_(sr_state_object_t obj_type, void *data)
break;
}
case SR_STATE_OBJ_CURSRV:
/* Always free the old SRV */
state_query_del_(SR_STATE_OBJ_CURSRV, NULL);
sr_state->current_srv = (sr_srv_t *) data;
/* Check if the new pointer is the same as the old one: if it is, it's
* probably a bug. The caller may have confused current and previous,
* or they may have forgotten to sr_srv_dup().
* Putting NULL multiple times is allowed. */
if (!BUG(data && sr_state->current_srv == (sr_srv_t *) data)) {
/* We own the old SRV, so we need to free it. */
state_query_del_(SR_STATE_OBJ_CURSRV, NULL);
sr_state->current_srv = (sr_srv_t *) data;
}
break;
case SR_STATE_OBJ_PREVSRV:
/* Always free the old SRV */
state_query_del_(SR_STATE_OBJ_PREVSRV, NULL);
sr_state->previous_srv = (sr_srv_t *) data;
/* Check if the new pointer is the same as the old one: if it is, it's
* probably a bug. The caller may have confused current and previous,
* or they may have forgotten to sr_srv_dup().
* Putting NULL multiple times is allowed. */
if (!BUG(data && sr_state->previous_srv == (sr_srv_t *) data)) {
/* We own the old SRV, so we need to free it. */
state_query_del_(SR_STATE_OBJ_PREVSRV, NULL);
sr_state->previous_srv = (sr_srv_t *) data;
}
break;
case SR_STATE_OBJ_VALID_AFTER:
sr_state->valid_after = *((time_t *) data);
@ -1059,7 +1071,9 @@ sr_state_get_phase(void)
return *(sr_phase_t *) ptr;
}
/* Return the previous SRV value from our state. Value CAN be NULL. */
/* Return the previous SRV value from our state. Value CAN be NULL.
* The state object owns the SRV, so the calling code should not free the SRV.
* Use sr_srv_dup() if you want to keep a copy of the SRV. */
const sr_srv_t *
sr_state_get_previous_srv(void)
{
@ -1078,7 +1092,9 @@ sr_state_set_previous_srv(const sr_srv_t *srv)
NULL);
}
/* Return the current SRV value from our state. Value CAN be NULL. */
/* Return the current SRV value from our state. Value CAN be NULL.
* The state object owns the SRV, so the calling code should not free the SRV.
* Use sr_srv_dup() if you want to keep a copy of the SRV. */
const sr_srv_t *
sr_state_get_current_srv(void)
{

212
src/test/test_shared_random.c
View File

@ -38,7 +38,8 @@ trusteddirserver_get_by_v3_auth_digest_m(const char *digest)
}
/* Setup a minimal dirauth environment by initializing the SR state and
* making sure the options are set to be an authority directory. */
* making sure the options are set to be an authority directory.
* You must only call this function once per process. */
static void
init_authority_state(void)
{
@ -451,7 +452,9 @@ test_encoding(void *arg)
/** Setup some SRVs in our SR state. If <b>also_current</b> is set, then set
* both current and previous SRVs.
* Helper of test_vote() and test_sr_compute_srv(). */
* Helper of test_vote() and test_sr_compute_srv().
* You must call sr_state_free() to free the state at the end of each test
* function (on pass or fail). */
static void
test_sr_setup_srv(int also_current)
{
@ -927,8 +930,9 @@ test_sr_get_majority_srv_from_votes(void *arg)
smartlist_free(votes);
}
/* Test utils that don't depend on authority state */
static void
test_utils(void *arg)
test_utils_general(void *arg)
{
(void) arg;
@ -991,9 +995,19 @@ test_utils(void *arg)
tt_str_op(get_phase_str(SR_PHASE_COMMIT), ==, "commit");
}
done:
return;
}
/* Test utils that depend on authority state */
static void
test_utils_auth(void *arg)
{
(void)arg;
init_authority_state();
/* Testing phase transition */
{
init_authority_state();
set_sr_phase(SR_PHASE_COMMIT);
tt_int_op(is_phase_transition(SR_PHASE_REVEAL), ==, 1);
tt_int_op(is_phase_transition(SR_PHASE_COMMIT), ==, 0);
@ -1004,8 +1018,193 @@ test_utils(void *arg)
tt_int_op(is_phase_transition(42), ==, 1);
}
/* Testing get, set, delete, clean SRVs */
{
/* Just set the previous SRV */
test_sr_setup_srv(0);
tt_ptr_op(sr_state_get_previous_srv(), OP_NE, NULL);
tt_ptr_op(sr_state_get_current_srv(), OP_EQ, NULL);
state_del_previous_srv();
tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL);
tt_ptr_op(sr_state_get_current_srv(), OP_EQ, NULL);
}
{
/* Delete the SRVs one at a time */
test_sr_setup_srv(1);
tt_ptr_op(sr_state_get_previous_srv(), OP_NE, NULL);
tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL);
state_del_current_srv();
tt_ptr_op(sr_state_get_previous_srv(), OP_NE, NULL);
tt_ptr_op(sr_state_get_current_srv(), OP_EQ, NULL);
state_del_previous_srv();
tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL);
tt_ptr_op(sr_state_get_current_srv(), OP_EQ, NULL);
/* And in the opposite order */
test_sr_setup_srv(1);
tt_ptr_op(sr_state_get_previous_srv(), OP_NE, NULL);
tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL);
state_del_previous_srv();
tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL);
tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL);
state_del_current_srv();
tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL);
tt_ptr_op(sr_state_get_current_srv(), OP_EQ, NULL);
/* And both at once */
test_sr_setup_srv(1);
tt_ptr_op(sr_state_get_previous_srv(), OP_NE, NULL);
tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL);
sr_state_clean_srvs();
tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL);
tt_ptr_op(sr_state_get_current_srv(), OP_EQ, NULL);
/* And do the gets and sets multiple times */
test_sr_setup_srv(1);
tt_ptr_op(sr_state_get_previous_srv(), OP_NE, NULL);
tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL);
tt_ptr_op(sr_state_get_previous_srv(), OP_NE, NULL);
tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL);
state_del_previous_srv();
tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL);
tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL);
state_del_previous_srv();
tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL);
tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL);
tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL);
tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL);
sr_state_clean_srvs();
tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL);
tt_ptr_op(sr_state_get_current_srv(), OP_EQ, NULL);
state_del_current_srv();
tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL);
tt_ptr_op(sr_state_get_current_srv(), OP_EQ, NULL);
sr_state_clean_srvs();
tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL);
tt_ptr_op(sr_state_get_current_srv(), OP_EQ, NULL);
state_del_current_srv();
tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL);
tt_ptr_op(sr_state_get_current_srv(), OP_EQ, NULL);
tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL);
tt_ptr_op(sr_state_get_current_srv(), OP_EQ, NULL);
}
{
/* Now set the SRVs to NULL instead */
test_sr_setup_srv(1);
tt_ptr_op(sr_state_get_previous_srv(), OP_NE, NULL);
tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL);
sr_state_set_current_srv(NULL);
tt_ptr_op(sr_state_get_previous_srv(), OP_NE, NULL);
tt_ptr_op(sr_state_get_current_srv(), OP_EQ, NULL);
sr_state_set_previous_srv(NULL);
tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL);
tt_ptr_op(sr_state_get_current_srv(), OP_EQ, NULL);
/* And in the opposite order */
test_sr_setup_srv(1);
tt_ptr_op(sr_state_get_previous_srv(), OP_NE, NULL);
tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL);
sr_state_set_previous_srv(NULL);
tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL);
tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL);
sr_state_set_current_srv(NULL);
tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL);
tt_ptr_op(sr_state_get_current_srv(), OP_EQ, NULL);
/* And both at once */
test_sr_setup_srv(1);
tt_ptr_op(sr_state_get_previous_srv(), OP_NE, NULL);
tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL);
sr_state_clean_srvs();
tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL);
tt_ptr_op(sr_state_get_current_srv(), OP_EQ, NULL);
/* And do the gets and sets multiple times */
test_sr_setup_srv(1);
tt_ptr_op(sr_state_get_previous_srv(), OP_NE, NULL);
tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL);
sr_state_set_previous_srv(NULL);
sr_state_set_previous_srv(NULL);
tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL);
tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL);
tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL);
tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL);
sr_state_set_current_srv(NULL);
sr_state_set_previous_srv(NULL);
sr_state_set_current_srv(NULL);
tt_ptr_op(sr_state_get_previous_srv(), OP_EQ, NULL);
tt_ptr_op(sr_state_get_current_srv(), OP_EQ, NULL);
}
{
/* Now copy the values across */
test_sr_setup_srv(1);
/* Check that the pointers are non-NULL, and different from each other */
tt_ptr_op(sr_state_get_previous_srv(), OP_NE, NULL);
tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL);
tt_ptr_op(sr_state_get_previous_srv(), OP_NE,
sr_state_get_current_srv());
/* Check that the content is different */
tt_mem_op(sr_state_get_previous_srv(), OP_NE,
sr_state_get_current_srv(), sizeof(sr_srv_t));
/* Set the current to the previous: the protocol goes the other way */
sr_state_set_current_srv(sr_srv_dup(sr_state_get_previous_srv()));
/* Check that the pointers are non-NULL, and different from each other */
tt_ptr_op(sr_state_get_previous_srv(), OP_NE, NULL);
tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL);
tt_ptr_op(sr_state_get_previous_srv(), OP_NE,
sr_state_get_current_srv());
/* Check that the content is the same */
tt_mem_op(sr_state_get_previous_srv(), OP_EQ,
sr_state_get_current_srv(), sizeof(sr_srv_t));
}
{
/* Now copy a value onto itself */
test_sr_setup_srv(1);
/* Check that the pointers are non-NULL, and different from each other */
tt_ptr_op(sr_state_get_previous_srv(), OP_NE, NULL);
tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL);
tt_ptr_op(sr_state_get_previous_srv(), OP_NE,
sr_state_get_current_srv());
/* Take a copy of the old value */
sr_srv_t old_current_srv;
memcpy(&old_current_srv, sr_state_get_current_srv(), sizeof(sr_srv_t));
/* Check that the content is different */
tt_mem_op(sr_state_get_previous_srv(), OP_NE,
sr_state_get_current_srv(), sizeof(sr_srv_t));
/* Set the current to the current: the protocol never replaces an SRV with
* the same value */
sr_state_set_current_srv(sr_srv_dup(sr_state_get_current_srv()));
/* Check that the pointers are non-NULL, and different from each other */
tt_ptr_op(sr_state_get_previous_srv(), OP_NE, NULL);
tt_ptr_op(sr_state_get_current_srv(), OP_NE, NULL);
tt_ptr_op(sr_state_get_previous_srv(), OP_NE,
sr_state_get_current_srv());
/* Check that the content is different between current and previous */
tt_mem_op(sr_state_get_previous_srv(), OP_NE,
sr_state_get_current_srv(), sizeof(sr_srv_t));
/* Check that the content is the same as the old content */
tt_mem_op(&old_current_srv, OP_EQ,
sr_state_get_current_srv(), sizeof(sr_srv_t));
}
/* I don't think we can say "expect a BUG()" in our tests. */
#if 0
{
/* Now copy a value onto itself without sr_srv_dup().
* This should fail with a BUG() warning. */
test_sr_setup_srv(1);
sr_state_set_current_srv(sr_state_get_current_srv());
sr_state_set_previous_srv(sr_state_get_previous_srv());
}
#endif
done:
return;
sr_state_free();
}
static void
@ -1293,7 +1492,8 @@ struct testcase_t sr_tests[] = {
{ "sr_compute_srv", test_sr_compute_srv, TT_FORK, NULL, NULL },
{ "sr_get_majority_srv_from_votes", test_sr_get_majority_srv_from_votes,
TT_FORK, NULL, NULL },
{ "utils", test_utils, TT_FORK, NULL, NULL },
{ "utils_general", test_utils_general, TT_FORK, NULL, NULL },
{ "utils_auth", test_utils_auth, TT_FORK, NULL, NULL },
{ "state_transition", test_state_transition, TT_FORK, NULL, NULL },
{ "state_update", test_state_update, TT_FORK,
NULL, NULL },