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

Alex Jia ajia at redhat.com
Fri Aug 5 01:00:05 UTC 2011


On 08/05/2011 04:17 AM, Eric Blake wrote:
> On 08/04/2011 02:07 PM, Eric Blake wrote:
>>
>> 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));
>
> Also squash this in - no need to free buf twice.
>
> diff --git i/src/qemu/qemu_process.c w/src/qemu/qemu_process.c
> index 1eea45f..a37f709 100644
> --- i/src/qemu/qemu_process.c
> +++ w/src/qemu/qemu_process.c
> @@ -1221,16 +1221,14 @@ cleanup:
>          qemuReportError(VIR_ERR_INTERNAL_ERROR,
>                          _("process exited while connecting to 
> monitor: %s"),
>                          buf);
>          ret = -1;
>      }
>
>  closelog:
> -    VIR_FREE(buf);
> -
>      if (VIR_CLOSE(logfd) < 0) {
>          char ebuf[1024];
>          VIR_WARN("Unable to close logfile: %s",
>                   virStrerror(errno, ebuf, sizeof ebuf));
>      }
>
>      VIR_FREE(buf);
>
Right, this is a double free, Eric, Hasn't Coverity found this issue?

Thanks,
Alex






More information about the libvir-list mailing list