[libvirt] [PATCH] qemu: avoid pass null pointer as an argument

[Eric Blake]

On 08/04/2011 09:31 AM, Alex Jia wrote:
> In fact, 'pos' is always -1, this reason is because qemuProcessStart function
> assigns -1 to 'pos' variable then call qemuProcessWaitForMonitor,

Actually, qemuProcessStart always calls qemuProcessWaitForMonitor with 
non-negative pos (pos was set by lseek()ing to the end of the log file).

 > meanwhile,
> qemuProcessAttach function also call qemuProcessWaitForMonitor and directly
> pass -1 as an argument, so if (pos != -1) statement can't been run for ever,

Rather, this merely means that the if (pos != -1) statement wasn't run 
when called by qemuProcessAttach.

> it also means we can't allocate memory to 'buf' variable, that is, 'buf' is
> a initial value NULL, however, the function
> qemuProcessReadLogFD(logfd, buf, buf_size, strlen(buf)) will be called
> on 'cleanup' section, null pointer passed as an argument.
>
> * src/qemu/qemu_process.c: avoid null pointer passed as an argument to a
>   'nonnull' parameter.

We definitely have a bug here, but this is not the right fix.  The bug 
is that the cleanup: label is trying to read from logfd if the vm 
crashed, without having opened logfd in the qemuProcessAttach case.

I think the more appropriate patch is this:

diff --git i/src/qemu/qemu_process.c w/src/qemu/qemu_process.c
index 8508ff6..1eea45f 100644
--- i/src/qemu/qemu_process.c
+++ w/src/qemu/qemu_process.c
@@ -1214,7 +1214,7 @@ qemuProcessWaitForMonitor(struct qemud_driver* driver,
  cleanup:
      virHashFree(paths);

-    if (kill(vm->pid, 0) == -1 && errno == ESRCH) {
+    if (pos != -1 && kill(vm->pid, 0) == -1 && errno == ESRCH) {
          /* VM is dead, any other error raised in the interim is probably
           * not as important as the qemu cmdline output */
          qemuProcessReadLogFD(logfd, buf, buf_size, strlen(buf));

-- 
Eric Blake   eblake at redhat.com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org