[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