From 6f50d20ba362315dea4ecaf44336c3aaa4284975 Mon Sep 17 00:00:00 2001 From: Matt Fleming Date: Thu, 15 Nov 2012 20:39:30 +0000 Subject: bufprintf: Add va_end() for our va_copy() According to the stdarg(3) man page each invocation of va_copy() should be paired with an invocation of va_end(). Cc: Erwan Velu Signed-off-by: Matt Fleming --- com32/lib/bufprintf.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/com32/lib/bufprintf.c b/com32/lib/bufprintf.c index 939bcec3..d2812311 100644 --- a/com32/lib/bufprintf.c +++ b/com32/lib/bufprintf.c @@ -17,8 +17,10 @@ int vbufprintf(struct print_buf *buf, const char *format, va_list ap) char *newbuf; newbuf = realloc(buf->buf, newsize); - if (!newbuf) - return -1; + if (!newbuf) { + rv = -1; + goto bail; + } buf->buf = newbuf; buf->size = newsize; @@ -26,6 +28,8 @@ int vbufprintf(struct print_buf *buf, const char *format, va_list ap) rv = vsnprintf(buf->buf + buf->len, buf->size - buf->len, format, ap2); buf->len += rv; +bail: + va_end(ap2); return rv; } -- cgit From 3da9e71f9ffa0ca968ccfcf26d536908d198ef5c Mon Sep 17 00:00:00 2001 From: Matt Fleming Date: Thu, 15 Nov 2012 20:56:09 +0000 Subject: asprintf: Ensure we always call va_end(ap) There's currently the potential for us to exit early from asprintf() without calling va_end(ap). Rearrange things so that we always make the call. Signed-off-by: Matt Fleming --- com32/lib/asprintf.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/com32/lib/asprintf.c b/com32/lib/asprintf.c index ef5b4b2f..eab20118 100644 --- a/com32/lib/asprintf.c +++ b/com32/lib/asprintf.c @@ -21,9 +21,10 @@ int asprintf(char **bufp, const char *format, ...) *bufp = p = malloc(bytes); if (!p) - return -1; + rv = -1; + else + rv = vsnprintf(p, bytes, format, ap); - rv = vsnprintf(p, bytes, format, ap); va_end(ap); return rv; -- cgit From c6ce504b8b79729a4e0acc2bebc5e497e4098e60 Mon Sep 17 00:00:00 2001 From: Matt Fleming Date: Thu, 15 Nov 2012 21:27:59 +0000 Subject: vesa: Fix double close() bug in vesacon_load_background() We always call fclose() on 'fp' if fopen() was successful, so delete the extraneous fclose() call in read_jpeg_file(). Signed-off-by: Matt Fleming --- com32/lib/sys/vesa/background.c | 1 - 1 file changed, 1 deletion(-) diff --git a/com32/lib/sys/vesa/background.c b/com32/lib/sys/vesa/background.c index 93577461..15e90895 100644 --- a/com32/lib/sys/vesa/background.c +++ b/com32/lib/sys/vesa/background.c @@ -205,7 +205,6 @@ static int read_jpeg_file(FILE * fp, uint8_t * header, int len) unsigned int bytes_per_row[1]; rv = floadfile(fp, &jpeg_file, &length_of_file, header, len); - fclose(fp); if (rv) goto err; -- cgit From 39acf04ff0eda2e86b53187bee31c6c67e058491 Mon Sep 17 00:00:00 2001 From: Matt Fleming Date: Thu, 15 Nov 2012 21:32:18 +0000 Subject: chainboot: Delete extraneous free() in chainboot_file() We don't need to call free(buf) if we're jumping to the 'bail' label because 'buf' is always free'd there. Signed-off-by: Matt Fleming --- com32/elflink/ldlinux/chainboot.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/com32/elflink/ldlinux/chainboot.c b/com32/elflink/ldlinux/chainboot.c index 4a4a2e1a..ff19c530 100644 --- a/com32/elflink/ldlinux/chainboot.c +++ b/com32/elflink/ldlinux/chainboot.c @@ -54,10 +54,8 @@ void chainboot_file(const char *file, uint32_t type) goto bail; rv = open_file(file, &fd); - if (rv == -1) { - free(buf); + if (rv == -1) goto bail; - } reg.eax.l = max; reg.ebx.l = 0; -- cgit From d43c57d945f42dd0dbaca747a646a769e49082da Mon Sep 17 00:00:00 2001 From: Matt Fleming Date: Fri, 16 Nov 2012 14:23:45 +0000 Subject: hdt-cli: Correct malloc() size argument We need to be allocating sizeof(char *) (4) not sizeof(char) (1) for 'new_argv'. Cc: Erwan Velu Signed-off-by: Matt Fleming --- com32/hdt/hdt-cli.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/com32/hdt/hdt-cli.c b/com32/hdt/hdt-cli.c index 7542da83..216b6bde 100644 --- a/com32/hdt/hdt-cli.c +++ b/com32/hdt/hdt-cli.c @@ -649,7 +649,7 @@ static void exec_command(char *line, struct s_hardware *hardware) if ((current_module->nomodule == true) && ( module != NULL)) { dprintf("CLI_DEBUG exec: Reworking arguments with argc=%d\n",argc); char **new_argv=NULL; - new_argv=malloc((argc + 2)*sizeof(char)); + new_argv=malloc((argc + 2)*sizeof(char *)); for (int argc_iter=0; argc_iter Date: Tue, 27 Nov 2012 16:03:36 +0000 Subject: screensize: Dereference pointers when checking cols/rows Dereference 'rows' and 'cols' to check whether the data they point to is zero, which would indicate the screen size is bogus, instead of checking if they point to NULL. Signed-off-by: Matt Fleming --- com32/lib/sys/screensize.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/com32/lib/sys/screensize.c b/com32/lib/sys/screensize.c index 340227cd..bcd4496c 100644 --- a/com32/lib/sys/screensize.c +++ b/com32/lib/sys/screensize.c @@ -14,7 +14,7 @@ int getscreensize(int fd, int *rows, int *cols) *rows = fp->o.rows; *cols = fp->o.cols; - if (!rows || !cols) { + if (!*rows || !*cols) { errno = ENOTTY; return -1; } -- cgit From 73bf7c135dfce1d0194de6677ed85495c64e300f Mon Sep 17 00:00:00 2001 From: Matt Fleming Date: Tue, 27 Nov 2012 16:11:26 +0000 Subject: pxe: Don't leak inode on timeout Signed-off-by: Matt Fleming --- core/fs/pxe/pxe.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/core/fs/pxe/pxe.c b/core/fs/pxe/pxe.c index 6f490ce8..3cc64973 100644 --- a/core/fs/pxe/pxe.c +++ b/core/fs/pxe/pxe.c @@ -796,8 +796,10 @@ static void __pxe_searchdir(const char *filename, struct file *file) sendreq: timeout = *timeout_ptr++; - if (!timeout) + if (!timeout) { + free_socket(inode); return; /* No file available... */ + } oldtime = jiffies(); socket->tftp_remoteip = ip; -- cgit From 30ebd4f6bc83fa4832b658705d4020cb82dfdaea Mon Sep 17 00:00:00 2001 From: Matt Fleming Date: Tue, 27 Nov 2012 16:19:17 +0000 Subject: module: Fix memory leak in spawn_load() If for some reason we fail to load a module then we need to free the memory allocated to that module. Signed-off-by: Matt Fleming --- com32/lib/sys/module/exec.c | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/com32/lib/sys/module/exec.c b/com32/lib/sys/module/exec.c index 29d0a2fd..9ccab36e 100644 --- a/com32/lib/sys/module/exec.c +++ b/com32/lib/sys/module/exec.c @@ -194,8 +194,10 @@ int spawn_load(const char *name, int argc, char **argv) return -1; if (get_module_type(module) == EXEC_MODULE) { - if (!argc || !argv || strcmp(argv[0], name)) - return -1; + if (!argc || !argv || strcmp(argv[0], name)) { + res = -1; + goto out; + } } if (!strcmp(cur_module->name, module->name)) { @@ -218,10 +220,8 @@ int spawn_load(const char *name, int argc, char **argv) } res = module_load(module); - if (res != 0) { - _module_unload(module); - return res; - } + if (res != 0) + goto out; type = get_module_type(module); prev_module = cur_module; @@ -259,14 +259,16 @@ int spawn_load(const char *name, int argc, char **argv) cur_module = prev_module; res = module_unload(module); - if (res != 0) { - return res; - } + if (res != 0) + goto out; return ((unsigned int)ret_val & 0xFF); } - return 0; +out: + if (res) + _module_unload(module); + return res; } void exec_term(void) -- cgit From 6f4575c2ad3950af53bcdfd40fe2cce6171179fe Mon Sep 17 00:00:00 2001 From: Matt Fleming Date: Tue, 27 Nov 2012 16:25:37 +0000 Subject: module: Fix off-by-one error in findpath() We need to make sure that 'path' still has enough space to write the trailing NUL-byte. Without this patch it's possible to write a NUL-byte past the end of the on-stack buffer. Signed-off-by: Matt Fleming --- com32/lib/sys/module/common.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/com32/lib/sys/module/common.c b/com32/lib/sys/module/common.c index 30c57b4b..dfbdf617 100644 --- a/com32/lib/sys/module/common.c +++ b/com32/lib/sys/module/common.c @@ -71,7 +71,7 @@ FILE *findpath(char *name) p = PATH; again: i = 0; - while (*p && *p != ':' && i < FILENAME_MAX) { + while (*p && *p != ':' && i < FILENAME_MAX - 1) { path[i++] = *p++; } @@ -79,7 +79,7 @@ again: p++; n = name; - while (*n && i < FILENAME_MAX) + while (*n && i < FILENAME_MAX - 1) path[i++] = *n++; path[i] = '\0'; -- cgit From e955e5c00a852883f6972e1a9bc304413ff79627 Mon Sep 17 00:00:00 2001 From: Matt Fleming Date: Tue, 27 Nov 2012 20:12:58 +0000 Subject: core/elflink: Fix off-by-one error We need to remember to allocate space for the terminating NULL in create_args_and_load() otherwise we will write a NUL-byte past the bounds of 'argv[]' to some random part of the stack. Signed-off-by: Matt Fleming --- core/elflink/load_env32.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/core/elflink/load_env32.c b/core/elflink/load_env32.c index 49c5989c..23d6baa1 100644 --- a/core/elflink/load_env32.c +++ b/core/elflink/load_env32.c @@ -170,9 +170,10 @@ int create_args_and_load(char *cmdline) * Generate a copy of argv on the stack as this is * traditionally where process arguments go. * - * argv[0] must be the command name. + * argv[0] must be the command name. Remember to allocate + * space for the sentinel NULL. */ - argv = alloca(argc * sizeof(char *)); + argv = alloca((argc + 1) * sizeof(char *)); for (i = 0, p = cmdline; i < argc; i++) { char *start; -- cgit