[libvirt] [PATCH] Sanitize qemu monitor reads
Reading from the qemu monitor pulls in a whole bunch of useless control characters. For example, sending the command 'somecomm' to the monitor returns: somecomm unknown command: 'somecomm' (qemu) Which is 36 characters, however we end up reading over 200. The amount we read actually grows quadratically as a function of the command length. This prevents us from reporting monitor output to the user (incase some command fails) and seriously diminishes the value of the domain logfiles. The attached patch tries to sanitize this a bit. After reading all the output, we search for the first occurrence of the full command string. If found, we search from there for the first newline (which marks the beginning of the monitor response). If these are found, we only return the command, and everything after the newline. If only the command is found, we just drop everything before it. I've tested this on f9 and f8, there didn't seem to be any problems. I think this should be pretty future proof as well, so we hopefully won't be throwing out anything valuable. Thanks, Cole
On Tue, Sep 23, 2008 at 04:56:08PM -0400, Cole Robinson wrote:
@@ -1670,7 +1670,7 @@ qemudMonitorCommand (const struct qemud_driver *driver ATTRIBUTE_UNUSED, const char *cmd, char **reply) { int size = 0; - char *buf = NULL; + char *buf = NULL, *tmpbuf = NULL, *nlptr = NULL, *commptr = NULL; size_t cmdlen = strlen(cmd);
if (safewrite(vm->monitor, cmd, cmdlen) != cmdlen) @@ -1708,7 +1708,31 @@ qemudMonitorCommand (const struct qemud_driver *driver ATTRIBUTE_UNUSED,
/* Look for QEMU prompt to indicate completion */ if (buf && ((tmp = strstr(buf, "\n(qemu) ")) != NULL)) { - tmp[0] = '\0'; + /* Preserve the newline */ + tmp[1] = '\0'; + + /* The monitor doesn't dump clean output after we have written to + * it. Every character we write dumps a bunch of useless stuff, + * so the result looks like "cXcoXcomXcommXcommaXcommanXcommand" + * Try to throw away everything before the first full command + * occurence, and inbetween the command and the newline starting + * the response + */ + if ((commptr = strstr(buf, cmd))) { + tmpbuf = buf; + buf = NULL; + if ((nlptr = strchr(commptr, '\n'))) { + if (VIR_ALLOC_N(buf, strlen(cmd) + strlen(nlptr) + 1) < 0) + goto error; + strncpy(buf, cmd, strlen(cmd)); + strcat(buf, nlptr); + } else { + if ((buf = strdup(commptr)) == NULL) + goto error; + } + VIR_FREE(tmpbuf); + }
It looks to me like tmpbuf is leaked on the two error paths. garbage.collection++ Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into Xen guests. http://et.redhat.com/~rjones/virt-p2v
participants (2)
-
Cole Robinson -
Richard W.M. Jones