From: Ben Pfaff <blp@nicira.com>
Date: Mon, 16 Sep 2013 21:53:27 +0000 (-0700)
Subject: ofproto-dpif: Fix use-after-free error deleting last bridge.
X-Git-Tag: v2.0~27
X-Git-Url: http://git.cascardo.eti.br/?p=cascardo%2Fovs.git;a=commitdiff_plain;h=68542154b46e18a96d871e421dcbfb630c7bf0b0

ofproto-dpif: Fix use-after-free error deleting last bridge.

valgrind reported:

    Invalid read of size 4
       at 0x806ADC1: odp_port_to_ofport (hmap.h:267)
       by 0x8077C05: xlate_receive (ofproto-dpif-xlate.c:523)
       by 0x8073994: handle_miss_upcalls (ofproto-dpif-upcall.c:642)
       by 0x80741AA: udpif_miss_handler (ofproto-dpif-upcall.c:412)
       by 0x56FCC38: start_thread (pthread_create.c:304)
       by 0x735378D: clone (clone.S:130)
     Address 0x786c084 is 4 bytes inside a block of size 16 free'd
       at 0x4D8350C: free (vg_replace_malloc.c:427)
       by 0x8065EDA: close_dpif_backer (ofproto-dpif.c:1094)

The problem is that close_dpif_backer() destroys odp_to_ofport_map and the
associated mutex before it calls udpif_destroy() to stop the forwarding
threads.  This gives the forwarding threads a window in which to try to
use odp_to_ofport_map.

This commit moves the udpif_destroy() call much earlier, solving the
problem.  (The call to udpif_destroy() must follow the call to
drop_key_clear() because drop_key_clear() uses the udpif.)

Signed-off-by: Ben Pfaff <blp@nicira.com>
Acked-by: Jarno Rajahalme <jrajahalme@nicira.com>
---

diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index b5c9c9339..8945b0069 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -1091,13 +1091,14 @@ close_dpif_backer(struct dpif_backer *backer)
     drop_key_clear(backer);
     hmap_destroy(&backer->drop_keys);
 
+    udpif_destroy(backer->udpif);
+
     simap_destroy(&backer->tnl_backers);
     ovs_rwlock_destroy(&backer->odp_to_ofport_lock);
     hmap_destroy(&backer->odp_to_ofport_map);
     node = shash_find(&all_dpif_backers, backer->type);
     free(backer->type);
     shash_delete(&all_dpif_backers, node);
-    udpif_destroy(backer->udpif);
     dpif_close(backer->dpif);
 
     ovs_assert(hmap_is_empty(&backer->subfacets));