[ovs-dev] [PATCH 1/2] revalidator: Prevent double-delete of ukey.

Joe Stringer joe at ovn.org
Tue Jan 10 23:54:02 UTC 2017


revalidator_sweep__() splits checking for whether to delete a ukey from
the actual deletion to prevent taking the umap lock for too long.
However it uses information gathered from the first critical section to
decide to call ukey_delete() - ie, the second critical section.

Since 67f08985d769 ("upcall: Replace ukeys for deleted flows."), it is
possible for a handler thread to receive an upcall for the same flow and
to replace the ukey which is being deleted with a new one, in between
these critical sections. This will remove the ukey from the cmap,
rcu-defer its deletion, and update the existing ukey.

If this occurs in between the critical sections of revalidator cleanup
of the flow, then the revalidator will subsequently call ukey_delete()
to delete the original ukey, which was already deleted by the handler
thread.

Guard against this by checking the ukey state in ukey_delete().

Backtrace:
    Program terminated with signal 11, Segmentation fault.
    #0  0x00007fe969b13da3 in cmap_replace__ ()
    #1  0x00007fe969b14491 in cmap_replace ()
    #2  0x00007fe969aee9ff in ukey_delete ()
    #3  0x00007fe969aefd42 in revalidator_sweep__ ()
    #4  0x00007fe969af1bad in udpif_revalidator ()
    #5  0x00007fe969b8b2a6 in ovsthread_wrapper ()
    #6  0x00007fe968e07dc5 in start_thread () from /lib64/libpthread.so.0
    #7  0x00007fe96862c73d in clone () from /lib64/libc.so.6

Fixes: 54ebeff4c03d ("upcall: Track ukey states.")
Fixes: 67f08985d769 ("upcall: Replace ukeys for deleted flows.")
Reported-by: Numan Siddique <nusiddiq at redhat.com>
Signed-off-by: Joe Stringer <joe at ovn.org>
---
 ofproto/ofproto-dpif-upcall.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c
index cd2445fc4ab9..1ffeaabf7d8e 100644
--- a/ofproto/ofproto-dpif-upcall.c
+++ b/ofproto/ofproto-dpif-upcall.c
@@ -1794,9 +1794,11 @@ ukey_delete(struct umap *umap, struct udpif_key *ukey)
     OVS_REQUIRES(umap->mutex)
 {
     ovs_mutex_lock(&ukey->mutex);
-    cmap_remove(&umap->cmap, &ukey->cmap_node, ukey->hash);
-    ovsrcu_postpone(ukey_delete__, ukey);
-    transition_ukey(ukey, UKEY_DELETED);
+    if (ukey->state < UKEY_DELETED) {
+        cmap_remove(&umap->cmap, &ukey->cmap_node, ukey->hash);
+        ovsrcu_postpone(ukey_delete__, ukey);
+        transition_ukey(ukey, UKEY_DELETED);
+    }
     ovs_mutex_unlock(&ukey->mutex);
 }
 
-- 
2.10.2



More information about the dev mailing list