From 666b09669187daad7107b340dd7dde50070a2feb Mon Sep 17 00:00:00 2001 From: Jardel Weyrich Date: Sat, 18 Dec 2010 01:07:27 -0200 Subject: [PATCH 1/4] Detect and handle more allocation failures. --- evdns.c | 4 ++++ evutil.c | 7 ++++++- http.c | 44 +++++++++++++++++++++++++++++++++++++++++--- select.c | 9 +++++++-- 4 files changed, 58 insertions(+), 6 deletions(-) diff --git a/evdns.c b/evdns.c index a1636a14..d085bac8 100644 --- a/evdns.c +++ b/evdns.c @@ -3069,6 +3069,10 @@ search_request_new(struct evdns_base *base, struct evdns_request *handle, } EVUTIL_ASSERT(handle->search_origname == NULL); handle->search_origname = mm_strdup(name); + if (handle->search_origname == NULL) { + /* XXX Should we dealloc req? If yes, how? */ + return NULL; + } handle->search_state = base->global_search_state; handle->search_flags = flags; base->global_search_state->refcount++; diff --git a/evutil.c b/evutil.c index b769acc2..1d2c0fc9 100644 --- a/evutil.c +++ b/evutil.c @@ -958,8 +958,13 @@ addrinfo_from_hostent(const struct hostent *ent, res = evutil_addrinfo_append(res, ai); } - if (res && ((hints->ai_flags & EVUTIL_AI_CANONNAME) && ent->h_name)) + if (res && ((hints->ai_flags & EVUTIL_AI_CANONNAME) && ent->h_name)) { res->ai_canonname = mm_strdup(ent->h_name); + if (res->ai_canonname == NULL) { + evutil_freeaddrinfo(res); + return NULL; + } + } return res; } diff --git a/http.c b/http.c index f8de3b37..60ea21ee 100644 --- a/http.c +++ b/http.c @@ -2491,6 +2491,10 @@ evhttp_response_code(struct evhttp_request *req, int code, const char *reason) if (reason == NULL) reason = evhttp_response_phrase_internal(code); req->response_code_line = mm_strdup(reason); + if (req->response_code_line == NULL) { + event_warn("%s: strdup", __func__); + /* XXX what else can we do? */ + } } void @@ -3280,6 +3284,11 @@ evhttp_set_cb(struct evhttp *http, const char *uri, } http_cb->what = mm_strdup(uri); + if (http_cb->what == NULL) { + event_warn("%s: strdup", __func__); + mm_free(http_cb); + return (-3); + } http_cb->cb = cb; http_cb->cbarg = cbarg; @@ -3911,6 +3920,10 @@ parse_authority(struct evhttp_uri *uri, char *s, char *eos) EVUTIL_ASSERT(eos); if (eos == s) { uri->host = mm_strdup(""); + if (uri->host == NULL) { + event_warn("%s: strdup", __func__); + return -1; + } return 0; } @@ -3922,6 +3935,10 @@ parse_authority(struct evhttp_uri *uri, char *s, char *eos) return -1; *cp++ = '\0'; uri->userinfo = mm_strdup(s); + if (uri->userinfo == NULL) { + event_warn("%s: strdup", __func__); + return -1; + } } else { cp = s; } @@ -3949,6 +3966,10 @@ parse_authority(struct evhttp_uri *uri, char *s, char *eos) return -1; } uri->host = mm_malloc(eos-cp+1); + if (uri->host == NULL) { + event_warn("%s: malloc", __func__); + return -1; + } memcpy(uri->host, cp, eos-cp); uri->host[eos-cp] = '\0'; return 0; @@ -4039,7 +4060,10 @@ evhttp_uri_parse(const char *source_uri) if (token && scheme_ok(readp,token)) { *token = '\0'; uri->scheme = mm_strdup(readp); - + if (uri->scheme == NULL) { + event_err(1, "%s: strdup", __func__); + goto err; + } readp = token+1; /* eat : */ } @@ -4096,11 +4120,25 @@ evhttp_uri_parse(const char *source_uri) EVUTIL_ASSERT(path); uri->path = mm_strdup(path); + if (uri->path == NULL) { + event_err(1, "%s: strdup", __func__); + goto err; + } - if (query) + if (query) { uri->query = mm_strdup(query); - if (fragment) + if (uri->query == NULL) { + event_err(1, "%s: strdup", __func__); + goto err; + } + } + if (fragment) { uri->fragment = mm_strdup(fragment); + if (uri->fragment == NULL) { + event_err(1, "%s: strdup", __func__); + goto err; + } + } mm_free(readbuf); diff --git a/select.c b/select.c index 527462ca..993a4e60 100644 --- a/select.c +++ b/select.c @@ -99,7 +99,10 @@ select_init(struct event_base *base) if (!(sop = mm_calloc(1, sizeof(struct selectop)))) return (NULL); - select_resize(sop, howmany(32 + 1, NFDBITS)*sizeof(fd_mask)); + if (select_resize(sop, howmany(32 + 1, NFDBITS)*sizeof(fd_mask))) { + mm_free(sop); + return (NULL); + } evsig_init(base); @@ -198,8 +201,10 @@ select_resize(struct selectop *sop, int fdsz) if ((readset_in = mm_realloc(sop->event_readset_in, fdsz)) == NULL) goto error; sop->event_readset_in = readset_in; - if ((writeset_in = mm_realloc(sop->event_writeset_in, fdsz)) == NULL) + if ((writeset_in = mm_realloc(sop->event_writeset_in, fdsz)) == NULL) { + mm_free(readset_in); goto error; + } sop->event_writeset_in = writeset_in; sop->resize_out_sets = 1; From 3f8d22a123aef2698ce2e7d6ca1ad96d13760cef Mon Sep 17 00:00:00 2001 From: Jardel Weyrich Date: Sat, 18 Dec 2010 02:40:22 -0200 Subject: [PATCH 2/4] Use event_err() only if the failure is truly unrecoverable. --- http.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/http.c b/http.c index 60ea21ee..53b58b41 100644 --- a/http.c +++ b/http.c @@ -1110,7 +1110,7 @@ evhttp_connection_set_local_address(struct evhttp_connection *evcon, if (evcon->bind_address) mm_free(evcon->bind_address); if ((evcon->bind_address = mm_strdup(address)) == NULL) - event_err(1, "%s: strdup", __func__); + event_warn("%s: strdup", __func__); } void @@ -4032,14 +4032,14 @@ evhttp_uri_parse(const char *source_uri) struct evhttp_uri *uri = mm_calloc(1, sizeof(struct evhttp_uri)); if (uri == NULL) { - event_err(1, "%s: calloc", __func__); + event_warn("%s: calloc", __func__); goto err; } uri->port = -1; readbuf = mm_strdup(source_uri); if (readbuf == NULL) { - event_err(1, "%s: strdup", __func__); + event_warn("%s: strdup", __func__); goto err; } @@ -4061,7 +4061,7 @@ evhttp_uri_parse(const char *source_uri) *token = '\0'; uri->scheme = mm_strdup(readp); if (uri->scheme == NULL) { - event_err(1, "%s: strdup", __func__); + event_warn("%s: strdup", __func__); goto err; } readp = token+1; /* eat : */ @@ -4121,21 +4121,21 @@ evhttp_uri_parse(const char *source_uri) EVUTIL_ASSERT(path); uri->path = mm_strdup(path); if (uri->path == NULL) { - event_err(1, "%s: strdup", __func__); + event_warn("%s: strdup", __func__); goto err; } if (query) { uri->query = mm_strdup(query); if (uri->query == NULL) { - event_err(1, "%s: strdup", __func__); + event_warn("%s: strdup", __func__); goto err; } } if (fragment) { uri->fragment = mm_strdup(fragment); if (uri->fragment == NULL) { - event_err(1, "%s: strdup", __func__); + event_warn("%s: strdup", __func__); goto err; } } From 83e805a41554a9e69ab7d2d75e56c30f3147ffa6 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 7 Jan 2011 13:18:09 -0500 Subject: [PATCH 3/4] Handle resize failures in the select backend better. --- select.c | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/select.c b/select.c index 993a4e60..8327518e 100644 --- a/select.c +++ b/select.c @@ -100,7 +100,9 @@ select_init(struct event_base *base) return (NULL); if (select_resize(sop, howmany(32 + 1, NFDBITS)*sizeof(fd_mask))) { - mm_free(sop); + /* select_resize might have left this around. */ + if (sop->event_readset_in) + mm_free(sop->event_readset_in); return (NULL); } @@ -131,11 +133,14 @@ select_dispatch(struct event_base *base, struct timeval *tv) size_t sz = sop->event_fdsz; if (!(readset_out = mm_realloc(sop->event_readset_out, sz))) return (-1); + sop->event_readset_out = readset_out; if (!(writeset_out = mm_realloc(sop->event_writeset_out, sz))) { - mm_free(readset_out); + /* We don't free readset_out here, since it was + * already successfully reallocated. The next time + * we call select_dispatch, the realloc will be a + * no-op. */ return (-1); } - sop->event_readset_out = readset_out; sop->event_writeset_out = writeset_out; sop->resize_out_sets = 0; } @@ -188,7 +193,6 @@ select_dispatch(struct event_base *base, struct timeval *tv) return (0); } - static int select_resize(struct selectop *sop, int fdsz) { @@ -202,7 +206,12 @@ select_resize(struct selectop *sop, int fdsz) goto error; sop->event_readset_in = readset_in; if ((writeset_in = mm_realloc(sop->event_writeset_in, fdsz)) == NULL) { - mm_free(readset_in); + /* Note that this will leave event_readset_in expanded. + * That's okay; we wouldn't want to free it, since that would + * change the semantics of select_resize from "expand the + * readset_in and writeset_in, or return -1" to "expand the + * *set_in members, or trash them and return -1." + */ goto error; } sop->event_writeset_in = writeset_in; From 0c0ec0be2ba977b8133294e27b076a65a2d65711 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Wed, 12 Jan 2011 20:28:47 -0500 Subject: [PATCH 4/4] Correctly free selectop fields when select_resize fails in select_init --- select.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/select.c b/select.c index 8327518e..94956318 100644 --- a/select.c +++ b/select.c @@ -90,6 +90,7 @@ const struct eventop selectops = { }; static int select_resize(struct selectop *sop, int fdsz); +static void select_free_selectop(struct selectop *sop); static void * select_init(struct event_base *base) @@ -100,9 +101,7 @@ select_init(struct event_base *base) return (NULL); if (select_resize(sop, howmany(32 + 1, NFDBITS)*sizeof(fd_mask))) { - /* select_resize might have left this around. */ - if (sop->event_readset_in) - mm_free(sop->event_readset_in); + select_free_selectop(sop); return (NULL); } @@ -306,11 +305,8 @@ select_del(struct event_base *base, int fd, short old, short events, void *p) } static void -select_dealloc(struct event_base *base) +select_free_selectop(struct selectop *sop) { - struct selectop *sop = base->evbase; - - evsig_dealloc(base); if (sop->event_readset_in) mm_free(sop->event_readset_in); if (sop->event_writeset_in) @@ -323,3 +319,11 @@ select_dealloc(struct event_base *base) memset(sop, 0, sizeof(struct selectop)); mm_free(sop); } + +static void +select_dealloc(struct event_base *base) +{ + evsig_dealloc(base); + + select_free_selectop(base->evbase); +}