Daniel P. Berrange wrote:
Subject: [PATCH] xen_hypervisor.c: avoid NULL deref for NULL domain argument
* src/xen/xen_hypervisor.c (xenHypervisorGetVcpus): Don't attempt to diagnose an unlikely NULL-domain or NULL-domain->conn error. ... @@ -3475,11 +3475,8 @@ xenHypervisorGetVcpus(virDomainPtr domain, virVcpuInfoPtr info, int maxinfo, virVcpuInfoPtr ipt; int nbinfo, i;
- if (domain == NULL || domain->conn == NULL) { - virXenErrorFunc (domain->conn, VIR_ERR_INVALID_ARG, __FUNCTION__, - "invalid argument", 0); + if (domain == NULL || domain->conn == NULL) return -1; - }
I'd rather we just got rid of that check completely - its a left over from a time when Xen was the only driver & these entry points needed to do full checking. Now all mandatory parameters are checked in the previous libvirt.c layer.
Here's an additional patch, to eliminate all of the "domain == NULL" tests. I want to keep this global "clean-up" patch separate from the above bug-fixing one.
I'm less confident about removing the daomin->conn tests, and would be inclined to leave them as-is, or use an assert, if you want to remove them. If we also remove the daomin->conn == NULL tests, an added "assert" is the best way to keep clang/coverity from complaining about a possible NULL-dereference.
From 9e6f7ca7a0dfa6b220e598d04ca40d33e67feb22 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering@redhat.com> Date: Wed, 27 Jan 2010 13:34:03 +0100 Subject: [PATCH] xen_hypervisor.c: remove all "domain == NULL" tests, ...
* src/xen/xen_hypervisor.c: Remove all "domain == NULL" tests. * src/xen/xen_hypervisor.h: Instead, use ATTRIBUTE_NONNULL to mark each "domain" parameter as "known always to be non-NULL". ... ACK
Thanks. I'll push those as soon as "make distcheck" etc. passes.