[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
[tor-commits] [obfsproxy/master] Introduce allocate-memory-or-crash helpers and use them throughout the code base.
commit 6ba1db07b516763ef62df933fb46d88470d6a805
Author: Zack Weinberg <zackw@xxxxxxxxx>
Date: Mon Jul 18 12:23:20 2011 -0700
Introduce allocate-memory-or-crash helpers and use them throughout the code base.
---
src/crypt.c | 26 ++++++---------
src/crypt.h | 11 +++---
src/main.c | 34 ++++----------------
src/network.c | 25 ++++++---------
src/protocols/dummy.c | 6 +--
src/protocols/obfs2.c | 34 +++++---------------
src/socks.c | 28 ++++++----------
src/socks.h | 2 +-
src/test/unittest_obfs2.c | 3 +-
src/test/unittest_socks.c | 5 ---
src/util.c | 76 +++++++++++++++++++++++++++++++++++++++++---
src/util.h | 13 ++++++++
12 files changed, 138 insertions(+), 125 deletions(-)
diff --git a/src/crypt.c b/src/crypt.c
index 6be9726..8d1d607 100644
--- a/src/crypt.c
+++ b/src/crypt.c
@@ -4,9 +4,11 @@
#define CRYPT_PRIVATE
#include "crypt.h"
+#include "util.h"
#include <assert.h>
#include <fcntl.h>
+#include <limits.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
@@ -74,14 +76,12 @@ struct digest_t {
};
/**
- Returns a new SHA256 digest container, or NULL on failure.
+ Returns a new SHA256 digest container.
*/
digest_t *
digest_new(void)
{
- digest_t *d = malloc(sizeof(digest_t));
- if (!d)
- return NULL;
+ digest_t *d = xmalloc(sizeof(digest_t));
SHA256_Init(&d->ctx);
return d;
}
@@ -89,7 +89,7 @@ digest_new(void)
/**
Updates the contents of the SHA256 container 'd' with the first
'len' bytes of 'buf'.
-*/
+*/
void
digest_update(digest_t *d, const uchar *buf, size_t len)
{
@@ -118,9 +118,7 @@ struct digest_t {
digest_t *
digest_new(void)
{
- digest_t *d = malloc(sizeof(digest_t));
- if (!d)
- return NULL;
+ digest_t *d = xmalloc(sizeof(digest_t));
sha256_init(&d->ctx);
return d;
}
@@ -156,19 +154,15 @@ digest_free(digest_t *d)
/**
Initializes the AES cipher with 'key'.
-*/
+*/
crypt_t *
crypt_new(const uchar *key, size_t keylen)
{
crypt_t *k;
- if (keylen < AES_BLOCK_SIZE)
- return NULL;
-
- k = calloc(1, sizeof(crypt_t));
- if (k == NULL)
- return NULL;
- AES_set_encrypt_key(key, 128, &k->key);
+ assert(keylen == AES_BLOCK_SIZE);
+ k = xzalloc(sizeof(crypt_t));
+ AES_set_encrypt_key(key, AES_BLOCK_SIZE * CHAR_BIT, &k->key);
return k;
}
diff --git a/src/crypt.h b/src/crypt.h
index d87b1de..3f1e4df 100644
--- a/src/crypt.h
+++ b/src/crypt.h
@@ -21,7 +21,7 @@ int initialize_crypto(void);
/** Clean up global crypto state */
void cleanup_crypto(void);
-/** Return a newly allocated digest state, or NULL on failure. */
+/** Return a newly allocated digest state; cannot fail. */
digest_t *digest_new(void);
/** Add n bytes from b to the digest state. */
void digest_update(digest_t *, const uchar *b, size_t n);
@@ -31,10 +31,11 @@ size_t digest_getdigest(digest_t *, uchar *b, size_t n);
/** Clear and free a digest state */
void digest_free(digest_t *);
-/** Return a new stream cipher state taking key and IV from the data provided.
- * The data length must be exactly 32 */
-crypt_t *crypt_new(const uchar *, size_t);
-void crypt_set_iv(crypt_t *key, const uchar *iv, size_t ivlen);
+/** Return a new stream cipher state using 'key' as the symmetric key.
+ * The data length must be exactly 16 bytes. Cannot fail. */
+crypt_t *crypt_new(const uchar *key, size_t);
+/* Set the IV of a stream-cipher state. Cannot fail. */
+void crypt_set_iv(crypt_t *, const uchar *iv, size_t ivlen);
/** Encrypt n bytes of data in the buffer b, in place. */
void stream_crypt(crypt_t *, uchar *b, size_t n);
diff --git a/src/main.c b/src/main.c
index f41d502..21c4feb 100644
--- a/src/main.c
+++ b/src/main.c
@@ -181,13 +181,6 @@ handle_obfsproxy_args(const char **argv)
return i;
}
-static void
-die_oom(void)
-{
- log_warn("Memory allocation failed: %s",strerror(errno));
- exit(1);
-}
-
int
main(int argc, const char **argv)
{
@@ -214,7 +207,6 @@ main(int argc, const char **argv)
int start;
int end;
int n_options;
- void *realloc_temp;
int i;
/* The number of protocols. */
@@ -222,7 +214,7 @@ main(int argc, const char **argv)
/* An array which holds the position in argv of the command line
options for each protocol. */
unsigned int *protocols=NULL;
- /* keeps track of allocated space for the protocols array */
+ /* keeps track of allocated space for the protocols array */
unsigned int n_alloc;
if (argc < 2) {
@@ -235,9 +227,7 @@ main(int argc, const char **argv)
/** Handle optional obfsproxy arguments. */
start_of_protocols = handle_obfsproxy_args(&argv[1]);
- protocols = calloc(sizeof(int), (n_protocols+1));
- if (!protocols)
- die_oom();
+ protocols = xzalloc((n_protocols + 1) * sizeof(int));
n_alloc = n_protocols+1;
/* Populate protocols and calculate n_protocols. */
@@ -249,10 +239,7 @@ main(int argc, const char **argv)
/* Do we need to expand the protocols array? */
if (n_alloc <= n_protocols) {
n_alloc *= 2;
- realloc_temp = realloc(protocols, sizeof(int)*(n_alloc));
- if (!realloc_temp)
- die_oom();
- protocols = realloc_temp;
+ protocols = xrealloc(protocols, sizeof(int)*(n_alloc));
}
}
}
@@ -271,13 +258,9 @@ main(int argc, const char **argv)
that point to arrays carrying the options of the protocols.
Finally, we allocate enough space on the n_options_array so that
we can put the number of options there.
- */
- protocol_options = calloc(sizeof(char**), n_protocols);
- if (!protocol_options)
- die_oom();
- n_options_array = calloc(sizeof(int), n_protocols);
- if (!n_options_array)
- die_oom();
+ */
+ protocol_options = xzalloc(n_protocols * sizeof(char**));
+ n_options_array = xzalloc(n_protocols * sizeof(int));
/* Iterate through protocols. */
for (i=0;i<n_protocols;i++) {
@@ -304,10 +287,7 @@ main(int argc, const char **argv)
/* Allocate space for the array carrying the options of this
protocol. */
- protocol_options[actual_protocols-1] =
- calloc(sizeof(char*), (n_options));
- if (!protocol_options[actual_protocols-1])
- die_oom();
+ protocol_options[actual_protocols-1] = xzalloc(n_options * sizeof(char*));
/* Write the number of options to the correct place in n_options_array[]. */
n_options_array[actual_protocols-1] = n_options;
diff --git a/src/network.c b/src/network.c
index 365c61a..fb770c9 100644
--- a/src/network.c
+++ b/src/network.c
@@ -99,10 +99,15 @@ close_all_connections(void)
assert(!n_connections);
}
/**
- This function spawns a listener according to the 'proto_params'.
+ This function spawns a listener configured according to the
+ provided 'protocol_params_t' object'. Returns the listener on
+ success, NULL on fail.
- Returns the listener on success, NULL on fail.
+ If it succeeds, the new listener object takes ownership of the
+ protocol_params_t object provided; if it fails, the protocol_params_t
+ object is deallocated.
*/
+
listener_t *
listener_new(struct event_base *base,
protocol_params_t *proto_params)
@@ -110,14 +115,8 @@ listener_new(struct event_base *base,
const unsigned flags =
LEV_OPT_CLOSE_ON_FREE|LEV_OPT_CLOSE_ON_EXEC|LEV_OPT_REUSEABLE;
- listener_t *lsn = calloc(1, sizeof(listener_t));
- if (!lsn) {
- if (proto_params)
- free(proto_params);
- return NULL;
- }
+ listener_t *lsn = xzalloc(sizeof(listener_t));
- /** If we don't have a connection dll, create one now. */
lsn->proto_params = proto_params;
lsn->listener = evconnlistener_new_bind(base, simple_listener_cb, lsn,
@@ -132,6 +131,7 @@ listener_new(struct event_base *base,
return NULL;
}
+ /** If we don't have a connection dll, create one now. */
dll_append(&listener_list, &lsn->dll_node);
return lsn;
@@ -189,15 +189,12 @@ simple_listener_cb(struct evconnlistener *evcl,
{
listener_t *lsn = arg;
struct event_base *base;
- conn_t *conn = calloc(1, sizeof(conn_t));
+ conn_t *conn = xzalloc(sizeof(conn_t));
n_connections++; /* If we call conn_free() later on error, it will decrement
* n_connections. Therefore, we had better increment it at
* the start. */
- if (!conn)
- goto err;
-
log_debug("Got a connection attempt.");
conn->mode = lsn->proto_params->mode;
@@ -211,8 +208,6 @@ simple_listener_cb(struct evconnlistener *evcl,
if (conn->mode == LSN_SOCKS_CLIENT) {
/* Construct SOCKS state. */
conn->socks_state = socks_state_new();
- if (!conn->socks_state)
- goto err;
}
/* New bufferevent to wrap socket we received. */
diff --git a/src/protocols/dummy.c b/src/protocols/dummy.c
index f121725..d82818d 100644
--- a/src/protocols/dummy.c
+++ b/src/protocols/dummy.c
@@ -28,9 +28,7 @@ static struct protocol_params_t *
dummy_init(int n_options, const char *const *options)
{
struct protocol_params_t *params
- = calloc(1, sizeof(struct protocol_params_t));
- if (!params)
- return NULL;
+ = xzalloc(sizeof(struct protocol_params_t));
if (parse_and_set_options(n_options, options, params) < 0) {
free(params);
@@ -101,7 +99,7 @@ static struct protocol_t *
dummy_create(struct protocol_params_t *params)
{
/* Dummy needs no per-connection protocol-specific state. */
- struct protocol_t *proto = calloc(1, sizeof(struct protocol_t));
+ struct protocol_t *proto = xzalloc(sizeof(struct protocol_t));
proto->vtable = &dummy_vtable;
return proto;
}
diff --git a/src/protocols/obfs2.c b/src/protocols/obfs2.c
index 41a2614..c42d1c4 100644
--- a/src/protocols/obfs2.c
+++ b/src/protocols/obfs2.c
@@ -36,9 +36,7 @@ static struct protocol_params_t *
obfs2_init(int n_options, const char *const *options)
{
struct protocol_params_t *params
- = calloc(1, sizeof(struct protocol_params_t));
- if (!params)
- return NULL;
+ = xzalloc(sizeof(struct protocol_params_t));
if (parse_and_set_options(n_options, options, params) < 0) {
usage();
@@ -85,8 +83,9 @@ parse_and_set_options(int n_options, const char *const *options,
if (got_ss)
return -1;
/* this is freed in proto_params_free() */
- params->shared_secret = strdup(*options+16);
params->shared_secret_len = strlen(*options+16);
+ params->shared_secret = xmemdup(*options+16,
+ params->shared_secret_len + 1);
got_ss=1;
} else {
log_warn("obfs2: Unknown argument.");
@@ -163,7 +162,7 @@ seed_nonzero(const uchar *seed)
/**
Derive and return key of type 'keytype' from the seeds currently set in
- 'state'. Returns NULL on failure.
+ 'state'.
*/
static crypt_t *
derive_key(void *s, const char *keytype)
@@ -202,7 +201,7 @@ derive_key(void *s, const char *keytype)
/**
Derive and return padding key of type 'keytype' from the seeds
- currently set in state 's'. Returns NULL on failure.
+ currently set in state 's'.
*/
static crypt_t *
derive_padding_key(void *s, const uchar *seed,
@@ -250,12 +249,10 @@ derive_padding_key(void *s, const uchar *seed,
static struct protocol_t *
obfs2_create(protocol_params_t *params)
{
- obfs2_protocol_t *proto = calloc(1, sizeof(obfs2_protocol_t));
+ obfs2_protocol_t *proto = xzalloc(sizeof(obfs2_protocol_t));
uchar *seed;
const char *send_pad_type;
- if (!proto)
- return NULL;
proto->state = ST_WAIT_FOR_KEY;
proto->we_are_initiator = params->is_initiator;
if (proto->we_are_initiator) {
@@ -275,10 +272,6 @@ obfs2_create(protocol_params_t *params)
if (params->shared_secret) {
/* ASN we must say in spec that we hash command line shared secret. */
digest_t *c = digest_new();
- if (!c) {
- free(proto);
- return NULL;
- }
digest_update(c, (uchar*)params->shared_secret, params->shared_secret_len);
digest_getdigest(c, proto->secret_seed, SHARED_SECRET_LENGTH);
digest_free(c);
@@ -286,11 +279,6 @@ obfs2_create(protocol_params_t *params)
/* Derive the key for what we're sending */
proto->send_padding_crypto = derive_padding_key(proto, seed, send_pad_type);
- if (proto->send_padding_crypto == NULL) {
- free(proto);
- return NULL;
- }
-
proto->super.vtable = &obfs2_vtable;
return &proto->super;
}
@@ -420,7 +408,7 @@ obfs2_send(struct protocol_t *s,
Helper: called after reciving our partner's setup message. Initializes all
keys. Returns 0 on success, -1 on failure.
*/
-static int
+static void
init_crypto(void *s)
{
obfs2_protocol_t *state = s;
@@ -447,11 +435,6 @@ init_crypto(void *s)
state->recv_crypto = derive_key(state, recv_keytype);
state->recv_padding_crypto =
derive_padding_key(state, recv_seed, recv_pad_keytype);
-
- if (state->send_crypto && state->recv_crypto && state->recv_padding_crypto)
- return 0;
- else
- return -1;
}
/* Called when we receive data in an evbuffer 'source': deobfuscates that data
@@ -491,8 +474,7 @@ obfs2_recv(struct protocol_t *s, struct evbuffer *source,
memcpy(other_seed, buf, OBFUSCATE_SEED_LENGTH);
/* Now we can set up all the keys from the seed */
- if (init_crypto(state) < 0)
- return RECV_BAD;
+ init_crypto(state);
/* Decrypt the next 8 bytes */
stream_crypt(state->recv_padding_crypto, buf+OBFUSCATE_SEED_LENGTH, 8);
diff --git a/src/socks.c b/src/socks.c
index e89812f..b67455f 100644
--- a/src/socks.c
+++ b/src/socks.c
@@ -43,24 +43,19 @@
typedef unsigned char uchar;
/**
- Creates a new SOCKS state.
-
- Returns a 'socks_state_t' on success, NULL on fail.
+ Creates a new 'socks_state_t' object.
*/
socks_state_t *
socks_state_new(void)
{
- socks_state_t *state = calloc(1, sizeof(socks_state_t));
- if (!state)
- return NULL;
+ socks_state_t *state = xzalloc(sizeof(socks_state_t));
state->state = ST_WAITING;
-
return state;
}
/**
Deallocates memory of socks_state_t 's'.
-*/
+*/
void
socks_state_free(socks_state_t *s)
{
@@ -318,8 +313,8 @@ socks5_handle_negotiation(struct evbuffer *source,
struct evbuffer *dest, socks_state_t *state)
{
unsigned int found_noauth, i;
-
uchar nmethods;
+ uchar methods[0xFF];
evbuffer_copyout(source, &nmethods, 1);
@@ -329,25 +324,22 @@ socks5_handle_negotiation(struct evbuffer *source,
evbuffer_drain(source, 1);
- uchar *p;
- /* XXX user controlled malloc(). range should be: 0x00-0xff */
- p = malloc(nmethods);
- if (!p) {
- log_warn("malloc failed!");
+ /* this should be impossible, but we check it anyway for great defensiveness */
+ if (nmethods > 0xFF) {
+ log_warn("too many methods!");
return SOCKS_BROKEN;
}
- if (evbuffer_remove(source, p, nmethods) < 0)
+
+ if (evbuffer_remove(source, methods, nmethods) < 0)
assert(0);
for (found_noauth=0, i=0; i<nmethods ; i++) {
- if (p[i] == SOCKS5_METHOD_NOAUTH) {
+ if (methods[i] == SOCKS5_METHOD_NOAUTH) {
found_noauth = 1;
break;
}
}
- free(p);
-
return socks5_do_negotiation(dest,found_noauth);
}
diff --git a/src/socks.h b/src/socks.h
index e2c20b5..db10cb5 100644
--- a/src/socks.h
+++ b/src/socks.h
@@ -30,7 +30,7 @@ enum socks_ret {
enum socks_ret handle_socks(struct evbuffer *source,
struct evbuffer *dest,
socks_state_t *socks_state);
-socks_state_t *socks_state_new(void);
+socks_state_t *socks_state_new(void); /* cannot fail */
void socks_state_free(socks_state_t *s);
enum socks_status_t socks_state_get_status(const socks_state_t *state);
diff --git a/src/test/unittest_obfs2.c b/src/test/unittest_obfs2.c
index 01ce501..961bfd2 100644
--- a/src/test/unittest_obfs2.c
+++ b/src/test/unittest_obfs2.c
@@ -125,8 +125,7 @@ static const char *const options_server[] =
static void *
setup_obfs2_state(const struct testcase_t *unused)
{
- struct test_obfs2_state *s = calloc(1, sizeof(struct test_obfs2_state));
- tt_assert(s);
+ struct test_obfs2_state *s = xzalloc(sizeof(struct test_obfs2_state));
s->proto_params_client =
proto_params_init(ALEN(options_client), options_client);
diff --git a/src/test/unittest_socks.c b/src/test/unittest_socks.c
index 001c266..4d95666 100644
--- a/src/test/unittest_socks.c
+++ b/src/test/unittest_socks.c
@@ -31,7 +31,6 @@ test_socks_socks5_send_negotiation(void *data)
socks_state_t *state;
state = socks_state_new();
- tt_assert(state);
/* First test:
Only one method: NOAUTH.
@@ -143,7 +142,6 @@ test_socks_socks5_request(void *data)
socks_state_t *state;
state = socks_state_new();
- tt_assert(state);
const uint32_t addr_ipv4 = htonl(0x7f000001); /* 127.0.0.1 */
const uint8_t addr_ipv6[16] = {0,13,0,1,0,5,0,14,0,10,0,5,0,14,0,0}; /* d:1:5:e:a:5:e:0 */
@@ -308,7 +306,6 @@ test_socks_socks5_request_reply(void *data)
socks_state_t *state;
state = socks_state_new();
- tt_assert(state);
state->parsereq.af = AF_INET;
strcpy(state->parsereq.addr, "127.0.0.1");
@@ -415,7 +412,6 @@ test_socks_socks4_request(void *data)
socks_state_t *state;
state = socks_state_new();
- tt_assert(state);
/* First test:
Correct SOCKS4 req packet with nothing in the optional field. */
@@ -552,7 +548,6 @@ test_socks_socks4_request_reply(void *data)
socks_state_t *state;
state = socks_state_new();
- tt_assert(state);
state->parsereq.af = AF_INET;
strcpy(state->parsereq.addr, "127.0.0.1");
diff --git a/src/util.c b/src/util.c
index cab4bf1..6a0e4a0 100644
--- a/src/util.c
+++ b/src/util.c
@@ -5,6 +5,7 @@
#include "util.h"
#include <assert.h>
+#include <errno.h>
#include <fcntl.h>
#include <limits.h>
#include <stdio.h>
@@ -22,6 +23,73 @@
/** Any size_t larger than this amount is likely to be an underflow. */
#define SIZE_T_CEILING (SIZE_MAX/2 - 16)
+/**************************** Memory Allocation ******************************/
+
+static void __attribute__((noreturn))
+die_oom(void)
+{
+ log_warn("Memory allocation failed: %s",strerror(errno));
+ exit(1);
+}
+
+void *
+xmalloc(size_t size)
+{
+ void *result;
+
+ assert(size < SIZE_T_CEILING);
+
+ /* Some malloc() implementations return NULL when the input argument
+ is zero. We don't bother detecting whether the implementation we're
+ being compiled for does that, because it should hardly ever come up,
+ and avoiding it unconditionally does no harm. */
+ if (size == 0)
+ size = 1;
+
+ result = malloc(size);
+ if (result == NULL)
+ die_oom();
+
+ return result;
+}
+
+void *
+xrealloc(void *ptr, size_t size)
+{
+ void *result;
+ assert (size < SIZE_T_CEILING);
+ if (size == 0)
+ size = 1;
+
+ result = realloc(ptr, size);
+ if (result == NULL)
+ die_oom();
+
+ return result;
+}
+
+void *
+xzalloc(size_t size)
+{
+ void *result = xmalloc(size);
+ memset(result, 0, size);
+ return result;
+}
+
+void *
+xmemdup(const void *ptr, size_t size)
+{
+ void *copy = xmalloc(size);
+ memcpy(copy, ptr, size);
+ return copy;
+}
+
+char *
+xstrdup(const char *s)
+{
+ return xmemdup(s, strlen(s) + 1);
+}
+
/************************ Obfsproxy Network Routines *************************/
/**
@@ -45,10 +113,8 @@ resolve_address_port(const char *address,
struct evutil_addrinfo *ai = NULL;
struct evutil_addrinfo ai_hints;
int result = -1, ai_res;
- char *a = strdup(address), *cp;
+ char *a = xstrdup(address), *cp;
const char *portstr;
- if (!a)
- return -1;
if ((cp = strchr(a, ':'))) {
portstr = cp+1;
@@ -78,10 +144,8 @@ resolve_address_port(const char *address,
log_warn("No result for address %s", address);
goto done;
}
- struct sockaddr *addr = malloc(ai->ai_addrlen);
- memcpy(addr, ai->ai_addr, ai->ai_addrlen);
- *addr_out = addr;
*addrlen_out = ai->ai_addrlen;
+ *addr_out = xmemdup(ai->ai_addr, ai->ai_addrlen);
result = 0;
done:
diff --git a/src/util.h b/src/util.h
index b526dbf..d909b53 100644
--- a/src/util.h
+++ b/src/util.h
@@ -16,6 +16,19 @@ struct sockaddr;
struct event_base;
struct evdns_base;
+/***** Memory allocation. *****/
+
+/* Because this isn't Tor and functions named "tor_whatever" would be
+ confusing, I am instead following the GNU convention of naming
+ allocate-memory-or-crash functions "xwhatever". Also, at this time
+ I do not see a need for a free() wrapper. */
+
+void *xmalloc(size_t size) __attribute__((malloc)); /* does not clear memory */
+void *xzalloc(size_t size) __attribute__((malloc)); /* clears memory */
+void *xrealloc(void *ptr, size_t size);
+void *xmemdup(const void *ptr, size_t size) __attribute__((malloc));
+char *xstrdup(const char *s) __attribute__((malloc));
+
/***** Network functions stuff. *****/
int resolve_address_port(const char *address,
_______________________________________________
tor-commits mailing list
tor-commits@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits