[libvirt] [PATCH] qemu: avoid pass null pointer as an argument
Alex Jia
ajia at redhat.com
Fri Aug 5 01:10:29 UTC 2011
On 08/05/2011 04:07 AM, Eric Blake wrote:
> 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));
>
Agree, it indeed is a issue, I will check 'null pointer' issue with
thiis fixed again, to avoid some warning from ccc-analyzer, if you set
up this env, please also check it.
Thanks,
Alex
More information about the libvir-list
mailing list