| From foo@baz Sat 16 May 2020 02:04:40 PM CEST |
| From: Cong Wang <xiyou.wangcong@gmail.com> |
| Date: Thu, 7 May 2020 12:19:03 -0700 |
| Subject: net: fix a potential recursive NETDEV_FEAT_CHANGE |
| |
| From: Cong Wang <xiyou.wangcong@gmail.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@syzkaller.appspotmail.com |
| Reported-by: syzbot+c2fb6f9ddcea95ba49b5@syzkaller.appspotmail.com |
| Cc: Jarod Wilson <jarod@redhat.com> |
| Cc: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> |
| Cc: Josh Poimboeuf <jpoimboe@redhat.com> |
| Cc: Jann Horn <jannh@google.com> |
| Reviewed-by: Jay Vosburgh <jay.vosburgh@canonical.com> |
| Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com> |
| Acked-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com> |
| Signed-off-by: David S. Miller <davem@davemloft.net> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.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); |
| } |
| } |
| } |