[ovs-dev] [PATCH] netdev: Fix memory leak on error path.

William Tu u9012063 at gmail.com
Thu Oct 12 18:00:12 UTC 2017


> 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 <blp at ovn.org>
> 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 <u9012063 at gmail.com>
> Signed-off-by: Ben Pfaff <blp at ovn.org>
> ---
>  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
>
Thanks for the feedback.
Yes, I think it looks better, and since port insert is not in
performance critical path, allocation inside the mutex should not have
much impact.

Let's me test it and resubmit v2.
William


More information about the dev mailing list