Skip to content

Commit 803d39a

Browse files
idoschgregkh
authored andcommitted
bridge: Fix sleep in atomic context in netlink path
[ Upstream commit 5eec442 ] Since the introduction of the netlink configuration path for bridge ports in commit 25c71c7 ("bridge: bridge port parameters over netlink"), br_setport() was always called with the bridge lock held around it. Back then this decision made sense: The bridge lock protects the STP state of the bridge and its ports and at that time the function only processed three STP related netlink attributes (cost, priority and state). Nowadays, br_setport() processes a lot more attributes and most of them do not need the bridge lock: * Bridge flags: Only require RTNL. Read locklessly by the data path. Annotations can be added in net-next. * FDB port flushing: Only requires the FDB lock. * Multicast attributes: Only require the multicast lock. * Group forward mask: Only requires RTNL. Read locklessly by the data path. Annotations can be added in net-next. * Backup port and NHID: Only require RTNL. Read locklessly by the data path. This is a problem as the bridge calls dev_set_promiscuity() when certain bridge port flags change and this function can sleep since the commit cited below, resulting in a splat such as [1]. Fix this by reducing the scope of the bridge lock and only take it when processing the three STP related attributes that require it. This is consistent with the multicast attributes where each attribute acquires the multicast lock instead of having one critical section for all relevant attributes. [1] BUG: sleeping function called from invalid context at net/core/dev_addr_lists.c:1262 in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 356, name: bridge preempt_count: 201, expected: 0 RCU nest depth: 0, expected: 0 2 locks held by bridge/356: #0: ffffffff919473a0 (rtnl_mutex){+.+.}-{4:4}, at: rtnetlink_rcv_msg (net/core/rtnetlink.c:80 net/core/rtnetlink.c:7002) #1: ffff888115072d58 (&br->lock){+...}-{3:3}, at: br_setlink (./include/linux/spinlock.h:348 net/bridge/br_netlink.c:1117) Preemption disabled at: 0x0 Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011 Call Trace: <TASK> dump_stack_lvl (lib/dump_stack.c:94 lib/dump_stack.c:120) __might_resched.cold (kernel/sched/core.c:9163) netif_rx_mode_run (net/core/dev_addr_lists.c:1262) netif_rx_mode_sync (net/core/dev_addr_lists.c:1428) dev_set_promiscuity (net/core/dev_api.c:289) br_manage_promisc (net/bridge/br_if.c:135 net/bridge/br_if.c:172) br_port_flags_change (net/bridge/br_if.c:242 net/bridge/br_if.c:747) br_setport (net/bridge/br_netlink.c:1000) br_setlink (net/bridge/br_netlink.c:1118) rtnl_bridge_setlink (net/core/rtnetlink.c:5572) rtnetlink_rcv_msg (net/core/rtnetlink.c:7005) netlink_rcv_skb (net/netlink/af_netlink.c:2550) netlink_unicast (net/netlink/af_netlink.c:1318 net/netlink/af_netlink.c:1344) netlink_sendmsg (net/netlink/af_netlink.c:1894) __sock_sendmsg (net/socket.c:787 (discriminator 4) net/socket.c:802 (discriminator 4)) ____sys_sendmsg (net/socket.c:2698) ___sys_sendmsg (net/socket.c:2752) __sys_sendmsg (net/socket.c:2784) do_syscall_64 (arch/x86/entry/syscall_64.c:63 arch/x86/entry/syscall_64.c:94) entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:121) Fixes: 78cd408 ("net: add missing instance lock to dev_set_promiscuity") Reviewed-by: Nikolay Aleksandrov <nikolay@nvidia.com> Signed-off-by: Ido Schimmel <idosch@nvidia.com> Link: https://patch.msgid.link/20260526064818.272516-2-idosch@nvidia.com Signed-off-by: Jakub Kicinski <kuba@kernel.org> Signed-off-by: Sasha Levin <sashal@kernel.org>
1 parent 563090e commit 803d39a

1 file changed

Lines changed: 7 additions & 10 deletions

File tree

net/bridge/br_netlink.c

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1000,19 +1000,25 @@ static int br_setport(struct net_bridge_port *p, struct nlattr *tb[],
10001000
br_port_flags_change(p, changed_mask);
10011001

10021002
if (tb[IFLA_BRPORT_COST]) {
1003+
spin_lock_bh(&p->br->lock);
10031004
err = br_stp_set_path_cost(p, nla_get_u32(tb[IFLA_BRPORT_COST]));
1005+
spin_unlock_bh(&p->br->lock);
10041006
if (err)
10051007
return err;
10061008
}
10071009

10081010
if (tb[IFLA_BRPORT_PRIORITY]) {
1011+
spin_lock_bh(&p->br->lock);
10091012
err = br_stp_set_port_priority(p, nla_get_u16(tb[IFLA_BRPORT_PRIORITY]));
1013+
spin_unlock_bh(&p->br->lock);
10101014
if (err)
10111015
return err;
10121016
}
10131017

10141018
if (tb[IFLA_BRPORT_STATE]) {
1019+
spin_lock_bh(&p->br->lock);
10151020
err = br_set_port_state(p, nla_get_u8(tb[IFLA_BRPORT_STATE]));
1021+
spin_unlock_bh(&p->br->lock);
10161022
if (err)
10171023
return err;
10181024
}
@@ -1114,9 +1120,7 @@ int br_setlink(struct net_device *dev, struct nlmsghdr *nlh, u16 flags,
11141120
if (err)
11151121
return err;
11161122

1117-
spin_lock_bh(&p->br->lock);
11181123
err = br_setport(p, tb, extack);
1119-
spin_unlock_bh(&p->br->lock);
11201124
} else {
11211125
/* Binary compatibility with old RSTP */
11221126
if (nla_len(protinfo) < sizeof(u8))
@@ -1203,17 +1207,10 @@ static int br_port_slave_changelink(struct net_device *brdev,
12031207
struct nlattr *data[],
12041208
struct netlink_ext_ack *extack)
12051209
{
1206-
struct net_bridge *br = netdev_priv(brdev);
1207-
int ret;
1208-
12091210
if (!data)
12101211
return 0;
12111212

1212-
spin_lock_bh(&br->lock);
1213-
ret = br_setport(br_port_get_rtnl(dev), data, extack);
1214-
spin_unlock_bh(&br->lock);
1215-
1216-
return ret;
1213+
return br_setport(br_port_get_rtnl(dev), data, extack);
12171214
}
12181215

12191216
static int br_port_fill_slave_info(struct sk_buff *skb,

0 commit comments

Comments
 (0)