diff mbox

[ovs-dev] tnl-ports: fix missing netdev_close

Message ID 1476454326-22763-1-git-send-email-cascardo@redhat.com
State Accepted
Headers show

Commit Message

Thadeu Lima de Souza Cascardo Oct. 14, 2016, 2:12 p.m. UTC 
There were leaks on netdev_close references. Whenever an address was removed
from an interface, it was possible that a race would make the netlink message be
processed and no address be found on the interface, causing insert_ipdev to bail
out.

When that happened, the netdev would be left opened as type system, and when
adding an interface with the same name as type internal, even though the system
interface was already gone, netdev_open would return the one with type system.
And, then, no internal interface would be created.

Tested-by: Aaron Conole <aconole@redhat.com>
Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@redhat.com>
---

So, Aaron found out that applying the following commits would fix a bug in
branch 2.5, but we considered that would be too much. So, after some
investigation, we realized that the change below fixes the issue, as described
above. So, we would like to see this applied to branch 2.5, instead of doing a
backport for a larger change.

2b02db1 packets: Add new functions for IPv4 and IPv6 address parsing.
a8704b5 tunneling: Handle multiple ip address for given device.
3e6dc8b netdev: Verify ifa_addr is not NULL when iterating over getifaddrs.
c2a1cee route-table: flush addresses list when route table is reset

---
 lib/tnl-ports.c | 2 ++
 1 file changed, 2 insertions(+)

Comments

Pravin Shelar Oct. 19, 2016, 6:39 p.m. UTC | #1 
On Fri, Oct 14, 2016 at 7:12 AM, Thadeu Lima de Souza Cascardo
<cascardo@redhat.com> wrote:
> There were leaks on netdev_close references. Whenever an address was removed
> from an interface, it was possible that a race would make the netlink message be
> processed and no address be found on the interface, causing insert_ipdev to bail
> out.
>
> When that happened, the netdev would be left opened as type system, and when
> adding an interface with the same name as type internal, even though the system
> interface was already gone, netdev_open would return the one with type system.
> And, then, no internal interface would be created.
>
> Tested-by: Aaron Conole <aconole@redhat.com>
> Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@redhat.com>
> ---
>
> So, Aaron found out that applying the following commits would fix a bug in
> branch 2.5, but we considered that would be too much. So, after some
> investigation, we realized that the change below fixes the issue, as described
> above. So, we would like to see this applied to branch 2.5, instead of doing a
> backport for a larger change.
>
> 2b02db1 packets: Add new functions for IPv4 and IPv6 address parsing.
> a8704b5 tunneling: Handle multiple ip address for given device.
> 3e6dc8b netdev: Verify ifa_addr is not NULL when iterating over getifaddrs.
> c2a1cee route-table: flush addresses list when route table is reset
>

Thanks for the patch. I pushed patch to branch 2.5.
diff mbox

Patch

diff --git a/lib/tnl-ports.c b/lib/tnl-ports.c
index e7f2066..bcf4b94 100644
--- a/lib/tnl-ports.c
+++ b/lib/tnl-ports.c
@@ -368,12 +368,14 @@  insert_ipdev(const char dev_name[])
     ip_dev->change_seq = netdev_get_change_seq(dev);
     error = netdev_get_etheraddr(ip_dev->dev, &ip_dev->mac);
     if (error) {
+        netdev_close(dev);
         free(ip_dev);
         return;
     }
     error4 = netdev_get_in4(ip_dev->dev, (struct in_addr *)&ip_dev->addr4, NULL);
     error6 = netdev_get_in6(ip_dev->dev, &ip_dev->addr6);
     if (error4 && error6) {
+        netdev_close(dev);
         free(ip_dev);
         return;
     }