[tor-commits] [tor/master] Stop memcpy'ing uncompressed consensuses when making diffs
nickm at torproject.org
nickm at torproject.org
Wed Oct 31 14:13:23 UTC 2018
commit e014b72b73b2a299068f1ca5b7a22f2bea2f58f8
Author: Nick Mathewson <nickm at torproject.org>
Date: Tue Sep 11 10:09:12 2018 -0400
Stop memcpy'ing uncompressed consensuses when making diffs
---
src/feature/dircache/consdiffmgr.c | 50 +++++++++++++++++++++++---------------
src/feature/dircache/consdiffmgr.h | 5 ++--
src/test/fuzz/fuzz_diff.c | 17 ++++++++++---
src/test/test_consdiffmgr.c | 16 ++++++------
4 files changed, 56 insertions(+), 32 deletions(-)
diff --git a/src/feature/dircache/consdiffmgr.c b/src/feature/dircache/consdiffmgr.c
index 7999df08d..bf3a0ef3c 100644
--- a/src/feature/dircache/consdiffmgr.c
+++ b/src/feature/dircache/consdiffmgr.c
@@ -1387,19 +1387,21 @@ typedef struct consensus_diff_worker_job_t {
} consensus_diff_worker_job_t;
/** Given a consensus_cache_entry_t, check whether it has a label claiming
- * that it was compressed. If so, uncompress its contents into <b>out</b> and
- * set <b>outlen</b> to hold their size. If not, just copy the body into
- * <b>out</b> and set <b>outlen</b> to its length. Return 0 on success,
- * -1 on failure.
- *
- * In all cases, the output is nul-terminated. */
+ * that it was compressed. If so, uncompress its contents into *<b>out</b> and
+ * set <b>outlen</b> to hold their size, and set *<b>owned_out</b> to a pointer
+ * that the caller will need to free. If not, just set *<b>out</b> and
+ * <b>outlen</b> to its extent in memory. Return 0 on success, -1 on failure.
+ **/
STATIC int
-uncompress_or_copy(char **out, size_t *outlen,
- consensus_cache_entry_t *ent)
+uncompress_or_set_ptr(const char **out, size_t *outlen,
+ char **owned_out,
+ consensus_cache_entry_t *ent)
{
const uint8_t *body;
size_t bodylen;
+ *owned_out = NULL;
+
if (consensus_cache_entry_get_body(ent, &body, &bodylen) < 0)
return -1;
@@ -1410,8 +1412,17 @@ uncompress_or_copy(char **out, size_t *outlen,
if (lv_compression)
method = compression_method_get_by_name(lv_compression);
- return tor_uncompress(out, outlen, (const char *)body, bodylen,
+ int rv;
+ if (method == NO_METHOD) {
+ *out = (const char *)body;
+ *outlen = bodylen;
+ rv = 0;
+ } else {
+ rv = tor_uncompress(owned_out, outlen, (const char *)body, bodylen,
method, 1, LOG_WARN);
+ *out = *owned_out;
+ }
+ return rv;
}
/**
@@ -1478,16 +1489,17 @@ consensus_diff_worker_threadfn(void *state_, void *work_)
char *consensus_diff;
{
- char *diff_from_nt = NULL, *diff_to_nt = NULL;
+ const char *diff_from_nt = NULL, *diff_to_nt = NULL;
+ char *owned1 = NULL, *owned2 = NULL;
size_t diff_from_nt_len, diff_to_nt_len;
- if (uncompress_or_copy(&diff_from_nt, &diff_from_nt_len,
- job->diff_from) < 0) {
+ if (uncompress_or_set_ptr(&diff_from_nt, &diff_from_nt_len, &owned1,
+ job->diff_from) < 0) {
return WQ_RPL_REPLY;
}
- if (uncompress_or_copy(&diff_to_nt, &diff_to_nt_len,
- job->diff_to) < 0) {
- tor_free(diff_from_nt);
+ if (uncompress_or_set_ptr(&diff_to_nt, &diff_to_nt_len, &owned2,
+ job->diff_to) < 0) {
+ tor_free(owned1);
return WQ_RPL_REPLY;
}
tor_assert(diff_from_nt);
@@ -1497,11 +1509,11 @@ consensus_diff_worker_threadfn(void *state_, void *work_)
// XXXX inputs again, even though we already have that. Maybe it's time
// XXXX to change the API here?
consensus_diff = consensus_diff_generate(diff_from_nt,
- strlen(diff_from_nt),
+ diff_from_nt_len,
diff_to_nt,
- strlen(diff_to_nt));
- tor_free(diff_from_nt);
- tor_free(diff_to_nt);
+ diff_to_nt_len);
+ tor_free(owned1);
+ tor_free(owned2);
}
if (!consensus_diff) {
/* Couldn't generate consensus; we'll leave the reply blank. */
diff --git a/src/feature/dircache/consdiffmgr.h b/src/feature/dircache/consdiffmgr.h
index 66c3d6500..d6f273cc4 100644
--- a/src/feature/dircache/consdiffmgr.h
+++ b/src/feature/dircache/consdiffmgr.h
@@ -68,8 +68,9 @@ STATIC consensus_cache_entry_t *cdm_cache_lookup_consensus(
STATIC int cdm_entry_get_sha3_value(uint8_t *digest_out,
consensus_cache_entry_t *ent,
const char *label);
-STATIC int uncompress_or_copy(char **out, size_t *outlen,
- consensus_cache_entry_t *ent);
+STATIC int uncompress_or_set_ptr(const char **out, size_t *outlen,
+ char **owned_out,
+ consensus_cache_entry_t *ent);
#endif /* defined(CONSDIFFMGR_PRIVATE) */
#endif /* !defined(TOR_CONSDIFFMGR_H) */
diff --git a/src/test/fuzz/fuzz_diff.c b/src/test/fuzz/fuzz_diff.c
index 8966856be..64aecc8a6 100644
--- a/src/test/fuzz/fuzz_diff.c
+++ b/src/test/fuzz/fuzz_diff.c
@@ -48,18 +48,27 @@ fuzz_main(const uint8_t *stdin_buf, size_t data_size)
size_t c2_len = data_size - c1_len - SEPLEN;
const char *c2 = (const char *)separator + SEPLEN;
+ const char *cp = memchr(c1, 0, c1_len);
+ if (cp)
+ c1_len = cp - c1;
+
+ cp = memchr(c2, 0, c2_len);
+ if (cp)
+ c2_len = cp - c2;
+
char *c3 = consensus_diff_generate(c1, c1_len, c2, c2_len);
if (c3) {
char *c4 = consensus_diff_apply(c1, c1_len, c3, strlen(c3));
tor_assert(c4);
- if (strcmp(c2, c4)) {
- printf("%s\n", escaped(c1));
- printf("%s\n", escaped(c2));
+ int equal = (c2_len == strlen(c4)) && fast_memeq(c2, c4, c2_len);
+ if (! equal) {
+ //printf("%s\n", escaped(c1));
+ //printf("%s\n", escaped(c2));
printf("%s\n", escaped(c3));
printf("%s\n", escaped(c4));
}
- tor_assert(! strcmp(c2, c4));
+ tor_assert(equal);
tor_free(c3);
tor_free(c4);
}
diff --git a/src/test/test_consdiffmgr.c b/src/test/test_consdiffmgr.c
index dc4fea7f6..4b49fdb6a 100644
--- a/src/test/test_consdiffmgr.c
+++ b/src/test/test_consdiffmgr.c
@@ -191,14 +191,15 @@ lookup_apply_and_verify_diff(consensus_flavor_t flav,
consensus_cache_entry_incref(ent);
size_t size;
- char *diff_string = NULL;
- int r = uncompress_or_copy(&diff_string, &size, ent);
+ const char *diff_string = NULL;
+ char *diff_owned = NULL;
+ int r = uncompress_or_set_ptr(&diff_string, &size, &diff_owned, ent);
consensus_cache_entry_decref(ent);
if (diff_string == NULL || r < 0)
return -1;
- char *applied = consensus_diff_apply_(str1, diff_string);
- tor_free(diff_string);
+ char *applied = consensus_diff_apply(str1, strlen(str1), diff_string, size);
+ tor_free(diff_owned);
if (applied == NULL)
return -1;
@@ -298,7 +299,8 @@ test_consdiffmgr_add(void *arg)
(void) arg;
time_t now = approx_time();
- char *body = NULL;
+ const char *body = NULL;
+ char *body_owned = NULL;
consensus_cache_entry_t *ent = NULL;
networkstatus_t *ns_tmp = fake_ns_new(FLAV_NS, now);
@@ -340,7 +342,7 @@ test_consdiffmgr_add(void *arg)
tt_assert(ent);
consensus_cache_entry_incref(ent);
size_t s;
- r = uncompress_or_copy(&body, &s, ent);
+ r = uncompress_or_set_ptr(&body, &s, &body_owned, ent);
tt_int_op(r, OP_EQ, 0);
tt_int_op(s, OP_EQ, 4);
tt_mem_op(body, OP_EQ, "quux", 4);
@@ -353,7 +355,7 @@ test_consdiffmgr_add(void *arg)
networkstatus_vote_free(ns_tmp);
teardown_capture_of_logs();
consensus_cache_entry_decref(ent);
- tor_free(body);
+ tor_free(body_owned);
}
static void
More information about the tor-commits
mailing list