[CRIU] [PATCHv2] criu-util: do not double free and simplify xvstrcat

Dmitry Safonov dsafonov at virtuozzo.com
Fri Jul 15 12:16:24 PDT 2016


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 <dsafonov at virtuozzo.com>
---
v2: man 3 vsnprintf: If an output error is encountered, a negative
    value is returned. (it may return even -ENOMEM theoretically, fix this)

 criu/util.c | 36 ++++++++++++++++++------------------
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/criu/util.c b/criu/util.c
index b20c77bbff13..c44d900c66ed 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);
-- 
2.9.0



More information about the CRIU mailing list