| From 0f27b880bf7f54ba5b7401a7e44256ee03e9217a Mon Sep 17 00:00:00 2001 |
| From: Sasha Levin <sashal@kernel.org> |
| Date: Mon, 8 Sep 2025 10:36:14 -0700 |
| Subject: macsec: sync features on RTM_NEWLINK |
| |
| From: Stanislav Fomichev <sdf@fomichev.me> |
| |
| [ Upstream commit 0f82c3ba66c6b2e3cde0f255156a753b108ee9dc ] |
| |
| 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> |
| Reviewed-by: Sabrina Dubroca <sd@queasysnail.net> |
| Link: https://patch.msgid.link/20250908173614.3358264-1-sdf@fomichev.me |
| Signed-off-by: Jakub Kicinski <kuba@kernel.org> |
| Signed-off-by: Sasha Levin <sashal@kernel.org> |
| --- |
| drivers/net/macsec.c | 1 + |
| 1 file changed, 1 insertion(+) |
| |
| diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c |
| index 01329fe7451a1..0eca96eeed58a 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 |
| |