On Thu, Oct 12, 2017 at 10:26:03AM -0700, William Tu wrote: > Found by inspection. > > Signed-off-by: William Tu <[email protected]> > --- > lib/netdev.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/lib/netdev.c b/lib/netdev.c > index b4e570bafd08..bedc21dec48e 100644 > --- a/lib/netdev.c > +++ b/lib/netdev.c > @@ -2220,6 +2220,8 @@ netdev_ports_insert(struct netdev *netdev, const struct > dpif_class *dpif_class, > ovs_mutex_lock(&netdev_hmap_mutex); > if (netdev_ports_lookup(dpif_port->port_no, dpif_class)) { > ovs_mutex_unlock(&netdev_hmap_mutex); > + free(data); > + free(ifidx); > return EEXIST; > }
Thanks for the report and the fix. I think that we can avoid doing the allocations entirely for this case. What do you think of the following? I guess that it moves allocations into the lock, but I don't know of a performance bottleneck on this lock. --8<--------------------------cut here-------------------------->8-- From: Ben Pfaff <[email protected]> Date: Thu, 12 Oct 2017 10:33:06 -0700 Subject: [PATCH] netdev: Fix memory leak on error path in netdev_ports_insert(). Reported-by: William Tu <[email protected]> Signed-off-by: Ben Pfaff <[email protected]> --- lib/netdev.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/lib/netdev.c b/lib/netdev.c index b4e570bafd08..8d611bc826c2 100644 --- a/lib/netdev.c +++ b/lib/netdev.c @@ -2206,27 +2206,24 @@ netdev_ports_insert(struct netdev *netdev, const struct dpif_class *dpif_class, struct dpif_port *dpif_port) { size_t hash = NETDEV_PORTS_HASH_INT(dpif_port->port_no, dpif_class); - struct port_to_netdev_data *data; - struct ifindex_to_port_data *ifidx; int ifindex = netdev_get_ifindex(netdev); if (ifindex < 0) { return ENODEV; } - data = xzalloc(sizeof *data); - ifidx = xzalloc(sizeof *ifidx); - ovs_mutex_lock(&netdev_hmap_mutex); if (netdev_ports_lookup(dpif_port->port_no, dpif_class)) { ovs_mutex_unlock(&netdev_hmap_mutex); return EEXIST; } + struct port_to_netdev_data *data = xzalloc(sizeof *data); data->netdev = netdev_ref(netdev); data->dpif_class = dpif_class; dpif_port_clone(&data->dpif_port, dpif_port); + struct ifindex_to_port_data *ifidx = xzalloc(sizeof *ifidx); ifidx->ifindex = ifindex; ifidx->port = dpif_port->port_no; -- 2.10.2 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
