[tor-commits] [tor/master] In relay_digest_matches(), use stack instead of heap.
nickm at torproject.org
nickm at torproject.org
Fri Feb 16 01:56:22 UTC 2018
commit 91c63aae8497bc9de6533daae8f927ca09f96fd2
Author: Nick Mathewson <nickm at torproject.org>
Date: Wed Jan 24 12:33:13 2018 -0500
In relay_digest_matches(), use stack instead of heap.
We'd been using crypto_digest_dup() and crypto_digest_assign() here,
but they aren't necessary. Instead we can just use the stack to
store the previous state of the SHA_CTX and avoid a malloc/free pair.
Closes ticket 24914.
---
changes/bug24914 | 3 +++
configure.ac | 4 ++++
src/common/crypto.c | 28 ++++++++++++++++++++++++++++
src/common/crypto.h | 11 +++++++++++
src/or/relay.c | 16 +++++++++-------
5 files changed, 55 insertions(+), 7 deletions(-)
diff --git a/changes/bug24914 b/changes/bug24914
new file mode 100644
index 000000000..ea441fd38
--- /dev/null
+++ b/changes/bug24914
@@ -0,0 +1,3 @@
+ o Minor features (performance):
+ - Avoid a needless call to malloc() when processing an incoming
+ relay cell. Closes ticket 24914.
diff --git a/configure.ac b/configure.ac
index 3cb187b0e..0ba4c4652 100644
--- a/configure.ac
+++ b/configure.ac
@@ -834,6 +834,10 @@ AC_CHECK_MEMBERS([SSL.state], , ,
[#include <openssl/ssl.h>
])
+AC_CHECK_SIZEOF(SHA_CTX, , [AC_INCLUDES_DEFAULT()
+#include <openssl/sha.h>
+])
+
dnl Define the set of checks for KIST scheduler support.
AC_DEFUN([CHECK_KIST_SUPPORT],[
dnl KIST needs struct tcp_info and for certain members to exist.
diff --git a/src/common/crypto.c b/src/common/crypto.c
index 107b53ad2..880f92b4d 100644
--- a/src/common/crypto.c
+++ b/src/common/crypto.c
@@ -1822,6 +1822,7 @@ crypto_digest_alloc_bytes(digest_algorithm_t alg)
/* Gives the length of crypto_digest_t through the end of the field 'd' */
#define END_OF_FIELD(f) (offsetof(crypto_digest_t, f) + \
STRUCT_FIELD_SIZE(crypto_digest_t, f))
+
switch (alg) {
case DIGEST_SHA1:
return END_OF_FIELD(d.sha1);
@@ -2007,6 +2008,33 @@ crypto_digest_dup(const crypto_digest_t *digest)
return tor_memdup(digest, alloc_bytes);
}
+/** Temporarily save the state of <b>digest</b> in <b>checkpoint</b>.
+ * Asserts that <b>digest</b> is a SHA1 digest object.
+ */
+void
+crypto_digest_checkpoint(crypto_digest_checkpoint_t *checkpoint,
+ const crypto_digest_t *digest)
+{
+ tor_assert(digest->algorithm == DIGEST_SHA1);
+ /* The optimizer should turn this into a constant... */
+ const size_t bytes = crypto_digest_alloc_bytes(DIGEST_SHA1);
+ /* ... and remove this assertion entirely. */
+ tor_assert(bytes <= sizeof(checkpoint->mem));
+ memcpy(checkpoint->mem, digest, bytes);
+}
+
+/** Restore the state of <b>digest</b> from <b>checkpoint</b>.
+ * Asserts that <b>digest</b> is a SHA1 digest object. Requires that the
+ * state was previously stored with crypto_digest_checkpoint() */
+void
+crypto_digest_restore(crypto_digest_t *digest,
+ const crypto_digest_checkpoint_t *checkpoint)
+{
+ tor_assert(digest->algorithm == DIGEST_SHA1);
+ const size_t bytes = crypto_digest_alloc_bytes(DIGEST_SHA1);
+ memcpy(digest, checkpoint->mem, bytes);
+}
+
/** Replace the state of the digest object <b>into</b> with the state
* of the digest object <b>from</b>. Requires that 'into' and 'from'
* have the same digest type.
diff --git a/src/common/crypto.h b/src/common/crypto.h
index 3caa23773..852ab9ba7 100644
--- a/src/common/crypto.h
+++ b/src/common/crypto.h
@@ -98,6 +98,13 @@ typedef struct crypto_digest_t crypto_digest_t;
typedef struct crypto_xof_t crypto_xof_t;
typedef struct crypto_dh_t crypto_dh_t;
+#define DIGEST_CHECKPOINT_BYTES (SIZEOF_VOID_P + SIZEOF_SHA_CTX)
+/** Structure used to temporarily save the a digest object. Only implemented
+ * for SHA1 digest for now. */
+typedef struct crypto_digest_checkpoint_t {
+ uint8_t mem[DIGEST_CHECKPOINT_BYTES];
+} crypto_digest_checkpoint_t;
+
/* global state */
int crypto_early_init(void) ATTR_WUR;
int crypto_global_init(int hardwareAccel,
@@ -235,6 +242,10 @@ void crypto_digest_add_bytes(crypto_digest_t *digest, const char *data,
void crypto_digest_get_digest(crypto_digest_t *digest,
char *out, size_t out_len);
crypto_digest_t *crypto_digest_dup(const crypto_digest_t *digest);
+void crypto_digest_checkpoint(crypto_digest_checkpoint_t *checkpoint,
+ const crypto_digest_t *digest);
+void crypto_digest_restore(crypto_digest_t *digest,
+ const crypto_digest_checkpoint_t *checkpoint);
void crypto_digest_assign(crypto_digest_t *into,
const crypto_digest_t *from);
void crypto_hmac_sha256(char *hmac_out,
diff --git a/src/or/relay.c b/src/or/relay.c
index b1b99526d..aa53cda77 100644
--- a/src/or/relay.c
+++ b/src/or/relay.c
@@ -151,9 +151,9 @@ relay_digest_matches(crypto_digest_t *digest, cell_t *cell)
{
uint32_t received_integrity, calculated_integrity;
relay_header_t rh;
- crypto_digest_t *backup_digest=NULL;
+ crypto_digest_checkpoint_t backup_digest;
- backup_digest = crypto_digest_dup(digest);
+ crypto_digest_checkpoint(&backup_digest, digest);
relay_header_unpack(&rh, cell->payload);
memcpy(&received_integrity, rh.integrity, 4);
@@ -167,19 +167,21 @@ relay_digest_matches(crypto_digest_t *digest, cell_t *cell)
crypto_digest_add_bytes(digest, (char*) cell->payload, CELL_PAYLOAD_SIZE);
crypto_digest_get_digest(digest, (char*) &calculated_integrity, 4);
+ int rv = 1;
+
if (calculated_integrity != received_integrity) {
// log_fn(LOG_INFO,"Recognized=0 but bad digest. Not recognizing.");
// (%d vs %d).", received_integrity, calculated_integrity);
/* restore digest to its old form */
- crypto_digest_assign(digest, backup_digest);
+ crypto_digest_restore(digest, &backup_digest);
/* restore the relay header */
memcpy(rh.integrity, &received_integrity, 4);
relay_header_pack(cell->payload, &rh);
- crypto_digest_free(backup_digest);
- return 0;
+ rv = 0;
}
- crypto_digest_free(backup_digest);
- return 1;
+
+ memwipe(&backup_digest, 0, sizeof(backup_digest));
+ return rv;
}
/** Apply <b>cipher</b> to CELL_PAYLOAD_SIZE bytes of <b>in</b>
More information about the tor-commits
mailing list