From 8f95747e00ccc1f359cd42a06bc56db4d103ef48 Mon Sep 17 00:00:00 2001 From: Dmitry Safonov Date: Fri, 15 Jul 2016 22:16:00 +0300 Subject: [PATCH] criu-util: do not double free and simplify xvstrcat There is a bug, that if vsnprintf() wrote nothing to buffer: that may be xstrcat(0, "%s", "") or something like that, than vsnprintf's return value is 0, which will be lesser than delta. The code before would do following: o first cycle: 1. relocate str to new (str is not allocated anymore) 2. vsnprintf() retured 0, delta is greater. o second cycle: 1. relocate previously freed str to new..^C ^C Segmentation fault (core dumped) Weeell, I do think, we can do better job here. Signed-off-by: Dmitry Safonov Signed-off-by: Pavel Emelyanov --- criu/util.c | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) diff --git a/criu/util.c b/criu/util.c index b20c77bbf..c44d900c6 100644 --- a/criu/util.c +++ b/criu/util.c @@ -74,27 +74,27 @@ static char *xvstrcat(char *str, const char *fmt, va_list args) delta = strlen(fmt) * 2; do { - ret = -ENOMEM; new = xrealloc(str, offset + delta); - if (new) { - va_copy(tmp, args); - ret = vsnprintf(new + offset, delta, fmt, tmp); - va_end(tmp); - if (ret >= delta) { - /* NOTE: vsnprintf returns the amount of bytes - * to allocate. */ - delta = ret +1; - str = new; - ret = 0; - } + if (!new) { + /* realloc failed. We must release former string */ + xfree(str); + pr_err("Failed to allocate string\n"); + return new; } - } while (ret == 0); - if (ret == -ENOMEM) { - /* realloc failed. We must release former string */ - pr_err("Failed to allocate string\n"); - xfree(str); - } else if (ret < 0) { + va_copy(tmp, args); + ret = vsnprintf(new + offset, delta, fmt, tmp); + va_end(tmp); + if (ret < delta) /* an error, or all was written */ + break; + + /* NOTE: vsnprintf returns the amount of bytes + * to allocate. */ + delta = ret + 1; + str = new; + } while (1); + + if (ret < 0) { /* vsnprintf failed */ pr_err("Failed to print string\n"); xfree(new);