netdev - [PATCH net] netdevice: Fix promiscuity and allmulti negative overflow

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-next>] [day] [month] [year] [list]

Message-Id: <20190404124518.16794-1-liuhangbin@gmail.com>
Date:   Thu,  4 Apr 2019 20:45:18 +0800
From:   Hangbin Liu <liuhangbin@...il.com>
To:     netdev@...r.kernel.org
Cc:     Wang Chen <wangchen@...fujitsu.com>,
        David Miller <davem@...emloft.net>,
        Stefano Brivio <sbrivio@...hat.com>,
        Hangbin Liu <liuhangbin@...il.com>
Subject: [PATCH net] netdevice: Fix promiscuity and allmulti negative overflow

Similarly to dad9b335c694 ("netdevice: Fix promiscuity and allmulti
overflow"), we should not decrease promiscuity if it is already 0.

An example is after adding a team interface to bridge, the team interface
will enter promisc mode. Then if we add a slave to team0, the slave will
keep promisc off. If we remove team from bridge, both team0 and slave will
decrease the promiscuity, which will cause a negative overflow on the slave.
The team's issue will be fixed in a separate patch.

Reviewed-by: Stefano Brivio <sbrivio@...hat.com>
Signed-off-by: Hangbin Liu <liuhangbin@...il.com>
---
 net/core/dev.c | 54 ++++++++++++++++++++++----------------------------
 1 file changed, 24 insertions(+), 30 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index 2b67f2aa59dd..9744e9f696ba 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -7338,22 +7338,19 @@ static int __dev_set_promiscuity(struct net_device *dev, int inc, bool notify)
 
 	ASSERT_RTNL();
 
-	dev->flags |= IFF_PROMISC;
-	dev->promiscuity += inc;
-	if (dev->promiscuity == 0) {
-		/*
-		 * Avoid overflow.
-		 * If inc causes overflow, untouch promisc and return error.
-		 */
-		if (inc < 0)
-			dev->flags &= ~IFF_PROMISC;
-		else {
-			dev->promiscuity -= inc;
-			pr_warn("%s: promiscuity touches roof, set promiscuity failed. promiscuity feature of device might be broken.\n",
-				dev->name);
-			return -EOVERFLOW;
-		}
+	if ((inc < 0 && dev->promiscuity + inc > dev->promiscuity) ||
+	    (inc > 0 && dev->promiscuity + inc < dev->promiscuity)) {
+		pr_warn("%s: increase promiscuity %u by %d failed, promiscuity feature of device might be broken.\n",
+			dev->name, dev->promiscuity, inc);
+		return -EOVERFLOW;
 	}
+
+	dev->promiscuity += inc;
+	if (dev->promiscuity)
+		dev->flags |= IFF_PROMISC;
+	else
+		dev->flags &= ~IFF_PROMISC;
+
 	if (dev->flags != old_flags) {
 		pr_info("device %s %s promiscuous mode\n",
 			dev->name,
@@ -7409,22 +7406,19 @@ static int __dev_set_allmulti(struct net_device *dev, int inc, bool notify)
 
 	ASSERT_RTNL();
 
-	dev->flags |= IFF_ALLMULTI;
-	dev->allmulti += inc;
-	if (dev->allmulti == 0) {
-		/*
-		 * Avoid overflow.
-		 * If inc causes overflow, untouch allmulti and return error.
-		 */
-		if (inc < 0)
-			dev->flags &= ~IFF_ALLMULTI;
-		else {
-			dev->allmulti -= inc;
-			pr_warn("%s: allmulti touches roof, set allmulti failed. allmulti feature of device might be broken.\n",
-				dev->name);
-			return -EOVERFLOW;
-		}
+	if ((inc < 0 && dev->allmulti + inc > dev->allmulti) ||
+	    (inc > 0 && dev->allmulti + inc < dev->allmulti)) {
+		pr_warn("%s: increase allmulti %u by %d failed, allmulti feature of device might be broken.\n",
+			dev->name, dev->allmulti, inc);
+		return -EOVERFLOW;
 	}
+
+	dev->allmulti += inc;
+	if (dev->allmulti)
+		dev->flags |= IFF_ALLMULTI;
+	else
+		dev->flags &= ~IFF_ALLMULTI;
+
 	if (dev->flags ^ old_flags) {
 		dev_change_rx_flags(dev, IFF_ALLMULTI);
 		dev_set_rx_mode(dev);
-- 
2.19.2