Skip to content

Commit f7a11cb

Browse files
Stanislav Fomichevkuba-moo
authored andcommitted
bonding: hold ops lock around get_link
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 04efcee ("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 <[email protected]> Reported-by: [email protected] Closes: https://syzkaller.appspot.com/bug?extid=48c14f61594bdfadb086 Fixes: 04efcee ("net: hold instance lock during NETDEV_CHANGE") Signed-off-by: Stanislav Fomichev <[email protected]> Link: https://patch.msgid.link/[email protected] Signed-off-by: Jakub Kicinski <[email protected]>
1 parent 52024cd commit f7a11cb

File tree

1 file changed

+9
-4
lines changed

1 file changed

+9
-4
lines changed

drivers/net/bonding/bond_main.c

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -850,8 +850,9 @@ static int bond_check_dev_link(struct bonding *bond,
850850
struct net_device *slave_dev, int reporting)
851851
{
852852
const struct net_device_ops *slave_ops = slave_dev->netdev_ops;
853-
struct ifreq ifr;
854853
struct mii_ioctl_data *mii;
854+
struct ifreq ifr;
855+
int ret;
855856

856857
if (!reporting && !netif_running(slave_dev))
857858
return 0;
@@ -860,9 +861,13 @@ static int bond_check_dev_link(struct bonding *bond,
860861
return netif_carrier_ok(slave_dev) ? BMSR_LSTATUS : 0;
861862

862863
/* Try to get link status using Ethtool first. */
863-
if (slave_dev->ethtool_ops->get_link)
864-
return slave_dev->ethtool_ops->get_link(slave_dev) ?
865-
BMSR_LSTATUS : 0;
864+
if (slave_dev->ethtool_ops->get_link) {
865+
netdev_lock_ops(slave_dev);
866+
ret = slave_dev->ethtool_ops->get_link(slave_dev);
867+
netdev_unlock_ops(slave_dev);
868+
869+
return ret ? BMSR_LSTATUS : 0;
870+
}
866871

867872
/* Ethtool can't be used, fallback to MII ioctls. */
868873
if (slave_ops->ndo_eth_ioctl) {

0 commit comments

Comments
 (0)