From 81beeec429f6bb8b0323e418a6aeef1a17b0cefa Mon Sep 17 00:00:00 2001 From: Pavel Borzenkov Date: Mon, 31 Oct 2011 22:53:36 +0400 Subject: [PATCH 1/5] cmd: Fix coding style in cmd.c Before the next patches, fix coding style of the affected functions. Signed-off-by: Pavel Borzenkov Signed-off-by: Stefan Hajnoczi --- cmd.c | 164 ++++++++++++++++++++++++++++------------------------------ 1 file changed, 80 insertions(+), 84 deletions(-) diff --git a/cmd.c b/cmd.c index f77897e00a..a6e3ef4b17 100644 --- a/cmd.c +++ b/cmd.c @@ -45,13 +45,11 @@ compare(const void *a, const void *b) ((const cmdinfo_t *)b)->name); } -void -add_command( - const cmdinfo_t *ci) +void add_command(const cmdinfo_t *ci) { - cmdtab = realloc((void *)cmdtab, ++ncmds * sizeof(*cmdtab)); - cmdtab[ncmds - 1] = *ci; - qsort(cmdtab, ncmds, sizeof(*cmdtab), compare); + cmdtab = realloc((void *)cmdtab, ++ncmds * sizeof(*cmdtab)); + cmdtab[ncmds - 1] = *ci; + qsort(cmdtab, ncmds, sizeof(*cmdtab), compare); } static int @@ -122,16 +120,15 @@ find_command( return NULL; } -void -add_user_command(char *optarg) +void add_user_command(char *optarg) { - ncmdline++; - cmdline = realloc(cmdline, sizeof(char*) * (ncmdline)); - if (!cmdline) { - perror("realloc"); - exit(1); - } - cmdline[ncmdline-1] = optarg; + ncmdline++; + cmdline = realloc(cmdline, ncmdline * sizeof(char *)); + if (!cmdline) { + perror("realloc"); + exit(1); + } + cmdline[ncmdline-1] = optarg; } static int @@ -160,45 +157,44 @@ static void prep_fetchline(void *opaque) static char *get_prompt(void); -void -command_loop(void) +void command_loop(void) { - int c, i, j = 0, done = 0, fetchable = 0, prompted = 0; - char *input; - char **v; - const cmdinfo_t *ct; + int c, i, j = 0, done = 0, fetchable = 0, prompted = 0; + char *input; + char **v; + const cmdinfo_t *ct; - for (i = 0; !done && i < ncmdline; i++) { - input = strdup(cmdline[i]); - if (!input) { - fprintf(stderr, - _("cannot strdup command '%s': %s\n"), - cmdline[i], strerror(errno)); - exit(1); - } - v = breakline(input, &c); - if (c) { - ct = find_command(v[0]); - if (ct) { - if (ct->flags & CMD_FLAG_GLOBAL) - done = command(ct, c, v); - else { - j = 0; - while (!done && (j = args_command(j))) - done = command(ct, c, v); - } - } else - fprintf(stderr, _("command \"%s\" not found\n"), - v[0]); - } - doneline(input, v); - } - if (cmdline) { - free(cmdline); - return; + for (i = 0; !done && i < ncmdline; i++) { + input = strdup(cmdline[i]); + if (!input) { + fprintf(stderr, _("cannot strdup command '%s': %s\n"), + cmdline[i], strerror(errno)); + exit(1); + } + v = breakline(input, &c); + if (c) { + ct = find_command(v[0]); + if (ct) { + if (ct->flags & CMD_FLAG_GLOBAL) { + done = command(ct, c, v); + } else { + j = 0; + while (!done && (j = args_command(j))) { + done = command(ct, c, v); + } + } + } else { + fprintf(stderr, _("command \"%s\" not found\n"), v[0]); + } } + doneline(input, v); + } + if (cmdline) { + free(cmdline); + return; + } - while (!done) { + while (!done) { if (!prompted) { printf("%s", get_prompt()); fflush(stdout); @@ -212,22 +208,24 @@ command_loop(void) if (!fetchable) { continue; } - if ((input = fetchline()) == NULL) - break; - v = breakline(input, &c); - if (c) { - ct = find_command(v[0]); - if (ct) - done = command(ct, c, v); - else - fprintf(stderr, _("command \"%s\" not found\n"), - v[0]); - } - doneline(input, v); + input = fetchline(); + if (input == NULL) { + break; + } + v = breakline(input, &c); + if (c) { + ct = find_command(v[0]); + if (ct) { + done = command(ct, c, v); + } else { + fprintf(stderr, _("command \"%s\" not found\n"), v[0]); + } + } + doneline(input, v); prompted = 0; fetchable = 0; - } + } qemu_aio_set_fd_handler(STDIN_FILENO, NULL, NULL, NULL, NULL, NULL); } @@ -331,29 +329,27 @@ static char *qemu_strsep(char **input, const char *delim) return result; } -char ** -breakline( - char *input, - int *count) +char **breakline(char *input, int *count) { - int c = 0; - char *p; - char **rval = calloc(sizeof(char *), 1); + int c = 0; + char *p; + char **rval = calloc(sizeof(char *), 1); - while (rval && (p = qemu_strsep(&input, " ")) != NULL) { - if (!*p) - continue; - c++; - rval = realloc(rval, sizeof(*rval) * (c + 1)); - if (!rval) { - c = 0; - break; - } - rval[c - 1] = p; - rval[c] = NULL; - } - *count = c; - return rval; + while (rval && (p = qemu_strsep(&input, " ")) != NULL) { + if (!*p) { + continue; + } + c++; + rval = realloc(rval, sizeof(*rval) * (c + 1)); + if (!rval) { + c = 0; + break; + } + rval[c - 1] = p; + rval[c] = NULL; + } + *count = c; + return rval; } void From ba7806ad92a2f6b1625cfa67d44dc1b71e3be44e Mon Sep 17 00:00:00 2001 From: Pavel Borzenkov Date: Mon, 31 Oct 2011 22:53:37 +0400 Subject: [PATCH 2/5] cmd: Fix potential NULL pointer dereference Signed-off-by: Pavel Borzenkov Signed-off-by: Stefan Hajnoczi --- cmd.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/cmd.c b/cmd.c index a6e3ef4b17..75415d86a2 100644 --- a/cmd.c +++ b/cmd.c @@ -47,7 +47,7 @@ compare(const void *a, const void *b) void add_command(const cmdinfo_t *ci) { - cmdtab = realloc((void *)cmdtab, ++ncmds * sizeof(*cmdtab)); + cmdtab = g_realloc((void *)cmdtab, ++ncmds * sizeof(*cmdtab)); cmdtab[ncmds - 1] = *ci; qsort(cmdtab, ncmds, sizeof(*cmdtab), compare); } @@ -122,12 +122,7 @@ find_command( void add_user_command(char *optarg) { - ncmdline++; - cmdline = realloc(cmdline, ncmdline * sizeof(char *)); - if (!cmdline) { - perror("realloc"); - exit(1); - } + cmdline = g_realloc(cmdline, ++ncmdline * sizeof(char *)); cmdline[ncmdline-1] = optarg; } @@ -190,7 +185,7 @@ void command_loop(void) doneline(input, v); } if (cmdline) { - free(cmdline); + g_free(cmdline); return; } From 47e8dd8fe9d83e8b51d40c2b87d7983bd0a78206 Mon Sep 17 00:00:00 2001 From: Pavel Borzenkov Date: Mon, 31 Oct 2011 22:53:38 +0400 Subject: [PATCH 3/5] cmd: Fix potential memory leak Signed-off-by: Pavel Borzenkov Signed-off-by: Stefan Hajnoczi --- cmd.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/cmd.c b/cmd.c index 75415d86a2..0806e18ce0 100644 --- a/cmd.c +++ b/cmd.c @@ -329,16 +329,21 @@ char **breakline(char *input, int *count) int c = 0; char *p; char **rval = calloc(sizeof(char *), 1); + char **tmp; while (rval && (p = qemu_strsep(&input, " ")) != NULL) { if (!*p) { continue; } c++; - rval = realloc(rval, sizeof(*rval) * (c + 1)); - if (!rval) { + tmp = realloc(rval, sizeof(*rval) * (c + 1)); + if (!tmp) { + free(rval); + rval = NULL; c = 0; break; + } else { + rval = tmp; } rval[c - 1] = p; rval[c] = NULL; From 8af42882a51c632a14d77277df0740f1aa8c958a Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Fri, 4 Nov 2011 11:10:01 +0100 Subject: [PATCH 4/5] readline: Fix buffer overrun on re-add to history readline_hist_add() moves the history entry to the end of history. It uses memmove() to move rs->history[idx + 1..] to rs->history[idx..]. However, its size argument is off by two array elements, so it writes one element beyond rs->history[], and reads two. On my system, this clobbers rs->hist_entry and the hole right after it. Since the function assigns to rs->hist_entry in time, the bug has no ill effects for me. Spotted by Coverity. Signed-off-by: Markus Armbruster Signed-off-by: Stefan Hajnoczi --- readline.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/readline.c b/readline.c index 6a3160aba5..a6c0039ad2 100644 --- a/readline.c +++ b/readline.c @@ -236,7 +236,7 @@ static void readline_hist_add(ReadLineState *rs, const char *cmdline) new_entry = hist_entry; /* Put this entry at the end of history */ memmove(&rs->history[idx], &rs->history[idx + 1], - (READLINE_MAX_CMDS - idx + 1) * sizeof(char *)); + (READLINE_MAX_CMDS - (idx + 1)) * sizeof(char *)); rs->history[READLINE_MAX_CMDS - 1] = NULL; for (; idx < READLINE_MAX_CMDS; idx++) { if (rs->history[idx] == NULL) From e7b48c97fe7ea0b8149e1e3a20be88a908c2bfcd Mon Sep 17 00:00:00 2001 From: Anthony PERARD Date: Fri, 4 Nov 2011 15:35:11 +0000 Subject: [PATCH 5/5] xen-platform: Fix IO port read/write functions Somehow, the read/write functions handle an offset that does not exist anymore. Signed-off-by: Anthony PERARD Signed-off-by: Stefan Hajnoczi --- hw/xen_platform.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/hw/xen_platform.c b/hw/xen_platform.c index 6e3ba8b507..5e792f56f6 100644 --- a/hw/xen_platform.c +++ b/hw/xen_platform.c @@ -113,7 +113,7 @@ static void platform_fixed_ioport_writew(void *opaque, uint32_t addr, uint32_t v { PCIXenPlatformState *s = opaque; - switch (addr - XEN_PLATFORM_IOPORT) { + switch (addr) { case 0: /* Unplug devices. Value is a bitmask of which devices to unplug, with bit 0 the IDE devices, bit 1 the network @@ -152,7 +152,7 @@ static void platform_fixed_ioport_writew(void *opaque, uint32_t addr, uint32_t v static void platform_fixed_ioport_writel(void *opaque, uint32_t addr, uint32_t val) { - switch (addr - XEN_PLATFORM_IOPORT) { + switch (addr) { case 0: /* PV driver version */ break; @@ -163,7 +163,7 @@ static void platform_fixed_ioport_writeb(void *opaque, uint32_t addr, uint32_t v { PCIXenPlatformState *s = opaque; - switch (addr - XEN_PLATFORM_IOPORT) { + switch (addr) { case 0: /* Platform flags */ { hvmmem_type_t mem_type = (val & PFFLAG_ROM_LOCK) ? HVMMEM_ram_ro : HVMMEM_ram_rw; @@ -186,7 +186,7 @@ static uint32_t platform_fixed_ioport_readw(void *opaque, uint32_t addr) { PCIXenPlatformState *s = opaque; - switch (addr - XEN_PLATFORM_IOPORT) { + switch (addr) { case 0: if (s->drivers_blacklisted) { /* The drivers will recognise this magic number and refuse @@ -205,7 +205,7 @@ static uint32_t platform_fixed_ioport_readb(void *opaque, uint32_t addr) { PCIXenPlatformState *s = opaque; - switch (addr - XEN_PLATFORM_IOPORT) { + switch (addr) { case 0: /* Platform flags */ return s->flags; @@ -221,7 +221,7 @@ static void platform_fixed_ioport_reset(void *opaque) { PCIXenPlatformState *s = opaque; - platform_fixed_ioport_writeb(s, XEN_PLATFORM_IOPORT, 0); + platform_fixed_ioport_writeb(s, 0, 0); } const MemoryRegionPortio xen_platform_ioport[] = { @@ -251,7 +251,7 @@ static void platform_fixed_ioport_init(PCIXenPlatformState* s) static uint32_t xen_platform_ioport_readb(void *opaque, uint32_t addr) { if (addr == 0) { - return platform_fixed_ioport_readb(opaque, XEN_PLATFORM_IOPORT); + return platform_fixed_ioport_readb(opaque, 0); } else { return ~0u; } @@ -263,7 +263,7 @@ static void xen_platform_ioport_writeb(void *opaque, uint32_t addr, uint32_t val switch (addr) { case 0: /* Platform flags */ - platform_fixed_ioport_writeb(opaque, XEN_PLATFORM_IOPORT, val); + platform_fixed_ioport_writeb(opaque, 0, val); break; case 8: log_writeb(s, val); @@ -321,7 +321,7 @@ static int xen_platform_post_load(void *opaque, int version_id) { PCIXenPlatformState *s = opaque; - platform_fixed_ioport_writeb(s, XEN_PLATFORM_IOPORT, s->flags); + platform_fixed_ioport_writeb(s, 0, s->flags); return 0; }