netdev - Re: [PATCH net-next v4] net: bonding: use atomic instead of rtnl

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: <dc26959a-ee51-4480-9a03-2fe4fc897f70@redhat.com>
Date: Thu, 13 Nov 2025 11:25:01 +0100
From: Paolo Abeni <pabeni@...hat.com>
To: Tonghao Zhang <tonghao@...aicloud.com>, netdev@...r.kernel.org
Cc: Jay Vosburgh <jv@...sburgh.net>, "David S. Miller" <davem@...emloft.net>,
 Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
 Simon Horman <horms@...nel.org>, Jonathan Corbet <corbet@....net>,
 Andrew Lunn <andrew+netdev@...n.ch>,
 Nikolay Aleksandrov <razor@...ckwall.org>, Hangbin Liu <liuhangbin@...il.com>
Subject: Re: [PATCH net-next v4] net: bonding: use atomic instead of
 rtnl_mutex, to make sure peer notify updated

On 11/10/25 10:13 AM, Tonghao Zhang wrote:
> @@ -814,4 +814,11 @@ static inline netdev_tx_t bond_tx_drop(struct net_device *dev, struct sk_buff *s
>  	return NET_XMIT_DROP;
>  }
>  
> +static inline void bond_peer_notify_reset(struct bonding *bond)
> +{
> +	atomic_set(&bond->send_peer_notif,
> +		bond->params.num_peer_notif *
> +		max(1, bond->params.peer_notif_delay));

The above reads params.* without any due lock; at very least it should
include READ_ONCE() annotation here (and WRITE_ONCE on the param update
side).

But it also mean it could observe inconsistent values for
params.num_peer_notif and params.peer_notif_delay in case of concurrent
update.

The possible race between bond_mii_monitor() and
bond_change_active_slave() concurrently updating
send_peer_notif is still avoided by the rtnl lock, so the changelog is
IMHO confusing WRT the actual code semantic.

/P