Skip to content

Commit f5e305e

Browse files
committed
Merge branch 'bonding-fixes'
Nikolay Aleksandrov says: ==================== bonding: properly restore flags when bond changes ether type A bug was reported by syzbot[1] that causes a warning and a myriad of other potential issues if a bond, that is also a slave, fails to enslave a non-eth device. While fixing that bug I found that we have the same issues when such enslave passes and after that the bond changes back to ARPHRD_ETHER (again due to ether_setup). This set fixes all issues by extracting the ether_setup() sequence in a helper which does the right thing about bond flags when it needs to change back to ARPHRD_ETHER. It also adds selftests for these cases. Patch 01 adds the new bond_ether_setup helper and fixes the issues when a bond device changes its ether type due to successful enslave. Patch 02 fixes the issues when it changes its ether type due to an unsuccessful enslave. Note we need two patches because the bugs were introduced by different commits. Patch 03 adds the new selftests. Due to the comment adjustment and squash, could you please review patch 01 again? I've kept the other acks since there were no code changes. v3: squash the helper patch and the first fix, adjust the comment above it to be explicit about the bond device, no code changes v2: new set, all patches are new due to new approach of fixing these bugs [1] https://syzkaller.appspot.com/bug?id=391c7b1f6522182899efba27d891f1743e8eb3ef ==================== Signed-off-by: David S. Miller <davem@davemloft.net>
2 parents 53515a0 + 222c94e commit f5e305e

File tree

3 files changed

+103
-8
lines changed

3 files changed

+103
-8
lines changed

drivers/net/bonding/bond_main.c

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1775,6 +1775,19 @@ void bond_lower_state_changed(struct slave *slave)
17751775
slave_err(bond_dev, slave_dev, "Error: %s\n", errmsg); \
17761776
} while (0)
17771777

1778+
/* The bonding driver uses ether_setup() to convert a master bond device
1779+
* to ARPHRD_ETHER, that resets the target netdevice's flags so we always
1780+
* have to restore the IFF_MASTER flag, and only restore IFF_SLAVE if it was set
1781+
*/
1782+
static void bond_ether_setup(struct net_device *bond_dev)
1783+
{
1784+
unsigned int slave_flag = bond_dev->flags & IFF_SLAVE;
1785+
1786+
ether_setup(bond_dev);
1787+
bond_dev->flags |= IFF_MASTER | slave_flag;
1788+
bond_dev->priv_flags &= ~IFF_TX_SKB_SHARING;
1789+
}
1790+
17781791
/* enslave device <slave> to bond device <master> */
17791792
int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
17801793
struct netlink_ext_ack *extack)
@@ -1866,10 +1879,8 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
18661879

18671880
if (slave_dev->type != ARPHRD_ETHER)
18681881
bond_setup_by_slave(bond_dev, slave_dev);
1869-
else {
1870-
ether_setup(bond_dev);
1871-
bond_dev->priv_flags &= ~IFF_TX_SKB_SHARING;
1872-
}
1882+
else
1883+
bond_ether_setup(bond_dev);
18731884

18741885
call_netdevice_notifiers(NETDEV_POST_TYPE_CHANGE,
18751886
bond_dev);
@@ -2289,9 +2300,7 @@ int bond_enslave(struct net_device *bond_dev, struct net_device *slave_dev,
22892300
eth_hw_addr_random(bond_dev);
22902301
if (bond_dev->type != ARPHRD_ETHER) {
22912302
dev_close(bond_dev);
2292-
ether_setup(bond_dev);
2293-
bond_dev->flags |= IFF_MASTER;
2294-
bond_dev->priv_flags &= ~IFF_TX_SKB_SHARING;
2303+
bond_ether_setup(bond_dev);
22952304
}
22962305
}
22972306

tools/testing/selftests/drivers/net/bonding/Makefile

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,8 @@ TEST_PROGS := \
88
dev_addr_lists.sh \
99
mode-1-recovery-updelay.sh \
1010
mode-2-recovery-updelay.sh \
11-
option_prio.sh
11+
option_prio.sh \
12+
bond-eth-type-change.sh
1213

1314
TEST_FILES := \
1415
lag_lib.sh \
Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
#!/bin/bash
2+
# SPDX-License-Identifier: GPL-2.0
3+
#
4+
# Test bond device ether type changing
5+
#
6+
7+
ALL_TESTS="
8+
bond_test_unsuccessful_enslave_type_change
9+
bond_test_successful_enslave_type_change
10+
"
11+
REQUIRE_MZ=no
12+
NUM_NETIFS=0
13+
lib_dir=$(dirname "$0")
14+
source "$lib_dir"/net_forwarding_lib.sh
15+
16+
bond_check_flags()
17+
{
18+
local bonddev=$1
19+
20+
ip -d l sh dev "$bonddev" | grep -q "MASTER"
21+
check_err $? "MASTER flag is missing from the bond device"
22+
23+
ip -d l sh dev "$bonddev" | grep -q "SLAVE"
24+
check_err $? "SLAVE flag is missing from the bond device"
25+
}
26+
27+
# test enslaved bond dev type change from ARPHRD_ETHER and back
28+
# this allows us to test both MASTER and SLAVE flags at once
29+
bond_test_enslave_type_change()
30+
{
31+
local test_success=$1
32+
local devbond0="test-bond0"
33+
local devbond1="test-bond1"
34+
local devbond2="test-bond2"
35+
local nonethdev="test-noneth0"
36+
37+
# create a non-ARPHRD_ETHER device for testing (e.g. nlmon type)
38+
ip link add name "$nonethdev" type nlmon
39+
check_err $? "could not create a non-ARPHRD_ETHER device (nlmon)"
40+
ip link add name "$devbond0" type bond
41+
if [ $test_success -eq 1 ]; then
42+
# we need devbond0 in active-backup mode to successfully enslave nonethdev
43+
ip link set dev "$devbond0" type bond mode active-backup
44+
check_err $? "could not change bond mode to active-backup"
45+
fi
46+
ip link add name "$devbond1" type bond
47+
ip link add name "$devbond2" type bond
48+
ip link set dev "$devbond0" master "$devbond1"
49+
check_err $? "could not enslave $devbond0 to $devbond1"
50+
# change bond type to non-ARPHRD_ETHER
51+
ip link set dev "$nonethdev" master "$devbond0" 1>/dev/null 2>/dev/null
52+
ip link set dev "$nonethdev" nomaster 1>/dev/null 2>/dev/null
53+
# restore ARPHRD_ETHER type by enslaving such device
54+
ip link set dev "$devbond2" master "$devbond0"
55+
check_err $? "could not enslave $devbond2 to $devbond0"
56+
ip link set dev "$devbond1" nomaster
57+
58+
bond_check_flags "$devbond0"
59+
60+
# clean up
61+
ip link del dev "$devbond0"
62+
ip link del dev "$devbond1"
63+
ip link del dev "$devbond2"
64+
ip link del dev "$nonethdev"
65+
}
66+
67+
bond_test_unsuccessful_enslave_type_change()
68+
{
69+
RET=0
70+
71+
bond_test_enslave_type_change 0
72+
log_test "Change ether type of an enslaved bond device with unsuccessful enslave"
73+
}
74+
75+
bond_test_successful_enslave_type_change()
76+
{
77+
RET=0
78+
79+
bond_test_enslave_type_change 1
80+
log_test "Change ether type of an enslaved bond device with successful enslave"
81+
}
82+
83+
tests_run
84+
85+
exit "$EXIT_STATUS"

0 commit comments

Comments
 (0)