From 159188d08cba90fcbb9270b4610761d4468b65af Mon Sep 17 00:00:00 2001 From: Jeremy Lakeman Date: Mon, 31 Oct 2016 15:21:22 +1030 Subject: [PATCH] Don't overflow the stack if logging config causes more logging due to using memory diagnostics --- http_server.c | 4 ++-- http_server.h | 2 +- httpd.c | 2 +- log.c | 3 +++ mem.c | 2 +- overlay_buffer.c | 26 -------------------------- rhizome.c | 8 ++++---- rhizome.h | 4 ++-- 8 files changed, 14 insertions(+), 37 deletions(-) diff --git a/http_server.c b/http_server.c index 770d36f9..fea8b4eb 100644 --- a/http_server.c +++ b/http_server.c @@ -1909,8 +1909,8 @@ static void http_server_poll(struct sched_ent *alarm) else abort(); // should not be any other POLL bits set // Any of the above calls could change the phase to DONE. - if (r->phase == DONE && r->free) - r->free(r); // after this, *r is no longer valid + if (r->phase == DONE && r->release) + r->release(r); // after this, *r is no longer valid } /* Copy the array of byte ranges, closing it (converting all ranges to CLOSED) using the supplied diff --git a/http_server.h b/http_server.h index c567c148..f3a91720 100644 --- a/http_server.h +++ b/http_server.h @@ -189,7 +189,7 @@ struct http_request { // The following control the lifetime of this struct. enum http_request_phase { RECEIVE, TRANSMIT, PAUSE, DONE } phase; void (*finalise)(struct http_request *); - void (*free)(void*); + void (*release)(void*); // Identify request from others being run. Monotonic counter feeds it. Only // used for debugging when we write post-.log files for multi-part form // requests. diff --git a/httpd.c b/httpd.c index bd059ebd..e1e74950 100644 --- a/httpd.c +++ b/httpd.c @@ -270,7 +270,7 @@ void httpd_server_poll(struct sched_ent *alarm) request->http.debug = INDIRECT_CONFIG_DEBUG(httpd); request->http.disable_tx = INDIRECT_CONFIG_DEBUG(nohttptx); request->http.finalise = httpd_server_finalise_http_request; - request->http.free = free; + request->http.release = free; request->http.idle_timeout = RHIZOME_IDLE_TIMEOUT; http_request_init(&request->http, sock); } diff --git a/log.c b/log.c index 0e4ee6c2..a629835e 100644 --- a/log.c +++ b/log.c @@ -329,6 +329,8 @@ static void _log_software_version(_log_iterator *it, int level) static int _log_current_config(_log_iterator *it, int level) { if (!cf_limbo) { + // if formatting config causes anything else to be logged, ignore it. + cf_limbo=1; struct cf_om_node *root = NULL; int ret = cf_fmt_config_main(&root, &config); if (ret == CFERROR) { @@ -348,6 +350,7 @@ static int _log_current_config(_log_iterator *it, int level) } cf_om_free_node(&root); it->state->config_logged = 1; + cf_limbo=0; } return 1; } diff --git a/mem.c b/mem.c index 43b60ba8..57a8004d 100644 --- a/mem.c +++ b/mem.c @@ -84,6 +84,6 @@ void *_serval_debug_calloc(unsigned int bytes, unsigned int count, struct __sour void _serval_debug_free(void *p, struct __sourceloc __whence) { + _DEBUGF("free(%p)", p); free(p); - _DEBUGF("free(%p)", p); } diff --git a/overlay_buffer.c b/overlay_buffer.c index dc495d41..d16e6967 100644 --- a/overlay_buffer.c +++ b/overlay_buffer.c @@ -189,33 +189,7 @@ ssize_t _ob_makespace(struct __sourceloc __whence, struct overlay_buffer *b, siz if (newSize>65536 && (newSize&65535)) newSize+=65536-(newSize&65535); DEBUGF(overlaybuffer, "realloc(b->bytes=%p, newSize=%zu)", b->bytes, newSize); - /* XXX OSX realloc() seems to be able to corrupt things if the heap is not happy when calling realloc(), making debugging memory corruption much harder. - So will do a three-stage malloc,bcopy,free to see if we can tease bugs out that way. */ - /* - unsigned char *r=realloc(b->bytes,newSize); - if (!r) return WHY("realloc() failed"); - b->bytes=r; - */ -#ifdef MALLOC_PARANOIA -#warning adding lots of padding to try to catch overruns - if (b->bytes) { - int i; - int corrupt=0; - for(i=0;i<4096;i++) if (b->bytes[b->allocSize+i]!=0xbd) corrupt++; - if (corrupt) { - WHYF("!!!!!! %d corrupted bytes in overrun catch tray", corrupt); - dump("overrun catch tray",&b->bytes[b->allocSize],4096); - sleep_ms(36000000); - } - } - unsigned char *new = emalloc(newSize+4096); - { - int i; - for(i=0;i<4096;i++) new[newSize+i]=0xbd; - } -#else unsigned char *new = emalloc(newSize); -#endif if (!new) return 0; if (b->position) diff --git a/rhizome.c b/rhizome.c index 9a577112..a1178412 100644 --- a/rhizome.c +++ b/rhizome.c @@ -714,8 +714,8 @@ const char *rhizome_payload_status_message_nonnull(enum rhizome_payload_status s void rhizome_bundle_result_free(struct rhizome_bundle_result *resultp) { - if (resultp->free) { - resultp->free((void *)resultp->message); + if (resultp->release) { + resultp->release((void *)resultp->message); } *resultp = INVALID_RHIZOME_BUNDLE_RESULT; } @@ -788,7 +788,7 @@ struct rhizome_bundle_result _rhizome_bundle_result_strdup(struct __sourceloc __ struct rhizome_bundle_result result = INVALID_RHIZOME_BUNDLE_RESULT; result.status = status; result.message = str_edup(message); - result.free = free; + result.release = free; log_rhizome_bundle_result(__whence, result); return result; } @@ -800,7 +800,7 @@ struct rhizome_bundle_result _rhizome_bundle_result_sprintf(struct __sourceloc _ strbuf sb; STRBUF_ALLOCA_FIT(sb, 200, strbuf_va_printf(sb, fmt)); result.message = str_edup(strbuf_str(sb)); - result.free = free; + result.release = free; log_rhizome_bundle_result(__whence, result); return result; } diff --git a/rhizome.h b/rhizome.h index 84f7a81f..785d29af 100644 --- a/rhizome.h +++ b/rhizome.h @@ -410,10 +410,10 @@ const char *rhizome_bundle_status_message_nonnull(enum rhizome_bundle_status); struct rhizome_bundle_result { enum rhizome_bundle_status status; const char *message; - void (*free)(void *); // call r.free(r.message) before destroying r + void (*release)(void *); // call r.release(r.message) before destroying r }; -#define INVALID_RHIZOME_BUNDLE_RESULT ((struct rhizome_bundle_result){ .status = INVALID_RHIZOME_BUNDLE_STATUS, .message = NULL, .free = NULL }) +#define INVALID_RHIZOME_BUNDLE_RESULT ((struct rhizome_bundle_result){ .status = INVALID_RHIZOME_BUNDLE_STATUS, .message = NULL, .release = NULL }) // Call this before discarding a struct rhizome_bundle_result. void rhizome_bundle_result_free(struct rhizome_bundle_result *);