From 1f019ceea1ed39286e6bccfb3ff936c22fe0f7c0 Mon Sep 17 00:00:00 2001 From: Matthias Schiffer Date: Tue, 21 Jun 2016 17:19:10 +0200 Subject: [PATCH] Fix various memory management issues Consistently handle allocation failures. Some functions are changed to return bool or int instead of void to allow returning an error. Also fix a buffer size miscalculation in lua/uloop and use _exit() instead of exit() on errors after forking. Signed-off-by: Matthias Schiffer --- blobmsg.c | 22 ++++++++++++++++------ blobmsg.h | 4 ++-- blobmsg_json.c | 30 +++++++++++++++++++++++------- jshn.c | 4 ++++ json_script.c | 17 +++++++++++++++-- kvlist.c | 11 ++++++++--- kvlist.h | 2 +- lua/uloop.c | 7 +++++-- ustream.c | 7 +++++++ utils.c | 2 ++ 10 files changed, 83 insertions(+), 23 deletions(-) diff --git a/blobmsg.c b/blobmsg.c index 80b984a..1e93376 100644 --- a/blobmsg.c +++ b/blobmsg.c @@ -227,29 +227,38 @@ blobmsg_open_nested(struct blob_buf *buf, const char *name, bool array) return (void *)offset; } -void +int blobmsg_vprintf(struct blob_buf *buf, const char *name, const char *format, va_list arg) { va_list arg2; char cbuf; - int len; + char *sbuf; + int len, ret; va_copy(arg2, arg); len = vsnprintf(&cbuf, sizeof(cbuf), format, arg2); va_end(arg2); - vsprintf(blobmsg_alloc_string_buffer(buf, name, len + 1), format, arg); + sbuf = blobmsg_alloc_string_buffer(buf, name, len + 1); + if (!sbuf) + return -1; + ret = vsprintf(sbuf, format, arg); blobmsg_add_string_buffer(buf); + + return ret; } -void +int blobmsg_printf(struct blob_buf *buf, const char *name, const char *format, ...) { va_list ap; + int ret; va_start(ap, format); - blobmsg_vprintf(buf, name, format, ap); + ret = blobmsg_vprintf(buf, name, format, ap); va_end(ap); + + return ret; } void * @@ -278,7 +287,8 @@ blobmsg_realloc_string_buffer(struct blob_buf *buf, unsigned int maxlen) if (required <= 0) goto out; - blob_buf_grow(buf, required); + if (!blob_buf_grow(buf, required)) + return NULL; attr = blob_next(buf->head); out: diff --git a/blobmsg.h b/blobmsg.h index e58f95d..84997a6 100644 --- a/blobmsg.h +++ b/blobmsg.h @@ -224,8 +224,8 @@ void *blobmsg_alloc_string_buffer(struct blob_buf *buf, const char *name, unsign void *blobmsg_realloc_string_buffer(struct blob_buf *buf, unsigned int maxlen); void blobmsg_add_string_buffer(struct blob_buf *buf); -void blobmsg_vprintf(struct blob_buf *buf, const char *name, const char *format, va_list arg); -void blobmsg_printf(struct blob_buf *buf, const char *name, const char *format, ...) +int blobmsg_vprintf(struct blob_buf *buf, const char *name, const char *format, va_list arg); +int blobmsg_printf(struct blob_buf *buf, const char *name, const char *format, ...) __attribute__((format(printf, 3, 4))); diff --git a/blobmsg_json.c b/blobmsg_json.c index 5713948..2e318b2 100644 --- a/blobmsg_json.c +++ b/blobmsg_json.c @@ -119,15 +119,22 @@ struct strbuf { static bool blobmsg_puts(struct strbuf *s, const char *c, int len) { + size_t new_len; + char *new_buf; + if (len <= 0) return true; if (s->pos + len >= s->len) { - s->len += 16 + len; - s->buf = realloc(s->buf, s->len); - if (!s->buf) + new_len = s->len + 16 + len; + new_buf = realloc(s->buf, new_len); + if (!new_buf) return false; + + s->len = new_len; + s->buf = new_buf; } + memcpy(s->buf + s->pos, c, len); s->pos += len; return true; @@ -290,14 +297,18 @@ char *blobmsg_format_json_with_cb(struct blob_attr *attr, bool list, blobmsg_jso { struct strbuf s; bool array; + char *ret; s.len = blob_len(attr); - s.buf = malloc(s.len); s.pos = 0; s.custom_format = cb; s.priv = priv; s.indent = false; + s.buf = malloc(s.len); + if (!s.buf) + return NULL; + if (indent >= 0) { s.indent = true; s.indent_level = indent; @@ -316,8 +327,13 @@ char *blobmsg_format_json_with_cb(struct blob_attr *attr, bool list, blobmsg_jso return NULL; } - s.buf = realloc(s.buf, s.pos + 1); - s.buf[s.pos] = 0; + ret = realloc(s.buf, s.pos + 1); + if (!ret) { + free(s.buf); + return NULL; + } - return s.buf; + ret[s.pos] = 0; + + return ret; } diff --git a/jshn.c b/jshn.c index e2d9022..4989099 100644 --- a/jshn.c +++ b/jshn.c @@ -338,6 +338,10 @@ int main(int argc, char **argv) for (i = 0; environ[i]; i++); vars = calloc(i, sizeof(*vars)); + if (!vars) { + fprintf(stderr, "%m\n"); + return -1; + } for (i = 0; environ[i]; i++) { char *c; diff --git a/json_script.c b/json_script.c index b5d414d..463aac8 100644 --- a/json_script.c +++ b/json_script.c @@ -45,6 +45,9 @@ json_script_file_from_blobmsg(const char *name, void *data, int len) name_len = strlen(name) + 1; f = calloc_a(sizeof(*f) + len, &new_name, name_len); + if (!f) + return NULL; + memcpy(f->data, data, len); if (name) f->avl.key = strcpy(new_name, name); @@ -426,12 +429,15 @@ static int eval_string(struct json_call *call, struct blob_buf *buf, const char char c = '%'; dest = blobmsg_alloc_string_buffer(buf, name, 1); + if (!dest) + return -1; + next = alloca(strlen(pattern) + 1); strcpy(next, pattern); for (str = next; str; str = next) { const char *cur; - char *end; + char *end, *new_buf; int cur_len = 0; bool cur_var = var; @@ -464,7 +470,14 @@ static int eval_string(struct json_call *call, struct blob_buf *buf, const char cur_len = end - str; } - dest = blobmsg_realloc_string_buffer(buf, len + cur_len + 1); + new_buf = blobmsg_realloc_string_buffer(buf, len + cur_len + 1); + if (!new_buf) { + /* Make eval_string return -1 */ + var = true; + break; + } + + dest = new_buf; memcpy(dest + len, cur, cur_len); len += cur_len; } diff --git a/kvlist.c b/kvlist.c index e0a8acb..a7b6ea0 100644 --- a/kvlist.c +++ b/kvlist.c @@ -71,20 +71,25 @@ bool kvlist_delete(struct kvlist *kv, const char *name) return !!node; } -void kvlist_set(struct kvlist *kv, const char *name, const void *data) +bool kvlist_set(struct kvlist *kv, const char *name, const void *data) { struct kvlist_node *node; char *name_buf; int len = kv->get_len(kv, data); - kvlist_delete(kv, name); - node = calloc_a(sizeof(struct kvlist_node) + len, &name_buf, strlen(name) + 1); + if (!node) + return false; + + kvlist_delete(kv, name); + memcpy(node->data, data, len); node->avl.key = strcpy(name_buf, name); avl_insert(&kv->avl, &node->avl); + + return true; } void kvlist_free(struct kvlist *kv) diff --git a/kvlist.h b/kvlist.h index 0d35b46..d59ff9e 100644 --- a/kvlist.h +++ b/kvlist.h @@ -45,7 +45,7 @@ struct kvlist_node { void kvlist_init(struct kvlist *kv, int (*get_len)(struct kvlist *kv, const void *data)); void kvlist_free(struct kvlist *kv); void *kvlist_get(struct kvlist *kv, const char *name); -void kvlist_set(struct kvlist *kv, const char *name, const void *data); +bool kvlist_set(struct kvlist *kv, const char *name, const void *data); bool kvlist_delete(struct kvlist *kv, const char *name); int kvlist_strlen(struct kvlist *kv, const void *data); diff --git a/lua/uloop.c b/lua/uloop.c index 782b5a5..c5dd12f 100644 --- a/lua/uloop.c +++ b/lua/uloop.c @@ -325,9 +325,12 @@ static int ul_process(lua_State *L) int argn = lua_objlen(L, -3); int envn = lua_objlen(L, -2); char** argp = malloc(sizeof(char*) * (argn + 2)); - char** envp = malloc(sizeof(char*) * envn + 1); + char** envp = malloc(sizeof(char*) * (envn + 1)); int i = 1; + if (!argp || !envp) + _exit(-1); + argp[0] = (char*) lua_tostring(L, -4); for (i = 1; i <= argn; i++) { lua_rawgeti(L, -3, i); @@ -344,7 +347,7 @@ static int ul_process(lua_State *L) envp[i - 1] = NULL; execve(*argp, argp, envp); - exit(-1); + _exit(-1); } lua_getglobal(L, "__uloop_cb"); diff --git a/ustream.c b/ustream.c index e7ee9f0..d36ce08 100644 --- a/ustream.c +++ b/ustream.c @@ -65,6 +65,9 @@ static int ustream_alloc_default(struct ustream *s, struct ustream_buf_list *l) return -1; buf = malloc(sizeof(*buf) + l->buffer_len + s->string_data); + if (!buf) + return -1; + ustream_init_buf(buf, l->buffer_len); ustream_add_buf(l, buf); @@ -490,6 +493,8 @@ int ustream_vprintf(struct ustream *s, const char *format, va_list arg) return ustream_write_buffered(s, buf, maxlen, wr); } else { buf = malloc(maxlen + 1); + if (!buf) + return 0; wr = vsnprintf(buf, maxlen + 1, format, arg); wr = ustream_write(s, buf, wr, false); free(buf); @@ -517,6 +522,8 @@ int ustream_vprintf(struct ustream *s, const char *format, va_list arg) return wr; buf = malloc(maxlen + 1); + if (!buf) + return wr; maxlen = vsnprintf(buf, maxlen + 1, format, arg); wr = ustream_write_buffered(s, buf + wr, maxlen - wr, wr); free(buf); diff --git a/utils.c b/utils.c index 8fd19f4..91dd71e 100644 --- a/utils.c +++ b/utils.c @@ -43,6 +43,8 @@ void *__calloc_a(size_t len, ...) va_end(ap1); ptr = calloc(1, alloc_len); + if (!ptr) + return NULL; alloc_len = 0; foreach_arg(ap, cur_addr, cur_len, &ret, len) { *cur_addr = &ptr[alloc_len];