From 1f019ceea1ed39286e6bccfb3ff936c22fe0f7c0 Mon Sep 17 00:00:00 2001
From: Matthias Schiffer <mschiffer@universe-factory.net>
Date: Tue, 21 Jun 2016 17:19:10 +0200
Subject: [PATCH 1/1] 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 <mschiffer@universe-factory.net>
---
 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;
+	}
+
+	ret[s.pos] = 0;
 
-	return s.buf;
+	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];
-- 
2.11.0