| Message ID | 20180814131602.20050-1-colin.king@canonical.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | fwts_coreboot_cbmem: fix off-by-one error on last char of buffer | expand |
On 2018-08-14 06:16 AM, Colin King wrote: > From: Colin Ian King <colin.king@canonical.com> > > The coreboot_log has been allocated a size of console->size + 1, > however, the end of string terminator '\0' is being written one > character passed the end of the buffer because of an off-by-one > error. Fix this. > > Also insert some spaces as per the fwts coding style. > > Detected by CoverityScan, CID#1394472 ("Out-of-bounds write") > > Fixes: c9bf07f25d13 ("fwts_coreboot.c: add cbmem console parser") > Signed-off-by: Colin Ian King <colin.king@canonical.com> > --- > src/lib/src/fwts_coreboot_cbmem.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/src/lib/src/fwts_coreboot_cbmem.c b/src/lib/src/fwts_coreboot_cbmem.c > index 456ac694..52f83f47 100644 > --- a/src/lib/src/fwts_coreboot_cbmem.c > +++ b/src/lib/src/fwts_coreboot_cbmem.c > @@ -334,13 +334,13 @@ char *fwts_coreboot_cbmem_console_dump(void) > > free(console_p); > > - coreboot_log = malloc(console->size+1); > + coreboot_log = malloc(console->size + 1); > if (!coreboot_log) { > free(console); > return NULL; > } > > - coreboot_log[console->size + 1] = '\0'; > + coreboot_log[console->size] = '\0'; > > count = memconsole_coreboot_read(console, coreboot_log, 0, console->size); > free(console); > Acked-by: Alex Hung <alex.hung@canonical.com>
On 08/14/2018 09:16 PM, Colin King wrote: > From: Colin Ian King <colin.king@canonical.com> > > The coreboot_log has been allocated a size of console->size + 1, > however, the end of string terminator '\0' is being written one > character passed the end of the buffer because of an off-by-one > error. Fix this. > > Also insert some spaces as per the fwts coding style. > > Detected by CoverityScan, CID#1394472 ("Out-of-bounds write") > > Fixes: c9bf07f25d13 ("fwts_coreboot.c: add cbmem console parser") > Signed-off-by: Colin Ian King <colin.king@canonical.com> > --- > src/lib/src/fwts_coreboot_cbmem.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/src/lib/src/fwts_coreboot_cbmem.c b/src/lib/src/fwts_coreboot_cbmem.c > index 456ac694..52f83f47 100644 > --- a/src/lib/src/fwts_coreboot_cbmem.c > +++ b/src/lib/src/fwts_coreboot_cbmem.c > @@ -334,13 +334,13 @@ char *fwts_coreboot_cbmem_console_dump(void) > > free(console_p); > > - coreboot_log = malloc(console->size+1); > + coreboot_log = malloc(console->size + 1); > if (!coreboot_log) { > free(console); > return NULL; > } > > - coreboot_log[console->size + 1] = '\0'; > + coreboot_log[console->size] = '\0'; > > count = memconsole_coreboot_read(console, coreboot_log, 0, console->size); > free(console); Acked-by: Ivan Hu <ivan.hu@canonical.com>
diff --git a/src/lib/src/fwts_coreboot_cbmem.c b/src/lib/src/fwts_coreboot_cbmem.c index 456ac694..52f83f47 100644 --- a/src/lib/src/fwts_coreboot_cbmem.c +++ b/src/lib/src/fwts_coreboot_cbmem.c @@ -334,13 +334,13 @@ char *fwts_coreboot_cbmem_console_dump(void) free(console_p); - coreboot_log = malloc(console->size+1); + coreboot_log = malloc(console->size + 1); if (!coreboot_log) { free(console); return NULL; } - coreboot_log[console->size + 1] = '\0'; + coreboot_log[console->size] = '\0'; count = memconsole_coreboot_read(console, coreboot_log, 0, console->size); free(console);