netdev - Re: [RFC PATCH] net: team: switch to spinlock in team_change_rx

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: <df06dba7-f5bd-4ee1-b9af-c5dd4b5d4434@lunn.ch>
Date: Mon, 28 Jul 2025 15:26:58 +0200
From: Andrew Lunn <andrew@...n.ch>
To: Ujwal Kundur <ujwal.kundur@...il.com>
Cc: syzbot+8182574047912f805d59@...kaller.appspotmail.com,
	davem@...emloft.net, edumazet@...gle.com, horms@...nel.org,
	kuba@...nel.org, linux-kernel@...r.kernel.org,
	netdev@...r.kernel.org, pabeni@...hat.com,
	syzkaller-bugs@...glegroups.com, jiri@...nulli.us,
	andrew+netdev@...n.ch
Subject: Re: [RFC PATCH] net: team: switch to spinlock in team_change_rx_flags

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