[PATCH net] macsec: sync features on RTM_NEWLINK

[PATCH net] macsec: sync features on RTM_NEWLINK

[Stanislav Fomichev]

Syzkaller managed to lock the lower device via ETHTOOL_SFEATURES:

 netdev_lock include/linux/netdevice.h:2761 [inline]
 netdev_lock_ops include/net/netdev_lock.h:42 [inline]
 netdev_sync_lower_features net/core/dev.c:10649 [inline]
 __netdev_update_features+0xcb1/0x1be0 net/core/dev.c:10819
 netdev_update_features+0x6d/0xe0 net/core/dev.c:10876
 macsec_notify+0x2f5/0x660 drivers/net/macsec.c:4533
 notifier_call_chain+0x1b3/0x3e0 kernel/notifier.c:85
 call_netdevice_notifiers_extack net/core/dev.c:2267 [inline]
 call_netdevice_notifiers net/core/dev.c:2281 [inline]
 netdev_features_change+0x85/0xc0 net/core/dev.c:1570
 __dev_ethtool net/ethtool/ioctl.c:3469 [inline]
 dev_ethtool+0x1536/0x19b0 net/ethtool/ioctl.c:3502
 dev_ioctl+0x392/0x1150 net/core/dev_ioctl.c:759

It happens because lower features are out of sync with the upper:

  __dev_ethtool (real_dev)
    netdev_lock_ops(real_dev)
    ETHTOOL_SFEATURES
      __netdev_features_change
        netdev_sync_upper_features
          disable LRO on the lower
    if (old_features != dev->features)
      netdev_features_change
        fires NETDEV_FEAT_CHANGE
	macsec_notify
	  NETDEV_FEAT_CHANGE
	    netdev_update_features (for each macsec dev)
	      netdev_sync_lower_features
	        if (upper_features != lower_features)
	          netdev_lock_ops(lower) # lower == real_dev
		  stuck
		  ...

    netdev_unlock_ops(real_dev)

Per commit af5f54b0ef9e ("net: Lock lower level devices when updating
features"), we elide the lock/unlock when the upper and lower features
are synced. Makes sure the lower (real_dev) has proper features after
the macsec link has been created. This makes sure we never hit the
situation where we need to sync upper flags to the lower.

Reported-by: syzbot+7e0f89fb6cae5d002de0@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=7e0f89fb6cae5d002de0
Fixes: 7e4d784f5810 ("net: hold netdev instance lock during rtnetlink operations")
Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
---
 drivers/net/macsec.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c
index 01329fe7451a..0eca96eeed58 100644
--- a/drivers/net/macsec.c
+++ b/drivers/net/macsec.c
@@ -4286,6 +4286,7 @@ static int macsec_newlink(struct net_device *dev,
 	if (err < 0)
 		goto del_dev;
 
+	netdev_update_features(dev);
 	netif_stacked_transfer_operstate(real_dev, dev);
 	linkwatch_fire_event(dev);
 
-- 
2.51.0

Re: [PATCH net] macsec: sync features on RTM_NEWLINK

[Sabrina Dubroca]

2025-09-08, 10:36:14 -0700, Stanislav Fomichev wrote:
> Syzkaller managed to lock the lower device via ETHTOOL_SFEATURES:
> 
>  netdev_lock include/linux/netdevice.h:2761 [inline]
>  netdev_lock_ops include/net/netdev_lock.h:42 [inline]
>  netdev_sync_lower_features net/core/dev.c:10649 [inline]
>  __netdev_update_features+0xcb1/0x1be0 net/core/dev.c:10819
>  netdev_update_features+0x6d/0xe0 net/core/dev.c:10876
>  macsec_notify+0x2f5/0x660 drivers/net/macsec.c:4533
>  notifier_call_chain+0x1b3/0x3e0 kernel/notifier.c:85
>  call_netdevice_notifiers_extack net/core/dev.c:2267 [inline]
>  call_netdevice_notifiers net/core/dev.c:2281 [inline]
>  netdev_features_change+0x85/0xc0 net/core/dev.c:1570
>  __dev_ethtool net/ethtool/ioctl.c:3469 [inline]
>  dev_ethtool+0x1536/0x19b0 net/ethtool/ioctl.c:3502
>  dev_ioctl+0x392/0x1150 net/core/dev_ioctl.c:759
> 
> It happens because lower features are out of sync with the upper:
> 
>   __dev_ethtool (real_dev)
>     netdev_lock_ops(real_dev)
>     ETHTOOL_SFEATURES
>       __netdev_features_change
>         netdev_sync_upper_features
>           disable LRO on the lower
>     if (old_features != dev->features)
>       netdev_features_change
>         fires NETDEV_FEAT_CHANGE
> 	macsec_notify
> 	  NETDEV_FEAT_CHANGE
> 	    netdev_update_features (for each macsec dev)
> 	      netdev_sync_lower_features
> 	        if (upper_features != lower_features)
> 	          netdev_lock_ops(lower) # lower == real_dev
> 		  stuck
> 		  ...
> 
>     netdev_unlock_ops(real_dev)
> 
> Per commit af5f54b0ef9e ("net: Lock lower level devices when updating
> features"), we elide the lock/unlock when the upper and lower features
> are synced. Makes sure the lower (real_dev) has proper features after
> the macsec link has been created. This makes sure we never hit the
> situation where we need to sync upper flags to the lower.
> 
> Reported-by: syzbot+7e0f89fb6cae5d002de0@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=7e0f89fb6cae5d002de0
> Fixes: 7e4d784f5810 ("net: hold netdev instance lock during rtnetlink operations")
> Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
> ---
>  drivers/net/macsec.c | 1 +
>  1 file changed, 1 insertion(+)

Thanks for the detailed explanation of the problem.

Reviewed-by: Sabrina Dubroca <sd@queasysnail.net>

-- 
Sabrina

Re: [PATCH net] macsec: sync features on RTM_NEWLINK

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Mon,  8 Sep 2025 10:36:14 -0700 you wrote:
> Syzkaller managed to lock the lower device via ETHTOOL_SFEATURES:
> 
>  netdev_lock include/linux/netdevice.h:2761 [inline]
>  netdev_lock_ops include/net/netdev_lock.h:42 [inline]
>  netdev_sync_lower_features net/core/dev.c:10649 [inline]
>  __netdev_update_features+0xcb1/0x1be0 net/core/dev.c:10819
>  netdev_update_features+0x6d/0xe0 net/core/dev.c:10876
>  macsec_notify+0x2f5/0x660 drivers/net/macsec.c:4533
>  notifier_call_chain+0x1b3/0x3e0 kernel/notifier.c:85
>  call_netdevice_notifiers_extack net/core/dev.c:2267 [inline]
>  call_netdevice_notifiers net/core/dev.c:2281 [inline]
>  netdev_features_change+0x85/0xc0 net/core/dev.c:1570
>  __dev_ethtool net/ethtool/ioctl.c:3469 [inline]
>  dev_ethtool+0x1536/0x19b0 net/ethtool/ioctl.c:3502
>  dev_ioctl+0x392/0x1150 net/core/dev_ioctl.c:759
> 
> [...]

Here is the summary with links:
  - [net] macsec: sync features on RTM_NEWLINK
    https://git.kernel.org/netdev/net/c/0f82c3ba66c6

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html