From: Johannes Berg <johannes.berg@intel.com> mainline inclusion from mainline-v6.7-rc3 commit facd15dfd69122042502d99ab8c9f888b48ee994 category: bugfix bugzilla: https://atomgit.com/openeuler/kernel/issues/9730 CVE: NA Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i... -------------------------------- There are multiple ways to query for the carrier state: through rtnetlink, sysfs, and (possibly) ethtool. Synchronize linkwatch work before these operations so that we don't have a situation where userspace queries the carrier state between the driver's carrier off->on transition and linkwatch running and expects it to work, when really (at least) TX cannot work until linkwatch has run. I previously posted a longer explanation of how this applies to wireless [1] but with this wireless can simply query the state before sending data, to ensure the kernel is ready for it. [1] https://lore.kernel.org/all/346b21d87c69f817ea3c37caceb34f1f56255884.camel@s... Signed-off-by: Johannes Berg <johannes.berg@intel.com> Reviewed-by: Jiri Pirko <jiri@nvidia.com> Link: https://lore.kernel.org/r/20231204214706.303c62768415.I1caedccae72ee5a45c908... Signed-off-by: Jakub Kicinski <kuba@kernel.org> Signed-off-by: Dong Chenchen <dongchenchen2@huawei.com> --- include/linux/netdevice.h | 9 +++++++++ net/core/dev.c | 2 +- net/core/dev.h | 1 - net/core/link_watch.c | 2 +- net/core/net-sysfs.c | 8 +++++++- net/core/rtnetlink.c | 8 ++++++++ net/ethtool/ioctl.c | 3 +++ 7 files changed, 29 insertions(+), 4 deletions(-) diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h index 2bdce0b48965..2cda65c6006f 100644 --- a/include/linux/netdevice.h +++ b/include/linux/netdevice.h @@ -4245,18 +4245,27 @@ static inline void netdev_ref_replace(struct net_device *odev, * and _off may be called from IRQ context, but it is caller * who is responsible for serialization of these calls. * * The name carrier is inappropriate, these functions should really be * called netif_lowerlayer_*() because they represent the state of any * kind of lower layer not just hardware media. */ void linkwatch_fire_event(struct net_device *dev); +/** + * linkwatch_sync_dev - sync linkwatch for the given device + * @dev: network device to sync linkwatch for + * + * Sync linkwatch for the given device, removing it from the + * pending work list (if queued). + */ +void linkwatch_sync_dev(struct net_device *dev); + /** * netif_carrier_ok - test if carrier present * @dev: network device * * Check if carrier is present on device */ static inline bool netif_carrier_ok(const struct net_device *dev) { return !test_bit(__LINK_STATE_NOCARRIER, &dev->state); diff --git a/net/core/dev.c b/net/core/dev.c index a54d8761b86f..d21b3bf2700b 100644 --- a/net/core/dev.c +++ b/net/core/dev.c @@ -10751,19 +10751,19 @@ void netdev_run_todo(void) if (unlikely(dev->reg_state != NETREG_UNREGISTERING)) { netdev_WARN(dev, "run_todo but not unregistering\n"); list_del(&dev->todo_list); continue; } write_lock(&dev_base_lock); dev->reg_state = NETREG_UNREGISTERED; write_unlock(&dev_base_lock); - linkwatch_forget_dev(dev); + linkwatch_sync_dev(dev); } while (!list_empty(&list)) { dev = netdev_wait_allrefs_any(&list); list_del(&dev->todo_list); /* paranoia */ BUG_ON(netdev_refcnt_read(dev) != 1); BUG_ON(!list_empty(&dev->ptype_all)); diff --git a/net/core/dev.h b/net/core/dev.h index f2037d402144..8cb96532a7c7 100644 --- a/net/core/dev.h +++ b/net/core/dev.h @@ -24,19 +24,18 @@ struct sd_flow_limit { extern int netdev_flow_limit_table_len; #ifdef CONFIG_PROC_FS int __init dev_proc_init(void); #else #define dev_proc_init() 0 #endif void linkwatch_init_dev(struct net_device *dev); -void linkwatch_forget_dev(struct net_device *dev); void linkwatch_run_queue(void); void dev_addr_flush(struct net_device *dev); int dev_addr_init(struct net_device *dev); void dev_addr_check(struct net_device *dev); /* sysctls not referred to from outside net/core/ */ extern int netdev_budget; extern unsigned int netdev_budget_usecs; diff --git a/net/core/link_watch.c b/net/core/link_watch.c index e2e8a341318b..423f8583ebdd 100644 --- a/net/core/link_watch.c +++ b/net/core/link_watch.c @@ -248,19 +248,19 @@ static void __linkwatch_run_queue(int urgent_only) /* Add the remaining work back to lweventlist */ list_splice_init(&wrk, &lweventlist); if (!list_empty(&lweventlist)) linkwatch_schedule_work(0); spin_unlock_irq(&lweventlist_lock); } -void linkwatch_forget_dev(struct net_device *dev) +void linkwatch_sync_dev(struct net_device *dev) { unsigned long flags; int clean = 0; spin_lock_irqsave(&lweventlist_lock, flags); if (!list_empty(&dev->link_watch_list)) { list_del_init(&dev->link_watch_list); clean = 1; /* We must release netdev tracker under diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c index f7404bc67974..97582c1d01c2 100644 --- a/net/core/net-sysfs.c +++ b/net/core/net-sysfs.c @@ -188,20 +188,26 @@ static ssize_t carrier_store(struct device *dev, struct device_attribute *attr, return netdev_store(dev, attr, buf, len, change_carrier); } static ssize_t carrier_show(struct device *dev, struct device_attribute *attr, char *buf) { struct net_device *netdev = to_net_dev(dev); - if (netif_running(netdev)) + if (netif_running(netdev)) { + /* Synchronize carrier state with link watch, + * see also rtnl_getlink(). + */ + linkwatch_sync_dev(netdev); + return sysfs_emit(buf, fmt_dec, !!netif_carrier_ok(netdev)); + } return -EINVAL; } static DEVICE_ATTR_RW(carrier); static ssize_t speed_show(struct device *dev, struct device_attribute *attr, char *buf) { struct net_device *netdev = to_net_dev(dev); diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c index dc82b95bb245..2296b3052732 100644 --- a/net/core/rtnetlink.c +++ b/net/core/rtnetlink.c @@ -3863,18 +3863,26 @@ static int rtnl_getlink(struct sk_buff *skb, struct nlmsghdr *nlh, err = -ENODEV; if (dev == NULL) goto out; err = -ENOBUFS; nskb = nlmsg_new(if_nlmsg_size(dev, ext_filter_mask), GFP_KERNEL); if (nskb == NULL) goto out; + /* Synchronize the carrier state so we don't report a state + * that we're not actually going to honour immediately; if + * the driver just did a carrier off->on transition, we can + * only TX if link watch work has run, but without this we'd + * already report carrier on, even if it doesn't work yet. + */ + linkwatch_sync_dev(dev); + err = rtnl_fill_ifinfo(nskb, dev, net, RTM_NEWLINK, NETLINK_CB(skb).portid, nlh->nlmsg_seq, 0, 0, ext_filter_mask, 0, NULL, 0, netnsid, GFP_KERNEL); if (err < 0) { /* -EMSGSIZE implies BUG in if_nlmsg_size */ WARN_ON(err == -EMSGSIZE); kfree_skb(nskb); } else diff --git a/net/ethtool/ioctl.c b/net/ethtool/ioctl.c index 6258b4f1bc0d..22be4dd167bb 100644 --- a/net/ethtool/ioctl.c +++ b/net/ethtool/ioctl.c @@ -52,18 +52,21 @@ static struct devlink *netdev_to_devlink_get(struct net_device *dev) /* * Some useful ethtool_ops methods that're device independent. * If we find that all drivers want to do the same thing here, * we can turn these into dev_() function calls. */ u32 ethtool_op_get_link(struct net_device *dev) { + /* Synchronize carrier state with link watch, see also rtnl_getlink() */ + linkwatch_sync_dev(dev); + return netif_carrier_ok(dev) ? 1 : 0; } EXPORT_SYMBOL(ethtool_op_get_link); int ethtool_op_get_ts_info(struct net_device *dev, struct ethtool_ts_info *info) { info->so_timestamping = SOF_TIMESTAMPING_TX_SOFTWARE | SOF_TIMESTAMPING_RX_SOFTWARE | -- 2.43.0