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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CALkFLL+qhX94cQfFhm7JFLE5s2JtEcgZnf_kfsaaE091xyzNvw@mail.gmail.com>
Date: Mon, 28 Jul 2025 10:55:13 +0530
From: Ujwal Kundur <ujwal.kundur@...il.com>
To: Andrew Lunn <andrew@...n.ch>
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

> 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