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

[Jim Meyering]

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 f2d1a49095ed6f7caa3d5ee67409ac561c55ba77 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering at 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