Skip to content

Commit

Permalink
ovn-controller: fix use-after-free in physical_run()
Browse files Browse the repository at this point in the history
The hmap "tunnels" is persistent across IDL loop iterations, but
stores pointers to strings in the local db replica which can be
freed as database updates are processed. Fix by storing a copy
of the string in the hmap instead of a pointer to the string in
the replica.

Found via valgrind.

Fixes: 40128e3 ("physical: Refactor port binding processing.")
Signed-off-by: Lance Richardson <lrichard@redhat.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
  • Loading branch information
hlrichardson authored and blp committed Jul 12, 2017
1 parent e2d6d56 commit 122f8af
Showing 1 changed file with 3 additions and 2 deletions.
5 changes: 3 additions & 2 deletions ovn/controller/physical.c
Expand Up @@ -63,7 +63,7 @@ static struct hmap tunnels = HMAP_INITIALIZER(&tunnels);
* used to reach that chassis. */
struct chassis_tunnel {
struct hmap_node hmap_node;
const char *chassis_id;
char *chassis_id;
ofp_port_t ofport;
enum chassis_tunnel_type type;
};
Expand Down Expand Up @@ -722,7 +722,7 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id mff_ovn_geneve,
tun = xmalloc(sizeof *tun);
hmap_insert(&tunnels, &tun->hmap_node,
hash_string(chassis_id, 0));
tun->chassis_id = chassis_id;
tun->chassis_id = xstrdup(chassis_id);
tun->ofport = u16_to_ofp(ofport);
tun->type = tunnel_type;
physical_map_changed = true;
Expand All @@ -744,6 +744,7 @@ physical_run(struct controller_ctx *ctx, enum mf_field_id mff_ovn_geneve,
if (!simap_find(&new_tunnel_to_ofport, tun->chassis_id)) {
hmap_remove(&tunnels, &tun->hmap_node);
physical_map_changed = true;
free(tun->chassis_id);
free(tun);
}
}
Expand Down

0 comments on commit 122f8af

Please sign in to comment.