| Message ID | 20250410161117.3519250-1-sdf@fomichev.me (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | f7a11cba0ed79d9d37941dddf69a8a655c8644bc |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net,v2] bonding: hold ops lock around get_link | expand |
Hello: This patch was applied to netdev/net.git (main) by Jakub Kicinski <kuba@kernel.org>: On Thu, 10 Apr 2025 09:11:17 -0700 you wrote: > syzbot reports a case of ethtool_ops->get_link being called without > ops lock: > > ethtool_op_get_link+0x15/0x60 net/ethtool/ioctl.c:63 > bond_check_dev_link+0x1fb/0x4b0 drivers/net/bonding/bond_main.c:864 > bond_miimon_inspect drivers/net/bonding/bond_main.c:2734 [inline] > bond_mii_monitor+0x49d/0x3170 drivers/net/bonding/bond_main.c:2956 > process_one_work kernel/workqueue.c:3238 [inline] > process_scheduled_works+0xac3/0x18e0 kernel/workqueue.c:3319 > worker_thread+0x870/0xd50 kernel/workqueue.c:3400 > kthread+0x7b7/0x940 kernel/kthread.c:464 > ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:153 > ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:245 > > [...] Here is the summary with links: - [net,v2] bonding: hold ops lock around get_link https://git.kernel.org/netdev/net/c/f7a11cba0ed7 You are awesome, thank you!
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c index 950d8e4d86f8..8ea183da8d53 100644 --- a/drivers/net/bonding/bond_main.c +++ b/drivers/net/bonding/bond_main.c @@ -850,8 +850,9 @@ static int bond_check_dev_link(struct bonding *bond, struct net_device *slave_dev, int reporting) { const struct net_device_ops *slave_ops = slave_dev->netdev_ops; - struct ifreq ifr; struct mii_ioctl_data *mii; + struct ifreq ifr; + int ret; if (!reporting && !netif_running(slave_dev)) return 0; @@ -860,9 +861,13 @@ static int bond_check_dev_link(struct bonding *bond, return netif_carrier_ok(slave_dev) ? BMSR_LSTATUS : 0; /* Try to get link status using Ethtool first. */ - if (slave_dev->ethtool_ops->get_link) - return slave_dev->ethtool_ops->get_link(slave_dev) ? - BMSR_LSTATUS : 0; + if (slave_dev->ethtool_ops->get_link) { + netdev_lock_ops(slave_dev); + ret = slave_dev->ethtool_ops->get_link(slave_dev); + netdev_unlock_ops(slave_dev); + + return ret ? BMSR_LSTATUS : 0; + } /* Ethtool can't be used, fallback to MII ioctls. */ if (slave_ops->ndo_eth_ioctl) {
syzbot reports a case of ethtool_ops->get_link being called without ops lock: ethtool_op_get_link+0x15/0x60 net/ethtool/ioctl.c:63 bond_check_dev_link+0x1fb/0x4b0 drivers/net/bonding/bond_main.c:864 bond_miimon_inspect drivers/net/bonding/bond_main.c:2734 [inline] bond_mii_monitor+0x49d/0x3170 drivers/net/bonding/bond_main.c:2956 process_one_work kernel/workqueue.c:3238 [inline] process_scheduled_works+0xac3/0x18e0 kernel/workqueue.c:3319 worker_thread+0x870/0xd50 kernel/workqueue.c:3400 kthread+0x7b7/0x940 kernel/kthread.c:464 ret_from_fork+0x4b/0x80 arch/x86/kernel/process.c:153 ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:245 Commit 04efcee6ef8d ("net: hold instance lock during NETDEV_CHANGE") changed to lockless __linkwatch_sync_dev in ethtool_op_get_link. All paths except bonding are coming via locked ioctl. Add necessary locking to bonding. Reviewed-by: Hangbin Liu <liuhangbin@gmail.com> Reported-by: syzbot+48c14f61594bdfadb086@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=48c14f61594bdfadb086 Fixes: 04efcee6ef8d ("net: hold instance lock during NETDEV_CHANGE") Signed-off-by: Stanislav Fomichev <sdf@fomichev.me> --- v2: - move 'BMSR_LSTATUS : 0' part out (Jakub) --- drivers/net/bonding/bond_main.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-)