netdev - Re: [PATCH net-next] bonding: Remove support for use

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: <1934950.1750126872@famine>
Date: Mon, 16 Jun 2025 19:21:12 -0700
From: Jay Vosburgh <jv@...sburgh.net>
To: Stanislav Fomichev <stfomichev@...il.com>
cc: netdev@...r.kernel.org, "David S. Miller" <davem@...emloft.net>,
    Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
    Paolo Abeni <pabeni@...hat.com>, Simon Horman <horms@...nel.org>,
    Jonathan Corbet <corbet@....net>,
    Andrew Lunn <andrew+netdev@...n.ch>,
    Stanislav Fomichev <sdf@...ichev.me>,
    Hangbin Liu <liuhangbin@...il.com>, linux-doc@...r.kernel.org
Subject: Re: [PATCH net-next] bonding: Remove support for use_carrier = 0

Stanislav Fomichev <stfomichev@...il.com> wrote:

>On 06/16, Jay Vosburgh wrote:
>> 	 Remove the ability to disable use_carrier in bonding, and remove
>> all code related to the old link state check that utilizes ethtool or
>> ioctl to determine the link state of an interface in a bond.
>> 
>> 	To avoid acquiring RTNL many times per second, bonding's miimon
>> link monitor inspects link state under RCU, but not under RTNL.  However,
>> ethtool implementations in drivers may sleep, and therefore the ethtool or
>> ioctl strategy is unsuitable for use with calls into driver ethtool
>> functions.
>> 
>> 	The use_carrier option was introduced in 2003, to provide
>> backwards compatibility for network device drivers that did not support
>> the then-new netif_carrier_ok/on/off system.  Today, device drivers are
>> expected to support netif_carrier_*, and the use_carrier backwards
>> compatibility logic is no longer necessary.
>> 
>> 	Bonding now always behaves as if use_carrier=1, which relies on
>> netif_carrier_ok() to determine the link state of interfaces.  This has
>> been the default setting for use_carrier since its introduction.  For
>> backwards compatibility, the option itself remains, but may only be set to
>> 1, and queries will always return 1.
>> 
>> Reported-by: syzbot+b8c48ea38ca27d150063@...kaller.appspotmail.com
>> Closes: https://syzkaller.appspot.com/bug?extid=b8c48ea38ca27d150063
>> Link: https://lore.kernel.org/lkml/000000000000eb54bf061cfd666a@google.com/
>> Link: https://lore.kernel.org/netdev/20240718122017.d2e33aaac43a.I10ab9c9ded97163aef4e4de10985cd8f7de60d28@changeid/
>> Link: http://lore.kernel.org/netdev/aEt6LvBMwUMxmUyx@mini-arch
>> Signed-off-by: Jay Vosburgh <jv@...sburgh.net>
>
>Acked-by: Stanislav Fomichev <sdf@...ichev.me>
>
>Maybe better to target 'net' with the following?
>Fixes: f7a11cba0ed7 ("bonding: hold ops lock around get_link")

	I targeted net-next and left the Fixes: tag off on purpose.

	First, the bug this nominally fixes is many years old, and
wasn't introduced by f7a11cba0ed7.

	More importantly, though, this patch is removing functionality
that someone theoretically could be relying on, and I don't think such
removals should happen in the middle of a stable series.  The default
setting for use_carrier (i.e., using netif_carrier) will never hit the
issue in practice, so the exposure seems to be minimal for common use.

	-J

---
	-Jay Vosburgh, jv@...sburgh.net