[ovs-dev] dpif-netlink: Free leaked nl_sock
diff mbox series

Message ID 1570834247-31500-1-git-send-email-pkusunyifeng@gmail.com
State New
Headers show
Series
  • [ovs-dev] dpif-netlink: Free leaked nl_sock
Related show

Commit Message

Yifeng Sun Oct. 11, 2019, 10:50 p.m. UTC
Valgrind reports:
20 bytes in 1 blocks are definitely lost in loss record 94 of 353
    by 0x532594: xmalloc (util.c:138)
    by 0x553EAD: nl_sock_create (netlink-socket.c:146)
    by 0x54331D: create_nl_sock (dpif-netlink.c:255)
    by 0x54331D: dpif_netlink_port_add__ (dpif-netlink.c:756)
    by 0x5435F6: dpif_netlink_port_add_compat (dpif-netlink.c:876)
    by 0x5435F6: dpif_netlink_port_add (dpif-netlink.c:922)
    by 0x47EC1D: dpif_port_add (dpif.c:584)
    by 0x42B35F: port_add (ofproto-dpif.c:3721)
    by 0x41E64A: ofproto_port_add (ofproto.c:2032)
    by 0x40B3FE: iface_do_create (bridge.c:1817)
    by 0x40B3FE: iface_create (bridge.c:1855)
    by 0x40B3FE: bridge_add_ports__ (bridge.c:943)
    by 0x40D14A: bridge_add_ports (bridge.c:959)
    by 0x40D14A: bridge_reconfigure (bridge.c:673)
    by 0x410D75: bridge_run (bridge.c:3050)
    by 0x407614: main (ovs-vswitchd.c:127)

This leak is because when vport_add_channel() returns 0, it is expected
to take the ownership of 'socksp'. This patch fixes this issue.

Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com>
---
 lib/dpif-netlink.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Ben Pfaff Oct. 14, 2019, 6:06 p.m. UTC | #1
On Fri, Oct 11, 2019 at 03:50:47PM -0700, Yifeng Sun wrote:
> Valgrind reports:
> 20 bytes in 1 blocks are definitely lost in loss record 94 of 353
>     by 0x532594: xmalloc (util.c:138)
>     by 0x553EAD: nl_sock_create (netlink-socket.c:146)
>     by 0x54331D: create_nl_sock (dpif-netlink.c:255)
>     by 0x54331D: dpif_netlink_port_add__ (dpif-netlink.c:756)
>     by 0x5435F6: dpif_netlink_port_add_compat (dpif-netlink.c:876)
>     by 0x5435F6: dpif_netlink_port_add (dpif-netlink.c:922)
>     by 0x47EC1D: dpif_port_add (dpif.c:584)
>     by 0x42B35F: port_add (ofproto-dpif.c:3721)
>     by 0x41E64A: ofproto_port_add (ofproto.c:2032)
>     by 0x40B3FE: iface_do_create (bridge.c:1817)
>     by 0x40B3FE: iface_create (bridge.c:1855)
>     by 0x40B3FE: bridge_add_ports__ (bridge.c:943)
>     by 0x40D14A: bridge_add_ports (bridge.c:959)
>     by 0x40D14A: bridge_reconfigure (bridge.c:673)
>     by 0x410D75: bridge_run (bridge.c:3050)
>     by 0x407614: main (ovs-vswitchd.c:127)
> 
> This leak is because when vport_add_channel() returns 0, it is expected
> to take the ownership of 'socksp'. This patch fixes this issue.
> 
> Signed-off-by: Yifeng Sun <pkusunyifeng@gmail.com>

Thank you.  I applied this to master and backported it as far as it
would go.

There's some bad naming here.  I'll follow up with improvements.

Patch
diff mbox series

diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
index ceb39cde1ab1..ebe22106e0fc 100644
--- a/lib/dpif-netlink.c
+++ b/lib/dpif-netlink.c
@@ -458,6 +458,7 @@  vport_add_channel(struct dpif_netlink *dpif, odp_port_t port_no,
     int error;
 
     if (dpif->handlers == NULL) {
+        close_nl_sock(socksp);
         return 0;
     }