drivers/net/team/team_core.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
Syzkaller reports the following issue:
BUG: sleeping function called from invalid context in
team_change_rx_flags
3 locks held by syz.1.1814/12326:
#0: ffffffff8fa21eb8 (&ops->srcu#2){.+.+}-{0:0}, at: rcu_lock_acquire include/linux/rcupdate.h:331 [inline]
#0: ffffffff8fa21eb8 (&ops->srcu#2){.+.+}-{0:0}, at: rcu_read_lock include/linux/rcupdate.h:841 [inline]
#0: ffffffff8fa21eb8 (&ops->srcu#2){.+.+}-{0:0}, at: rtnl_link_ops_get+0x23/0x250 net/core/rtnetlink.c:570
#1: ffffffff8f51c5c8 (rtnl_mutex){+.+.}-{4:4}, at: rtnl_lock net/core/rtnetlink.c:80 [inline]
#1: ffffffff8f51c5c8 (rtnl_mutex){+.+.}-{4:4}, at: rtnl_nets_lock net/core/rtnetlink.c:341 [inline]
#1: ffffffff8f51c5c8 (rtnl_mutex){+.+.}-{4:4}, at: rtnl_newlink+0x8db/0x1c70 net/core/rtnetlink.c:4054
#2: ffff8880635e8368 (&macsec_netdev_addr_lock_key#2/2){+...}-{3:3}, at: netif_addr_lock_bh include/linux/netdevice.h:4805 [inline]
#2: ffff8880635e8368 (&macsec_netdev_addr_lock_key#2/2){+...}-{3:3}, at: dev_uc_add+0x67/0x120 net/core/dev_addr_lists.c:689
Preemption disabled at:
[<ffffffff895a7d26>] local_bh_disable include/linux/bottom_half.h:20 [inline]
^^^^
[<ffffffff895a7d26>] netif_addr_lock_bh include/linux/netdevice.h:4804 [inline]
[<ffffffff895a7d26>] dev_uc_add+0x56/0x120 net/core/dev_addr_lists.c:689
Call Trace:
<TASK>
dump_stack_lvl+0x189/0x250 lib/dump_stack.c:120
print_lock_invalid_wait_context kernel/locking/lockdep.c:4833 [inline]
check_wait_context kernel/locking/lockdep.c:4905 [inline]
__lock_acquire+0xbcb/0xd20 kernel/locking/lockdep.c:5190
lock_acquire+0x120/0x360 kernel/locking/lockdep.c:5871
__mutex_lock_common kernel/locking/mutex.c:602 [inline]
__mutex_lock+0x182/0xe80 kernel/locking/mutex.c:747
team_change_rx_flags+0x38/0x220 drivers/net/team/team_core.c:1781
dev_change_rx_flags net/core/dev.c:9241 [inline]
__dev_set_promiscuity+0x534/0x740 net/core/dev.c:9285
netif_set_promiscuity+0x50/0xe0 net/core/dev.c:9305
dev_set_promiscuity+0x126/0x260 net/core/dev_api.c:287
dev_change_rx_flags net/core/dev.c:9241 [inline]
__dev_set_promiscuity+0x534/0x740 net/core/dev.c:9285
__dev_set_rx_mode+0x17c/0x260 net/core/dev.c:-1
dev_uc_add+0xc8/0x120 net/core/dev_addr_lists.c:693
macsec_dev_open+0xd9/0x530 drivers/net/macsec.c:3634
__dev_open+0x470/0x880 net/core/dev.c:1683
__dev_change_flags+0x1ea/0x6d0 net/core/dev.c:9458
rtnl_configure_link net/core/rtnetlink.c:3577 [inline]
rtnl_newlink_create+0x555/0xb00 net/core/rtnetlink.c:3833
mutex_lock/mutex_unlock are called from team_change_rx_flags with
BH disabled (caused by netif_addr_lock_bh). Switch to spinlock instead
to avoid sleeping with BH disabled.
Reported-by: syzbot+8182574047912f805d59@syzkaller.appspotmail.com
Signed-off-by: Ujwal Kundur <ujwal.kundur@gmail.com>
---
drivers/net/team/team_core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/team/team_core.c b/drivers/net/team/team_core.c
index 8bc56186b2a3..4568075fea6e 100644
--- a/drivers/net/team/team_core.c
+++ b/drivers/net/team/team_core.c
@@ -1778,7 +1778,7 @@ static void team_change_rx_flags(struct net_device *dev, int change)
struct team_port *port;
int inc;
- mutex_lock(&team->lock);
+ spin_lock(&team->lock);
list_for_each_entry(port, &team->port_list, list) {
if (change & IFF_PROMISC) {
inc = dev->flags & IFF_PROMISC ? 1 : -1;
@@ -1789,7 +1789,7 @@ static void team_change_rx_flags(struct net_device *dev, int change)
dev_set_allmulti(port->dev, inc);
}
}
- mutex_unlock(&team->lock);
+ spin_unlock(&team->lock);
}
static void team_set_rx_mode(struct net_device *dev)
--
2.30.2Sun, Jul 27, 2025 at 08:09:21PM +0200, ujwal.kundur@gmail.com wrote:
>Syzkaller reports the following issue:
>BUG: sleeping function called from invalid context in
>team_change_rx_flags
>
>3 locks held by syz.1.1814/12326:
> #0: ffffffff8fa21eb8 (&ops->srcu#2){.+.+}-{0:0}, at: rcu_lock_acquire include/linux/rcupdate.h:331 [inline]
> #0: ffffffff8fa21eb8 (&ops->srcu#2){.+.+}-{0:0}, at: rcu_read_lock include/linux/rcupdate.h:841 [inline]
> #0: ffffffff8fa21eb8 (&ops->srcu#2){.+.+}-{0:0}, at: rtnl_link_ops_get+0x23/0x250 net/core/rtnetlink.c:570
> #1: ffffffff8f51c5c8 (rtnl_mutex){+.+.}-{4:4}, at: rtnl_lock net/core/rtnetlink.c:80 [inline]
> #1: ffffffff8f51c5c8 (rtnl_mutex){+.+.}-{4:4}, at: rtnl_nets_lock net/core/rtnetlink.c:341 [inline]
> #1: ffffffff8f51c5c8 (rtnl_mutex){+.+.}-{4:4}, at: rtnl_newlink+0x8db/0x1c70 net/core/rtnetlink.c:4054
> #2: ffff8880635e8368 (&macsec_netdev_addr_lock_key#2/2){+...}-{3:3}, at: netif_addr_lock_bh include/linux/netdevice.h:4805 [inline]
> #2: ffff8880635e8368 (&macsec_netdev_addr_lock_key#2/2){+...}-{3:3}, at: dev_uc_add+0x67/0x120 net/core/dev_addr_lists.c:689
>Preemption disabled at:
>[<ffffffff895a7d26>] local_bh_disable include/linux/bottom_half.h:20 [inline]
>^^^^
>[<ffffffff895a7d26>] netif_addr_lock_bh include/linux/netdevice.h:4804 [inline]
>[<ffffffff895a7d26>] dev_uc_add+0x56/0x120 net/core/dev_addr_lists.c:689
>Call Trace:
> <TASK>
> dump_stack_lvl+0x189/0x250 lib/dump_stack.c:120
> print_lock_invalid_wait_context kernel/locking/lockdep.c:4833 [inline]
> check_wait_context kernel/locking/lockdep.c:4905 [inline]
> __lock_acquire+0xbcb/0xd20 kernel/locking/lockdep.c:5190
> lock_acquire+0x120/0x360 kernel/locking/lockdep.c:5871
> __mutex_lock_common kernel/locking/mutex.c:602 [inline]
> __mutex_lock+0x182/0xe80 kernel/locking/mutex.c:747
> team_change_rx_flags+0x38/0x220 drivers/net/team/team_core.c:1781
> dev_change_rx_flags net/core/dev.c:9241 [inline]
> __dev_set_promiscuity+0x534/0x740 net/core/dev.c:9285
> netif_set_promiscuity+0x50/0xe0 net/core/dev.c:9305
> dev_set_promiscuity+0x126/0x260 net/core/dev_api.c:287
> dev_change_rx_flags net/core/dev.c:9241 [inline]
> __dev_set_promiscuity+0x534/0x740 net/core/dev.c:9285
> __dev_set_rx_mode+0x17c/0x260 net/core/dev.c:-1
> dev_uc_add+0xc8/0x120 net/core/dev_addr_lists.c:693
> macsec_dev_open+0xd9/0x530 drivers/net/macsec.c:3634
> __dev_open+0x470/0x880 net/core/dev.c:1683
> __dev_change_flags+0x1ea/0x6d0 net/core/dev.c:9458
> rtnl_configure_link net/core/rtnetlink.c:3577 [inline]
> rtnl_newlink_create+0x555/0xb00 net/core/rtnetlink.c:3833
>
>mutex_lock/mutex_unlock are called from team_change_rx_flags with
>BH disabled (caused by netif_addr_lock_bh). Switch to spinlock instead
>to avoid sleeping with BH disabled.
>
>Reported-by: syzbot+8182574047912f805d59@syzkaller.appspotmail.com
>Signed-off-by: Ujwal Kundur <ujwal.kundur@gmail.com>
This is already fixed by:
bfb4fb77f9a8 ("team: replace team lock with rtnl lock")
>---
> drivers/net/team/team_core.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/net/team/team_core.c b/drivers/net/team/team_core.c
>index 8bc56186b2a3..4568075fea6e 100644
>--- a/drivers/net/team/team_core.c
>+++ b/drivers/net/team/team_core.c
>@@ -1778,7 +1778,7 @@ static void team_change_rx_flags(struct net_device *dev, int change)
> struct team_port *port;
> int inc;
>
>- mutex_lock(&team->lock);
>+ spin_lock(&team->lock);
> list_for_each_entry(port, &team->port_list, list) {
> if (change & IFF_PROMISC) {
> inc = dev->flags & IFF_PROMISC ? 1 : -1;
>@@ -1789,7 +1789,7 @@ static void team_change_rx_flags(struct net_device *dev, int change)
> dev_set_allmulti(port->dev, inc);
> }
> }
>- mutex_unlock(&team->lock);
>+ spin_unlock(&team->lock);
> }
>
> static void team_set_rx_mode(struct net_device *dev)
>--
>2.30.2
>
> drivers/net/team/team_core.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/team/team_core.c b/drivers/net/team/team_core.c
> index 8bc56186b2a3..4568075fea6e 100644
> --- a/drivers/net/team/team_core.c
> +++ b/drivers/net/team/team_core.c
> @@ -1778,7 +1778,7 @@ static void team_change_rx_flags(struct net_device *dev, int change)
> struct team_port *port;
> int inc;
>
> - mutex_lock(&team->lock);
> + spin_lock(&team->lock);
> list_for_each_entry(port, &team->port_list, list) {
> if (change & IFF_PROMISC) {
> inc = dev->flags & IFF_PROMISC ? 1 : -1;
> @@ -1789,7 +1789,7 @@ static void team_change_rx_flags(struct net_device *dev, int change)
> dev_set_allmulti(port->dev, inc);
> }
> }
> - mutex_unlock(&team->lock);
> + spin_unlock(&team->lock);
> }
void __sched mutex_unlock(struct mutex *lock)
static __always_inline void spin_unlock(spinlock_t *lock)
They take different parameters. So you need to also change the struct
team to change lock from mutex to a spinlock. And what about all the
other users of team->lock?
Did not compile this change? I doubt you did, or you would of get
warnings, maybe errors.
Andrew
> Did not compile this change? I doubt you did, or you would of get warnings, maybe errors. Ah sorry, I shouldn't have relied on static analysis -- clangd did not complain so I did not wait for the compilation to succeed. > And what about all the other users of team->lock? I see the mutex is defined in `struct team` and cannot be changed as I've proposed here. Would switching to a spinlock across the board degrade performance? From what I understand, the NDO for ndo_change_rx_flags doesn't seem to disable BHs unlike ndo_set_rx_mode [1][2] so this seems to occur only when a new unicast address is being added via dev_uc_add [3] (which does disable BHs). Comparing other operations that use mutex_lock / mutex_unlock, looks like a few of them do not have RCU protection for their NDOs requiring lock / unlock pairs in the code, but none of them disable BHs (AFAICT) apart from the operations dealing with unicast / multicast addressing. If this is indeed the case, perhaps we can use a dedicated spinlock for team_change_rx_flags? Or switch back to rcu_read_lock as I believe it's being used in team_set_rx_mode [4] for precisely this reason. To be honest, I do not understand the intent behind this change as mentioned in 6b1d3c5f675cc794a015138b115afff172fb4c58. P.S: I'm still trying to get my head around the net subsystem, so please let me know if I've misunderstood the whole thing. [1] https://www.kernel.org/doc/html/v6.3/networking/netdevices.html [2] https://elixir.bootlin.com/linux/v6.16-rc7/source/net/core/dev.c#L9382 [3] https://elixir.bootlin.com/linux/v6.16-rc7/source/net/core/dev_addr_lists.c#L677 [4] https://elixir.bootlin.com/linux/v6.16-rc7/source/drivers/net/team/team_core.c#L1795 Thanks, Ujwal
On Mon, Jul 28, 2025 at 10:55:13AM +0530, Ujwal Kundur wrote: > > Did not compile this change? I doubt you did, or you would of get warnings, maybe errors. > > Ah sorry, I shouldn't have relied on static analysis -- clangd did not > complain so I did not wait for the compilation to succeed. > > > And what about all the other users of team->lock? > > I see the mutex is defined in `struct team` and cannot be changed as > I've proposed here. Would switching to a spinlock across the board > degrade performance? Sorry, team is not my area of expertise. > >From what I understand, the NDO for ndo_change_rx_flags doesn't seem > to disable BHs unlike ndo_set_rx_mode [1][2] so this seems to occur > only when a new unicast address is being added via dev_uc_add [3] > (which does disable BHs). > Comparing other operations that use mutex_lock / mutex_unlock, looks > like a few of them do not have RCU protection for their NDOs requiring > lock / unlock pairs in the code, but none of them disable BHs (AFAICT) > apart from the operations dealing with unicast / multicast addressing. > If this is indeed the case, perhaps we can use a dedicated spinlock > for team_change_rx_flags? Or switch back to rcu_read_lock as I believe > it's being used in team_set_rx_mode [4] for precisely this reason. To > be honest, I do not understand the intent behind this change as > mentioned in 6b1d3c5f675cc794a015138b115afff172fb4c58. I'm guessing, but is this about passing ndo_set_rx_mode from the upper device down to the lower devices? Maybe look at how this is implemented for other stacked devices. A VLAN interface on a base interface for example? A bridge interface on top of an interface. Andrew
> This is already fixed by:
> bfb4fb77f9a8 ("team: replace team lock with rtnl lock")
Thanks for letting me know. I now understand ASSERT_RTNL() is a viable
alternative.
> I'm guessing, but is this about passing ndo_set_rx_mode from the upper
> device down to the lower devices? Maybe look at how this is
> implemented for other stacked devices. A VLAN interface on a base
> interface for example? A bridge interface on top of an interface.
Oh this is about stacked devices, hence the references to "lower" and
"upper". Thanks, I'll read more about them.
© 2016 - 2025 Red Hat, Inc.