[PATCH] makedumpfile: Fix string append in dump_log_entry()

[Atsushi Kumagai]

Hello Petr,

>To quote the sprintf(3) man page:
>
>    Some programs imprudently rely on code such as the following
>
>        sprintf(buf, "%s some further text", buf);
>
>    to append text to buf.  However, the standards explicitly note that
>    the results are undefined if source and destination buffers overlap
>    when calling sprintf(), snprintf(), vsprintf(), and vsnprintf().
>    Depending on the version of gcc(1) used, and the compiler options
>    employed, calls such as the above will not produce the expected results.
>
>The original code is actually miscompiled on openSUSE 13.1.
>
>It's also overkill to call sprintf() for something that can be done
>with a simple assignment.
>
>Signed-off-by: Petr Tesarik <ptesarik at suse.cz>

Thanks, it seems good to me.

Actually, Nick sent the same patch in last July and we tried to
take care of buffer overflow at the same time as below:

http://lists.infradead.org/pipermail/kexec/2013-August/009430.html

However, this thread has been left open, so I was wondering if you
could take over this work. Of course you can decline this, then I'll
do it later as another patch.


Thanks
Atsushi Kumagai

>---
> makedumpfile.c | 13 +++++++------
> 1 file changed, 7 insertions(+), 6 deletions(-)
>
>diff --git a/makedumpfile.c b/makedumpfile.c
>index 579be61..013fce7 100644
>--- a/makedumpfile.c
>+++ b/makedumpfile.c
>@@ -3866,7 +3866,7 @@ reset_bitmap_of_free_pages(unsigned long node_zones, struct cycle *cycle)
> static int
> dump_log_entry(char *logptr, int fp)
> {
>-	char *msg, *p;
>+	char *msg, *p, *bufp;
> 	unsigned int i, text_len;
> 	unsigned long long ts_nsec;
> 	char buf[BUFSIZE];
>@@ -3881,18 +3881,19 @@ dump_log_entry(char *logptr, int fp)
>
> 	msg = logptr + SIZE(printk_log);
>
>-	sprintf(buf, "[%5lld.%06ld] ", nanos, rem/1000);
>+	bufp = buf;
>+	bufp += sprintf(buf, "[%5lld.%06ld] ", nanos, rem/1000);
>
> 	for (i = 0, p = msg; i < text_len; i++, p++) {
> 		if (isprint(*p) || isspace(*p))
>-			sprintf(buf, "%s%c", buf, *p);
>+			*bufp++ = *p;
> 		else
>-			sprintf(buf, "%s\\x%02x", buf, *p);
>+			bufp += sprintf(bufp, "\\x%02x", *p);
> 	}
>
>-	sprintf(buf, "%s\n", buf);
>+	*bufp++ = '\n';
>
>-	if (write(info->fd_dumpfile, buf, strlen(buf)) < 0)
>+	if (write(info->fd_dumpfile, buf, bufp - buf) < 0)
> 		return FALSE;
> 	else
> 		return TRUE;
>--
>1.8.4.5