linux-kernel - [PATCH 5.6 032/194] net: fix a potential recursive NETDEV_FEAT

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: <20200518173534.232518410@linuxfoundation.org>
Date:   Mon, 18 May 2020 19:35:22 +0200
From:   Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To:     linux-kernel@...r.kernel.org
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        stable@...r.kernel.org,
        syzbot+e73ceacfd8560cc8a3ca@...kaller.appspotmail.com,
        syzbot+c2fb6f9ddcea95ba49b5@...kaller.appspotmail.com,
        Jarod Wilson <jarod@...hat.com>,
        Nikolay Aleksandrov <nikolay@...ulusnetworks.com>,
        Josh Poimboeuf <jpoimboe@...hat.com>,
        Jann Horn <jannh@...gle.com>,
        Jay Vosburgh <jay.vosburgh@...onical.com>,
        Cong Wang <xiyou.wangcong@...il.com>,
        "David S. Miller" <davem@...emloft.net>
Subject: [PATCH 5.6 032/194] net: fix a potential recursive NETDEV_FEAT_CHANGE

From: Cong Wang <xiyou.wangcong@...il.com>

[ Upstream commit dd912306ff008891c82cd9f63e8181e47a9cb2fb ]

syzbot managed to trigger a recursive NETDEV_FEAT_CHANGE event
between bonding master and slave. I managed to find a reproducer
for this:

  ip li set bond0 up
  ifenslave bond0 eth0
  brctl addbr br0
  ethtool -K eth0 lro off
  brctl addif br0 bond0
  ip li set br0 up

When a NETDEV_FEAT_CHANGE event is triggered on a bonding slave,
it captures this and calls bond_compute_features() to fixup its
master's and other slaves' features. However, when syncing with
its lower devices by netdev_sync_lower_features() this event is
triggered again on slaves when the LRO feature fails to change,
so it goes back and forth recursively until the kernel stack is
exhausted.

Commit 17b85d29e82c intentionally lets __netdev_update_features()
return -1 for such a failure case, so we have to just rely on
the existing check inside netdev_sync_lower_features() and skip
NETDEV_FEAT_CHANGE event only for this specific failure case.

Fixes: fd867d51f889 ("net/core: generic support for disabling netdev features down stack")
Reported-by: syzbot+e73ceacfd8560cc8a3ca@...kaller.appspotmail.com
Reported-by: syzbot+c2fb6f9ddcea95ba49b5@...kaller.appspotmail.com
Cc: Jarod Wilson <jarod@...hat.com>
Cc: Nikolay Aleksandrov <nikolay@...ulusnetworks.com>
Cc: Josh Poimboeuf <jpoimboe@...hat.com>
Cc: Jann Horn <jannh@...gle.com>
Reviewed-by: Jay Vosburgh <jay.vosburgh@...onical.com>
Signed-off-by: Cong Wang <xiyou.wangcong@...il.com>
Acked-by: Nikolay Aleksandrov <nikolay@...ulusnetworks.com>
Signed-off-by: David S. Miller <davem@...emloft.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
---
 net/core/dev.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -8890,11 +8890,13 @@ static void netdev_sync_lower_features(s
 			netdev_dbg(upper, "Disabling feature %pNF on lower dev %s.\n",
 				   &feature, lower->name);
 			lower->wanted_features &= ~feature;
-			netdev_update_features(lower);
+			__netdev_update_features(lower);
 
 			if (unlikely(lower->features & feature))
 				netdev_WARN(upper, "failed to disable %pNF on %s!\n",
 					    &feature, lower->name);
+			else
+				netdev_features_change(lower);
 		}
 	}
 }