On 05/17/2010 05:53 AM, Daniel P. Berrange wrote:
> History has shown that there are frequent bugs in the QEMU driver
> code leading to the monitor being invoked with a NULL pointer.
> Although the QEMU driver code should always report an error in
> this case before invoking the monitor, as a safety net put in a
> generic check in the monitor code entry points.
>
> * src/qemu/qemu_monitor.c: Safety net to check for NULL monitor
> object
> ---
> src/qemu/qemu_monitor.c | 409 +++++++++++++++++++++++++++++++++++++++-------
> 1 files changed, 346 insertions(+), 63 deletions(-)
>
> diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
> index 2ce3d56..ec22c20 100644
> --- a/src/qemu/qemu_monitor.c
> +++ b/src/qemu/qemu_monitor.c
> @@ -894,7 +894,13 @@ int qemuMonitorEmitGraphics(qemuMonitorPtr mon,
> int qemuMonitorSetCapabilities(qemuMonitorPtr mon)
> {
> int ret;
> - DEBUG("mon=%p, fd=%d", mon, mon->fd);
> + DEBUG("mon=%p", mon);
> +
> + if (!mon) {
> + qemuReportError(VIR_ERR_INVALID_ARG, "%s",
> + _("monitor must not be NULL"));
> + return -1;
> + }
Wouldn't it be better to move the DEBUG() to be after the (!mon) check,
so that we can still print mon->fd? (Throughout the patch).
> @@ -1017,7 +1065,14 @@ int qemuMonitorSetVNCPassword(qemuMonitorPtr mon,
> const char *password)
> {
> int ret;
> - DEBUG("mon=%p, fd=%d", mon, mon->fd);
> + DEBUG("mon=%p, password=%p",
> + mon, password);
> +
> + if (!mon) {
> + qemuReportError(VIR_ERR_INVALID_ARG, "%s",
> + _("monitor must not be NULL"));
> + return -1;
> + }
Likewise. And while it was nice to add password=%p,...
>
> if (!password)
> password = "";
...you may have just dereferenced another NULL pointer (at least DEBUG
tends to only be used with glibc, where you get a sane "(null)" instead
of a crash).
--
Eric Blake eblake@xxxxxxxxxx +1-801-349-2682
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list