[ovs-dev] [reordering 1/7] ofproto-dpif: Fix use-after-free error deleting last bridge.
Jarno Rajahalme
jrajahalme at nicira.com
Tue Sep 17 19:24:58 UTC 2013
Acked-by: Jarno Rajahalme <jrajahalme at nicira.com>
On Sep 16, 2013, at 2:59 PM, Ben Pfaff <blp at nicira.com> wrote:
> 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 at nicira.com>
> ---
> ofproto/ofproto-dpif.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index b91b3df..b41c179 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -1090,13 +1090,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));
> --
> 1.7.10.4
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
More information about the dev
mailing list