Re: [libvirt] [PATCH v3] buf: Fix possible infinite loop in EscapeString, VSnprintf
On 09/09/2010 08:09 AM, Cole Robinson wrote:
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
I confirm that the only change in v3 is the addition of a test.
+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;
+
+ 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);
This seems a bit fragile; it is inspecting the innards of virBuffer.
Maybe add a comment here that any change to the innards of virBuffer may
warrant a change to this test at the same time. But having the test,
even if it is fragile, is a good thing, so...
ACK.
We could add further tests to this file to cover more lines of code,
such as: printing a string that needs escapes added, and actually
verifying that the escaped string matches expectations; passing a bad
format string (such as "%$2s" to trigger EINVAL,
"%*d%*d",1<<31,1,1<<31,1 to trigger EOVERFLOW) and verifying that
error
gets set. But those can be in a separate patch; let's get the bug fix
in now before 0.8.4.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org