edecca
From 6293887f71e7db7ea8a3923aacf7d0fb27b47559 Mon Sep 17 00:00:00 2001
edecca
Message-Id: <6293887f71e7db7ea8a3923aacf7d0fb27b47559@dist-git>
edecca
From: John Ferlan <jferlan@redhat.com>
edecca
Date: Thu, 26 Jul 2018 09:39:32 -0400
edecca
Subject: [PATCH] nwfilter: Resolve SEGV for NWFilter Snoop processing
edecca
edecca
https://bugzilla.redhat.com/show_bug.cgi?id=1599973
edecca
edecca
Commit id fca9afa08 changed the @req->ifname to use
edecca
@req->binding->portdevname fillingin the @req->binding
edecca
in a similar way that @req->ifname would have been
edecca
filled in during virNWFilterDHCPSnoopReq processing.
edecca
edecca
However, in doing so it did not take into account some
edecca
code paths where the @req->binding should be checked
edecca
instead of @req->binding->portdevname. These checks
edecca
led to SEGVs in some cases during libvirtd reload
edecca
processing in virNWFilterSnoopRemAllReqIter (for
edecca
stop during nwfilterStateCleanup processing) and
edecca
virNWFilterSnoopReqLeaseDel (for start during
edecca
nwfilterStateInitialize processing).
edecca
edecca
In particular, when reading the nwfilter.leases file
edecca
a new @req is created, but the @req->binding is not
edecca
filled in. That's left to virNWFilterDHCPSnoopReq
edecca
processing which checks if the @req already exists
edecca
in the @virNWFilterSnoopState.snoopReqs hash table
edecca
after adding a virNWFilterSnoopState.ifnameToKey
edecca
entry for the @req->binding->portdevname by a
edecca
@ref->ikey value.
edecca
edecca
NB: virNWFilterSnoopIPLeaseInstallRule and
edecca
    virNWFilterDHCPSnoopThread do not need the
edecca
    req->binding check since they can only be called
edecca
    after the filter->binding is created/assigned.
edecca
edecca
Signed-off-by: John Ferlan <jferlan@redhat.com>
edecca
ACKed-by: Michal Privoznik <mprivozn@redhat.com>
edecca
(cherry picked from commit 5229494b01acf97dbc6f3028e9718667e9e1426a)
edecca
Reviewed-by: Erik Skultety <eskultet@redhat.com>
edecca
---
edecca
 src/nwfilter/nwfilter_dhcpsnoop.c | 7 ++++---
edecca
 1 file changed, 4 insertions(+), 3 deletions(-)
edecca
edecca
diff --git a/src/nwfilter/nwfilter_dhcpsnoop.c b/src/nwfilter/nwfilter_dhcpsnoop.c
edecca
index 533c45f080..2ae134ed19 100644
edecca
--- a/src/nwfilter/nwfilter_dhcpsnoop.c
edecca
+++ b/src/nwfilter/nwfilter_dhcpsnoop.c
edecca
@@ -846,7 +846,7 @@ virNWFilterSnoopReqLeaseDel(virNWFilterSnoopReqPtr req,
edecca
     int ret = 0;
edecca
     virNWFilterSnoopIPLeasePtr ipl;
edecca
     char *ipstr = NULL;
edecca
-    int ipAddrLeft;
edecca
+    int ipAddrLeft = 0;
edecca
 
edecca
     /* protect req->start, req->ifname and the lease */
edecca
     virNWFilterSnoopReqLock(req);
edecca
@@ -867,7 +867,8 @@ virNWFilterSnoopReqLeaseDel(virNWFilterSnoopReqPtr req,
edecca
     if (update_leasefile)
edecca
         virNWFilterSnoopLeaseFileSave(ipl);
edecca
 
edecca
-    ipAddrLeft = virNWFilterIPAddrMapDelIPAddr(req->binding->portdevname, ipstr);
edecca
+    if (req->binding)
edecca
+        ipAddrLeft = virNWFilterIPAddrMapDelIPAddr(req->binding->portdevname, ipstr);
edecca
 
edecca
     if (!req->threadkey || !instantiate)
edecca
         goto skip_instantiate;
edecca
@@ -2037,7 +2038,7 @@ virNWFilterSnoopRemAllReqIter(const void *payload,
edecca
     /* protect req->binding->portdevname */
edecca
     virNWFilterSnoopReqLock(req);
edecca
 
edecca
-    if (req->binding->portdevname) {
edecca
+    if (req->binding && req->binding->portdevname) {
edecca
         ignore_value(virHashRemoveEntry(virNWFilterSnoopState.ifnameToKey,
edecca
                                         req->binding->portdevname));
edecca
 
edecca
-- 
edecca
2.18.0
edecca