Merge branch 'bug21278_extra_029' into maint-0.2.9

This commit is contained in:
Nick Mathewson 2017-02-15 07:48:30 -05:00
commit 5d88267bf4
6 changed files with 60 additions and 43 deletions

3
changes/bug21278_extras Normal file
View File

@ -0,0 +1,3 @@
o Minor bugfixes (code correctness):
- Repair a couple of (unreachable or harmless) cases of the risky
comparison-by-subtraction pattern that caused bug 21278.

View File

@ -421,16 +421,30 @@ compare_vote_rs(const vote_routerstatus_t *a, const vote_routerstatus_t *b)
b->status.descriptor_digest,
DIGEST_LEN)))
return r;
if ((r = (int)(b->status.published_on - a->status.published_on)))
return r;
/* If we actually reached this point, then the identities and
* the descriptor digests matched, so somebody is making SHA1 collisions.
*/
#define CMP_FIELD(utype, itype, field) do { \
utype aval = (utype) (itype) a->status.field; \
utype bval = (utype) (itype) b->status.field; \
utype u = bval - aval; \
itype r2 = (itype) u; \
if (r2 < 0) { \
return -1; \
} else if (r2 > 0) { \
return 1; \
} \
} while (0)
CMP_FIELD(uint64_t, int64_t, published_on);
if ((r = strcmp(b->status.nickname, a->status.nickname)))
return r;
if ((r = (((int)b->status.addr) - ((int)a->status.addr))))
return r;
if ((r = (((int)b->status.or_port) - ((int)a->status.or_port))))
return r;
if ((r = (((int)b->status.dir_port) - ((int)a->status.dir_port))))
return r;
CMP_FIELD(unsigned, int, addr);
CMP_FIELD(unsigned, int, or_port);
CMP_FIELD(unsigned, int, dir_port);
return 0;
}

View File

@ -1198,48 +1198,48 @@ policies_parse_from_options(const or_options_t *options)
return ret;
}
/** Compare two provided address policy items, and return -1, 0, or 1
/** Compare two provided address policy items, and renturn -1, 0, or 1
* if the first is less than, equal to, or greater than the second. */
static int
cmp_single_addr_policy(addr_policy_t *a, addr_policy_t *b)
single_addr_policy_eq(const addr_policy_t *a, const addr_policy_t *b)
{
int r;
if ((r=((int)a->policy_type - (int)b->policy_type)))
return r;
if ((r=((int)a->is_private - (int)b->is_private)))
return r;
#define CMP_FIELD(field) do { \
if (a->field != b->field) { \
return 0; \
} \
} while (0)
CMP_FIELD(policy_type);
CMP_FIELD(is_private);
/* refcnt and is_canonical are irrelevant to equality,
* they are hash table implementation details */
if ((r=tor_addr_compare(&a->addr, &b->addr, CMP_EXACT)))
return r;
if ((r=((int)a->maskbits - (int)b->maskbits)))
return r;
if ((r=((int)a->prt_min - (int)b->prt_min)))
return r;
if ((r=((int)a->prt_max - (int)b->prt_max)))
return r;
return 0;
return 0;
CMP_FIELD(maskbits);
CMP_FIELD(prt_min);
CMP_FIELD(prt_max);
#undef CMP_FIELD
return 1;
}
/** Like cmp_single_addr_policy() above, but looks at the
* whole set of policies in each case. */
/** As single_addr_policy_eq, but compare every element of two policies.
*/
int
cmp_addr_policies(smartlist_t *a, smartlist_t *b)
addr_policies_eq(const smartlist_t *a, const smartlist_t *b)
{
int r, i;
int i;
int len_a = a ? smartlist_len(a) : 0;
int len_b = b ? smartlist_len(b) : 0;
for (i = 0; i < len_a && i < len_b; ++i) {
if ((r = cmp_single_addr_policy(smartlist_get(a, i), smartlist_get(b, i))))
return r;
}
if (i == len_a && i == len_b)
if (len_a != len_b)
return 0;
if (i < len_a)
return -1;
else
return 1;
for (i = 0; i < len_a; ++i) {
if (! single_addr_policy_eq(smartlist_get(a, i), smartlist_get(b, i)))
return 0;
}
return 1;
}
/** Node in hashtable used to store address policy entries. */
@ -1255,7 +1255,7 @@ static HT_HEAD(policy_map, policy_map_ent_t) policy_root = HT_INITIALIZER();
static inline int
policy_eq(policy_map_ent_t *a, policy_map_ent_t *b)
{
return cmp_single_addr_policy(a->policy, b->policy) == 0;
return single_addr_policy_eq(a->policy, b->policy);
}
/** Return a hashcode for <b>ent</b> */
@ -1306,7 +1306,7 @@ addr_policy_get_canonical_entry(addr_policy_t *e)
HT_INSERT(policy_map, &policy_root, found);
}
tor_assert(!cmp_single_addr_policy(found->policy, e));
tor_assert(single_addr_policy_eq(found->policy, e));
++found->policy->refcnt;
return found->policy;
}

View File

@ -76,7 +76,7 @@ void policy_expand_unspec(smartlist_t **policy);
int policies_parse_from_options(const or_options_t *options);
addr_policy_t *addr_policy_get_canonical_entry(addr_policy_t *ent);
int cmp_addr_policies(smartlist_t *a, smartlist_t *b);
int addr_policies_eq(const smartlist_t *a, const smartlist_t *b);
MOCK_DECL(addr_policy_result_t, compare_tor_addr_to_addr_policy,
(const tor_addr_t *addr, uint16_t port, const smartlist_t *policy));
addr_policy_result_t compare_tor_addr_to_node_policy(const tor_addr_t *addr,

View File

@ -5445,7 +5445,7 @@ router_differences_are_cosmetic(const routerinfo_t *r1, const routerinfo_t *r2)
(r1->contact_info && r2->contact_info &&
strcasecmp(r1->contact_info, r2->contact_info)) ||
r1->is_hibernating != r2->is_hibernating ||
cmp_addr_policies(r1->exit_policy, r2->exit_policy) ||
! addr_policies_eq(r1->exit_policy, r2->exit_policy) ||
(r1->supports_tunnelled_dir_requests !=
r2->supports_tunnelled_dir_requests))
return 0;

View File

@ -304,10 +304,10 @@ test_policies_general(void *arg)
tt_assert(!exit_policy_is_general_exit(policy10));
tt_assert(!exit_policy_is_general_exit(policy11));
tt_assert(cmp_addr_policies(policy, policy2));
tt_assert(cmp_addr_policies(policy, NULL));
tt_assert(!cmp_addr_policies(policy2, policy2));
tt_assert(!cmp_addr_policies(NULL, NULL));
tt_assert(!addr_policies_eq(policy, policy2));
tt_assert(!addr_policies_eq(policy, NULL));
tt_assert(addr_policies_eq(policy2, policy2));
tt_assert(addr_policies_eq(NULL, NULL));
tt_assert(!policy_is_reject_star(policy2, AF_INET, 1));
tt_assert(policy_is_reject_star(policy, AF_INET, 1));