netdev - Re: [PATCH net] ipvlan: fix addr hash list corruption

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20150325095851.1d8d1622@griffin>
Date:	Wed, 25 Mar 2015 09:58:51 +0100
From:	Jiri Benc <jbenc@...hat.com>
To:	Mahesh Bandewar <maheshb@...gle.com>
Cc:	David Miller <davem@...emloft.net>,
	linux-netdev <netdev@...r.kernel.org>, dcbw@...hat.com
Subject: Re: [PATCH net] ipvlan: fix addr hash list corruption

On Tue, 24 Mar 2015 16:16:38 -0700, Mahesh Bandewar wrote:
> Well, we already have hlist_unhashed().The following patch should fix
> the duplicate addition as well as deletion. Please give it a try.

Good idea, it's surely better than adding a new boolean.

However, I'm wondering that when there's apparently no problem with the
addresses being on the hash list when interface is down, what's the
point in clearing the hash list in the ndo_stop handler and
repopulating it in ndo_open?

The following patch fixes the problem, too, and as a bonus removes code:

diff --git a/drivers/net/ipvlan/ipvlan_main.c b/drivers/net/ipvlan/ipvlan_main.c
index 4f4099d5603d..03def841b1a9 100644
--- a/drivers/net/ipvlan/ipvlan_main.c
+++ b/drivers/net/ipvlan/ipvlan_main.c
@@ -149,10 +149,6 @@ static int ipvlan_open(struct net_device *dev)
 	else
 		dev->flags &= ~IFF_NOARP;
 
-	if (ipvlan->ipv6cnt > 0 || ipvlan->ipv4cnt > 0) {
-		list_for_each_entry(addr, &ipvlan->addrs, anode)
-			ipvlan_ht_addr_add(ipvlan, addr);
-	}
 	return dev_uc_add(phy_dev, phy_dev->dev_addr);
 }
 
@@ -166,11 +162,6 @@ static int ipvlan_stop(struct net_device *dev)
 	dev_mc_unsync(phy_dev, dev);
 
 	dev_uc_del(phy_dev, phy_dev->dev_addr);
-
-	if (ipvlan->ipv6cnt > 0 || ipvlan->ipv4cnt > 0) {
-		list_for_each_entry(addr, &ipvlan->addrs, anode)
-			ipvlan_ht_addr_del(addr, !dev->dismantle);
-	}
 	return 0;
 }
 

What do you think? Should I submit this formally?

 Jiri

-- 
Jiri Benc
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html