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

[Sabrina Dubroca <sd@queasysnail.net>]

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