[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] xen_hypervisor.c: avoid NULL deref for NULL domain argument



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.


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]