buf: Fix possible infinite loop in EscapeString, VSnprintf

The current code will go into an infinite loop if the printf generated
string is >= 1000, AND exactly 1 character smaller than the amount of free
space in the buffer. When this happens, we are dropped into the loop body,
but nothing will actually change, because count == (buf->size - buf->use - 1),
and virBufferGrow returns unchanged if count < (buf->size - buf->use)

Fix this by removing the '- 1' bit from 'size'. The *nprintf functions handle
the NULL byte for us anyways, so we shouldn't need to manually accommodate
for it.

Here's a bug where we are actually hitting this issue:
https://bugzilla.redhat.com/show_bug.cgi?id=602772

v2: Eric's improvements: while -> if (), remove extra va_list variable,
    make sure we report buffer error if snprintf fails

v3: Add tests/virbuftest which reproduces the infinite loop before this
    patch, works correctly after

tests/.gitignore

@@ -21,6 +21,7 @@ storagepoolxml2xmltest
 nodeinfotest
 statstest
 qparamtest
+virbuftest
 seclabeltest
 eventtest
 *.exe

tests/Makefile.am

@@ -73,7 +73,7 @@ EXTRA_DIST =		\
 	qemuhelpdata
 
 check_PROGRAMS = virshtest conftest \
-        nodeinfotest statstest qparamtest
+        nodeinfotest statstest qparamtest virbuftest
 
 if WITH_XEN
 check_PROGRAMS += xml2sexprtest sexpr2xmltest \
@@ -151,6 +151,7 @@ TESTS = virshtest \
         nodeinfotest \
 	statstest \
 	qparamtest \
+	virbuftest \
 	$(test_scripts)
 
 if WITH_XEN
@@ -352,6 +353,10 @@ qparamtest_SOURCES = \
 	qparamtest.c testutils.h testutils.c
 qparamtest_LDADD = $(LDADDS)
 
+virbuftest_SOURCES = \
+	virbuftest.c testutils.h testutils.c
+virbuftest_LDADD = $(LDADDS)
+
 if WITH_LIBVIRTD
 eventtest_SOURCES = \
 	eventtest.c testutils.h testutils.c ../daemon/event.c

tests/virbuftest.c

@@ -0,0 +1,86 @@
+#include <config.h>
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+#include "internal.h"
+#include "util.h"
+#include "testutils.h"
+#include "buf.h"
+#include "memory.h"
+
+#define TEST_ERROR(...)                             \
+    do {                                            \
+        if (virTestGetDebug())                      \
+            fprintf(stderr, __VA_ARGS__);           \
+    } while (0)
+
+struct testInfo {
+    int doEscape;
+};
+
+static int testBufInfiniteLoop(const void *data ATTRIBUTE_UNUSED)
+{
+    virBuffer bufinit = VIR_BUFFER_INITIALIZER;
+    virBufferPtr buf = &bufinit;
+    char *addstr = NULL, *bufret = NULL;
+    int ret = -1;
+    const struct testInfo *info = data;
+
+    /* This relies of virBuffer internals, so may break if things change
+     * in the future */
+    virBufferAddChar(buf, 'a');
+    if (buf->a != 1002 || buf->b != 1) {
+        TEST_ERROR("Buf was not expected size, size=%d use=%d\n",
+                   buf->a, buf->b);
+        goto out;
+    }
+
+    /*
+     * Infinite loop triggers if:
+     * (strlen + 1 > 1000) && (strlen == buf-size - buf-use - 1)
+     */
+    if (virAsprintf(&addstr, "%*s", buf->a - buf->b - 1, "a") < 0) {
+        goto out;
+    }
+
+    if (info->doEscape)
+        virBufferEscapeString(buf, "%s", addstr);
+    else
+        virBufferVSprintf(buf, "%s", addstr);
+
+    ret = 0;
+out:
+    bufret = virBufferContentAndReset(buf);
+    if (!bufret) {
+        TEST_ERROR("Buffer had error set");
+        ret = -1;
+    }
+
+    VIR_FREE(addstr);
+    VIR_FREE(bufret);
+    return ret;
+}
+
+static int
+mymain(int argc ATTRIBUTE_UNUSED,
+       char **argv ATTRIBUTE_UNUSED)
+{
+    int ret = 0;
+
+
+# define DO_TEST(msg, cb, data)                                        \
+    do {                                                               \
+        struct testInfo info = { data };                               \
+        if (virtTestRun("Buf: " msg, 1, cb, &info) < 0)                 \
+            ret = -1;                                                  \
+    } while (0)
+
+    DO_TEST("EscapeString infinite loop", testBufInfiniteLoop, 1);
+    DO_TEST("VSprintf infinite loop", testBufInfiniteLoop, 0);
+
+    return(ret==0 ? EXIT_SUCCESS : EXIT_FAILURE);
+}
+
+VIRT_TEST_MAIN(mymain)