[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