[libvirt] [PATCH] nwfilter_ebiptables_driver.c: avoid NULL dereference

Another one caught by clang: Note the first test to see if "inst" may be NULL. Then, in the following loop, "inst" is unconditionally dereferenced via "inst[i]". There are other unprotected used of "inst[i]" below, too. Rather than trying to protect all uses, one by one, I chose to return "success" when given an empty list of rules. In addition, not only does it appear to be possible to call this function with a NULL "inst" pointer, but it may even be undefined. At least one caller is virNWFilterInstantiate, where "inst" maps to the caller's "ptrs" variable. There, ptrs is initialized (or not, in some cases) by virNWFilterRuleInstancesToArray. Fortunately, at least this one caller (virNWFilterRuleInstancesToArray) does initialize "ptrs" to NULL, so in actuality, it cannot currently be used undefined. But the fact that a function like virNWFilterRuleInstancesToArray can return "successfully" without defining that output parameter is a little risky. From: Jim Meyering <meyering(a)redhat.com&gt; Date: Mon, 29 Mar 2010 18:27:26 +0200 Subject: [PATCH] nwfilter_ebiptables_driver.c: avoid NULL dereference * src/nwfilter/nwfilter_ebiptables_driver.c (ebiptablesApplyNewRules): Don't dereference a NULL or uninitialized pointer when given an empty list of rules --- src/nwfilter/nwfilter_ebiptables_driver.c | 7 ++++--- 1 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/nwfilter/nwfilter_ebiptables_driver.c b/src/nwfilter/nwfilter_ebiptables_driver.c index 7871926..3932b44 100644 --- a/src/nwfilter/nwfilter_ebiptables_driver.c +++ b/src/nwfilter/nwfilter_ebiptables_driver.c @@ -2378,31 +2378,32 @@ ebiptablesRuleOrderSort(const void *a, const void *b) static int ebiptablesApplyNewRules(virConnectPtr conn, const char *ifname, int nruleInstances, void **_inst) { int i; int cli_status; ebiptablesRuleInstPtr *inst = (ebiptablesRuleInstPtr *)_inst; int chains_in = 0, chains_out = 0; virBuffer buf = VIR_BUFFER_INITIALIZER; int haveIptables = 0; - if (inst) - qsort(inst, nruleInstances, sizeof(inst[0]), - ebiptablesRuleOrderSort); + if (nruleInstances == 0 || inst == NULL) + return 0; + + qsort(inst, nruleInstances, sizeof(inst[0]), ebiptablesRuleOrderSort); for (i = 0; i < nruleInstances; i++) { if (inst[i]->ruleType == RT_EBTABLES) { if (inst[i]->chainprefix == CHAINPREFIX_HOST_IN_TEMP) chains_in |= (1 << inst[i]->neededProtocolChain); else chains_out |= (1 << inst[i]->neededProtocolChain); } } ebtablesUnlinkTmpRootChain(conn, &buf, 1, ifname); ebtablesUnlinkTmpRootChain(conn, &buf, 0, ifname); ebtablesRemoveTmpSubChains(conn, &buf, ifname); ebtablesRemoveTmpRootChain(conn, &buf, 1, ifname); -- 1.7.0.3.448.g82eeb