libvir-list-bounces@redhat.com wrote on 03/29/2010 12:37:02 PM:
[image removed]
[libvirt] [PATCH] nwfilter_ebiptables_driver.c: avoid NULL dereference
Jim Meyering
to:
Libvirt
03/29/2010 12:43 PM
Sent by:
libvir-list-bounces@redhat.com
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.
Thanks again for looking at the code. I'd like to fix this differently. Rather than return with 0 early I'd like the qsort to be skipped. If you have zero entries that would be like clearing the ebtables/iptables rules, which then should really be the only code that's executed. Regards, Stefan
From f2d1a49095ed6f7caa3d5ee67409ac561c55ba77 Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering@redhat.com> 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
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list