| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20250204230057.1270362-4-sdf@fomichev.me>
Date: Tue, 4 Feb 2025 15:00:56 -0800
From: Stanislav Fomichev <sdf@...ichev.me>
To: netdev@...r.kernel.org
Cc: davem@...emloft.net,
edumazet@...gle.com,
kuba@...nel.org,
pabeni@...hat.com,
Saeed Mahameed <saeed@...nel.org>
Subject: [RFC net-next 3/4] net: Hold netdev instance lock for more NDOs
Convert all ndo_eth_ioctl invocations to dev_eth_ioctl which does the
locking. Reflow some of the dev_siocxxx to drop else clause.
Fix tabs vs spaces in neighboring lines while at it..
Remove rtnl_lock from ndo_get_stats and clarify that read path can
race with the write path. Still shaper-only drivers (iavf/netdevim).
Cc: Saeed Mahameed <saeed@...nel.org>
Signed-off-by: Stanislav Fomichev <sdf@...ichev.me>
---
Documentation/networking/netdevices.rst | 28 +++++++++-------
drivers/net/bonding/bond_main.c | 9 +++--
include/linux/netdevice.h | 2 ++
net/8021q/vlan_dev.c | 4 +--
net/core/dev_ioctl.c | 44 +++++++++++++++++--------
net/ieee802154/socket.c | 2 ++
net/phonet/pn_dev.c | 2 ++
7 files changed, 58 insertions(+), 33 deletions(-)
diff --git a/Documentation/networking/netdevices.rst b/Documentation/networking/netdevices.rst
index c6087d92d740..3ed1bf322a5c 100644
--- a/Documentation/networking/netdevices.rst
+++ b/Documentation/networking/netdevices.rst
@@ -221,40 +221,46 @@ struct net_device synchronization rules
Note: netif_running() is guaranteed false
ndo_do_ioctl:
- Synchronization: rtnl_lock() semaphore.
+ Synchronization: rtnl_lock() semaphore. In addition, netdev instance
+ lock if the driver implements shaper API.
Context: process
- This is only called by network subsystems internally,
- not by user space calling ioctl as it was in before
- linux-5.14.
+ This is only called by network subsystems internally,
+ not by user space calling ioctl as it was in before
+ linux-5.14.
ndo_siocbond:
- Synchronization: rtnl_lock() semaphore.
+ Synchronization: rtnl_lock() semaphore. In addition, netdev instance
+ lock if the driver implements shaper API.
Context: process
- Used by the bonding driver for the SIOCBOND family of
- ioctl commands.
+ Used by the bonding driver for the SIOCBOND family of
+ ioctl commands.
ndo_siocwandev:
- Synchronization: rtnl_lock() semaphore.
+ Synchronization: rtnl_lock() semaphore. In addition, netdev instance
+ lock if the driver implements shaper API.
Context: process
Used by the drivers/net/wan framework to handle
the SIOCWANDEV ioctl with the if_settings structure.
ndo_siocdevprivate:
- Synchronization: rtnl_lock() semaphore.
+ Synchronization: rtnl_lock() semaphore. In addition, netdev instance
+ lock if the driver implements shaper API.
Context: process
This is used to implement SIOCDEVPRIVATE ioctl helpers.
These should not be added to new drivers, so don't use.
ndo_eth_ioctl:
- Synchronization: rtnl_lock() semaphore.
+ Synchronization: rtnl_lock() semaphore. In addition, netdev instance
+ lock if the driver implements shaper API.
Context: process
ndo_get_stats:
- Synchronization: rtnl_lock() semaphore, or RCU.
+ Synchronization: RCU (can be called concurrently with the stats
+ update path).
Context: atomic (can't sleep under RCU)
ndo_setup_tc:
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index e45bba240cbc..025d605166c3 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -858,7 +858,6 @@ static int bond_check_dev_link(struct bonding *bond,
struct net_device *slave_dev, int reporting)
{
const struct net_device_ops *slave_ops = slave_dev->netdev_ops;
- int (*ioctl)(struct net_device *, struct ifreq *, int);
struct ifreq ifr;
struct mii_ioctl_data *mii;
@@ -874,8 +873,7 @@ static int bond_check_dev_link(struct bonding *bond,
BMSR_LSTATUS : 0;
/* Ethtool can't be used, fallback to MII ioctls. */
- ioctl = slave_ops->ndo_eth_ioctl;
- if (ioctl) {
+ if (slave_ops->ndo_eth_ioctl) {
/* TODO: set pointer to correct ioctl on a per team member
* bases to make this more efficient. that is, once
* we determine the correct ioctl, we will always
@@ -891,9 +889,10 @@ static int bond_check_dev_link(struct bonding *bond,
/* Yes, the mii is overlaid on the ifreq.ifr_ifru */
strscpy_pad(ifr.ifr_name, slave_dev->name, IFNAMSIZ);
mii = if_mii(&ifr);
- if (ioctl(slave_dev, &ifr, SIOCGMIIPHY) == 0) {
+
+ if (dev_eth_ioctl(slave_dev, &ifr, SIOCGMIIPHY) == 0) {
mii->reg_num = MII_BMSR;
- if (ioctl(slave_dev, &ifr, SIOCGMIIREG) == 0)
+ if (dev_eth_ioctl(slave_dev, &ifr, SIOCGMIIREG) == 0)
return mii->val_out & BMSR_LSTATUS;
}
}
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 6f2eb129ef3e..e49b818054b9 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -4159,6 +4159,8 @@ int put_user_ifreq(struct ifreq *ifr, void __user *arg);
int dev_ioctl(struct net *net, unsigned int cmd, struct ifreq *ifr,
void __user *data, bool *need_copyout);
int dev_ifconf(struct net *net, struct ifconf __user *ifc);
+int dev_eth_ioctl(struct net_device *dev,
+ struct ifreq *ifr, unsigned int cmd);
int generic_hwtstamp_get_lower(struct net_device *dev,
struct kernel_hwtstamp_config *kernel_cfg);
int generic_hwtstamp_set_lower(struct net_device *dev,
diff --git a/net/8021q/vlan_dev.c b/net/8021q/vlan_dev.c
index 91d134961357..ee3283400716 100644
--- a/net/8021q/vlan_dev.c
+++ b/net/8021q/vlan_dev.c
@@ -377,7 +377,6 @@ static int vlan_hwtstamp_set(struct net_device *dev,
static int vlan_dev_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
{
struct net_device *real_dev = vlan_dev_priv(dev)->real_dev;
- const struct net_device_ops *ops = real_dev->netdev_ops;
struct ifreq ifrr;
int err = -EOPNOTSUPP;
@@ -388,8 +387,7 @@ static int vlan_dev_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
case SIOCGMIIPHY:
case SIOCGMIIREG:
case SIOCSMIIREG:
- if (netif_device_present(real_dev) && ops->ndo_eth_ioctl)
- err = ops->ndo_eth_ioctl(real_dev, &ifrr, cmd);
+ err = dev_eth_ioctl(real_dev, &ifrr, cmd);
break;
}
diff --git a/net/core/dev_ioctl.c b/net/core/dev_ioctl.c
index 4c2098ac9d72..8dc2c323fe58 100644
--- a/net/core/dev_ioctl.c
+++ b/net/core/dev_ioctl.c
@@ -240,19 +240,23 @@ int net_hwtstamp_validate(const struct kernel_hwtstamp_config *cfg)
return 0;
}
-static int dev_eth_ioctl(struct net_device *dev,
- struct ifreq *ifr, unsigned int cmd)
+int dev_eth_ioctl(struct net_device *dev,
+ struct ifreq *ifr, unsigned int cmd)
{
const struct net_device_ops *ops = dev->netdev_ops;
+ int ret = -ENODEV;
if (!ops->ndo_eth_ioctl)
return -EOPNOTSUPP;
- if (!netif_device_present(dev))
- return -ENODEV;
+ netdev_lock_ops(dev);
+ if (netif_device_present(dev))
+ ret = ops->ndo_eth_ioctl(dev, ifr, cmd);
+ netdev_unlock_ops(dev);
- return ops->ndo_eth_ioctl(dev, ifr, cmd);
+ return ret;
}
+EXPORT_SYMBOL(dev_eth_ioctl);
/**
* dev_get_hwtstamp_phylib() - Get hardware timestamping settings of NIC
@@ -504,10 +508,14 @@ static int dev_siocbond(struct net_device *dev,
const struct net_device_ops *ops = dev->netdev_ops;
if (ops->ndo_siocbond) {
+ int ret = -ENODEV;
+
+ netdev_lock_ops(dev);
if (netif_device_present(dev))
- return ops->ndo_siocbond(dev, ifr, cmd);
- else
- return -ENODEV;
+ ret = ops->ndo_siocbond(dev, ifr, cmd);
+ netdev_unlock_ops(dev);
+
+ return ret;
}
return -EOPNOTSUPP;
@@ -519,10 +527,14 @@ static int dev_siocdevprivate(struct net_device *dev, struct ifreq *ifr,
const struct net_device_ops *ops = dev->netdev_ops;
if (ops->ndo_siocdevprivate) {
+ int ret = -ENODEV;
+
+ netdev_lock_ops(dev);
if (netif_device_present(dev))
- return ops->ndo_siocdevprivate(dev, ifr, data, cmd);
- else
- return -ENODEV;
+ ret = ops->ndo_siocdevprivate(dev, ifr, data, cmd);
+ netdev_unlock_ops(dev);
+
+ return ret;
}
return -EOPNOTSUPP;
@@ -533,10 +545,14 @@ static int dev_siocwandev(struct net_device *dev, struct if_settings *ifs)
const struct net_device_ops *ops = dev->netdev_ops;
if (ops->ndo_siocwandev) {
+ int ret = -ENODEV;
+
+ netdev_lock_ops(dev);
if (netif_device_present(dev))
- return ops->ndo_siocwandev(dev, ifs);
- else
- return -ENODEV;
+ ret = ops->ndo_siocwandev(dev, ifs);
+ netdev_unlock_ops(dev);
+
+ return ret;
}
return -EOPNOTSUPP;
diff --git a/net/ieee802154/socket.c b/net/ieee802154/socket.c
index 18d267921bb5..b86d090a2ba5 100644
--- a/net/ieee802154/socket.c
+++ b/net/ieee802154/socket.c
@@ -139,8 +139,10 @@ static int ieee802154_dev_ioctl(struct sock *sk, struct ifreq __user *arg,
if (!dev)
return -ENODEV;
+ netdev_lock_ops(dev);
if (dev->type == ARPHRD_IEEE802154 && dev->netdev_ops->ndo_do_ioctl)
ret = dev->netdev_ops->ndo_do_ioctl(dev, &ifr, cmd);
+ netdev_unlock_ops(dev);
if (!ret && put_user_ifreq(&ifr, arg))
ret = -EFAULT;
diff --git a/net/phonet/pn_dev.c b/net/phonet/pn_dev.c
index 5c36bae37b8f..945f2315499a 100644
--- a/net/phonet/pn_dev.c
+++ b/net/phonet/pn_dev.c
@@ -246,8 +246,10 @@ static int phonet_device_autoconf(struct net_device *dev)
if (!dev->netdev_ops->ndo_siocdevprivate)
return -EOPNOTSUPP;
+ netdev_lock_ops(dev);
ret = dev->netdev_ops->ndo_siocdevprivate(dev, (struct ifreq *)&req,
NULL, SIOCPNGAUTOCONF);
+ netdev_unlock_ops(dev);
if (ret < 0)
return ret;
--
2.48.1
Powered by blists - more mailing lists