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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Tue, 28 Sep 2021 14:55:00 +0200
From:   Antoine Tenart <atenart@...nel.org>
To:     davem@...emloft.net, kuba@...nel.org
Cc:     Antoine Tenart <atenart@...nel.org>, pabeni@...hat.com,
        gregkh@...uxfoundation.org, ebiederm@...ssion.com,
        stephen@...workplumber.org, herbert@...dor.apana.org.au,
        juri.lelli@...hat.com, netdev@...r.kernel.org
Subject: [RFC PATCH net-next 9/9] net-sysfs: remove the use of rtnl_trylock/restart_syscall

The ABBA deadlock avoided by using rtnl_trylock and restart_syscall was
fixed in previous commits, we can now remove the use of this
trylock/restart logic and have net-sysfs operations not spinning when
rtnl is already taken.

Signed-off-by: Antoine Tenart <atenart@...nel.org>
---
 net/core/net-sysfs.c | 95 +++++++-------------------------------------
 1 file changed, 14 insertions(+), 81 deletions(-)

diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index e754f00c117b..987b32fd8604 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -90,9 +90,7 @@ static ssize_t netdev_store(struct device *dev, struct device_attribute *attr,
 	if (ret)
 		goto err;
 
-	if (!rtnl_trylock())
-		return restart_syscall();
-
+	rtnl_lock();
 	if (dev_isalive(netdev)) {
 		ret = (*set)(netdev, new);
 		if (ret == 0)
@@ -196,15 +194,7 @@ static ssize_t speed_show(struct device *dev,
 	struct net_device *netdev = to_net_dev(dev);
 	int ret = -EINVAL;
 
-	/* The check is also done in __ethtool_get_link_ksettings; this helps
-	 * returning early without hitting the trylock/restart below.
-	 */
-	if (!netdev->ethtool_ops->get_link_ksettings)
-		return -EOPNOTSUPP;
-
-	if (!rtnl_trylock())
-		return restart_syscall();
-
+	rtnl_lock();
 	if (netif_running(netdev)) {
 		struct ethtool_link_ksettings cmd;
 
@@ -222,15 +212,7 @@ static ssize_t duplex_show(struct device *dev,
 	struct net_device *netdev = to_net_dev(dev);
 	int ret = -EINVAL;
 
-	/* The check is also done in __ethtool_get_link_ksettings; this helps
-	 * returning early without hitting the trylock/restart below.
-	 */
-	if (!netdev->ethtool_ops->get_link_ksettings)
-		return -EOPNOTSUPP;
-
-	if (!rtnl_trylock())
-		return restart_syscall();
-
+	rtnl_lock();
 	if (netif_running(netdev)) {
 		struct ethtool_link_ksettings cmd;
 
@@ -427,9 +409,7 @@ static ssize_t ifalias_store(struct device *dev, struct device_attribute *attr,
 	if (len >  0 && buf[len - 1] == '\n')
 		--count;
 
-	if (!rtnl_trylock())
-		return restart_syscall();
-
+	rtnl_lock();
 	if (dev_isalive(netdev)) {
 		ret = dev_set_alias(netdev, buf, count);
 		if (ret < 0)
@@ -490,15 +470,7 @@ static ssize_t phys_port_id_show(struct device *dev,
 	struct net_device *netdev = to_net_dev(dev);
 	ssize_t ret = -EINVAL;
 
-	/* The check is also done in dev_get_phys_port_id; this helps returning
-	 * early without hitting the trylock/restart below.
-	 */
-	if (!netdev->netdev_ops->ndo_get_phys_port_id)
-		return -EOPNOTSUPP;
-
-	if (!rtnl_trylock())
-		return restart_syscall();
-
+	rtnl_lock();
 	if (dev_isalive(netdev)) {
 		struct netdev_phys_item_id ppid;
 
@@ -518,16 +490,7 @@ static ssize_t phys_port_name_show(struct device *dev,
 	struct net_device *netdev = to_net_dev(dev);
 	ssize_t ret = -EINVAL;
 
-	/* The checks are also done in dev_get_phys_port_name; this helps
-	 * returning early without hitting the trylock/restart below.
-	 */
-	if (!netdev->netdev_ops->ndo_get_phys_port_name &&
-	    !netdev->netdev_ops->ndo_get_devlink_port)
-		return -EOPNOTSUPP;
-
-	if (!rtnl_trylock())
-		return restart_syscall();
-
+	rtnl_lock();
 	if (dev_isalive(netdev)) {
 		char name[IFNAMSIZ];
 
@@ -547,16 +510,7 @@ static ssize_t phys_switch_id_show(struct device *dev,
 	struct net_device *netdev = to_net_dev(dev);
 	ssize_t ret = -EINVAL;
 
-	/* The checks are also done in dev_get_phys_port_name; this helps
-	 * returning early without hitting the trylock/restart below.
-	 */
-	if (!netdev->netdev_ops->ndo_get_port_parent_id &&
-	    !netdev->netdev_ops->ndo_get_devlink_port)
-		return -EOPNOTSUPP;
-
-	if (!rtnl_trylock())
-		return restart_syscall();
-
+	rtnl_lock();
 	if (dev_isalive(netdev)) {
 		struct netdev_phys_item_id ppid = { };
 
@@ -576,9 +530,7 @@ static ssize_t threaded_show(struct device *dev,
 	struct net_device *netdev = to_net_dev(dev);
 	ssize_t ret = -EINVAL;
 
-	if (!rtnl_trylock())
-		return restart_syscall();
-
+	rtnl_lock();
 	if (dev_isalive(netdev))
 		ret = sprintf(buf, fmt_dec, netdev->threaded);
 
@@ -1214,9 +1166,7 @@ static ssize_t traffic_class_show(struct netdev_queue *queue,
 	if (!netif_is_multiqueue(dev))
 		return -ENOENT;
 
-	if (!rtnl_trylock())
-		return restart_syscall();
-
+	rtnl_lock();
 	index = get_netdev_queue_index(queue);
 
 	/* If queue belongs to subordinate dev use its TC mapping */
@@ -1258,18 +1208,11 @@ static ssize_t tx_maxrate_store(struct netdev_queue *queue,
 	if (!capable(CAP_NET_ADMIN))
 		return -EPERM;
 
-	/* The check is also done later; this helps returning early without
-	 * hitting the trylock/restart below.
-	 */
-	if (!dev->netdev_ops->ndo_set_tx_maxrate)
-		return -EOPNOTSUPP;
-
 	err = kstrtou32(buf, 10, &rate);
 	if (err < 0)
 		return err;
 
-	if (!rtnl_trylock())
-		return restart_syscall();
+	rtnl_lock();
 
 	err = -EOPNOTSUPP;
 	if (dev->netdev_ops->ndo_set_tx_maxrate)
@@ -1460,8 +1403,7 @@ static ssize_t xps_cpus_show(struct netdev_queue *queue, char *buf)
 
 	index = get_netdev_queue_index(queue);
 
-	if (!rtnl_trylock())
-		return restart_syscall();
+	rtnl_lock();
 
 	/* If queue belongs to subordinate dev use its map */
 	dev = netdev_get_tx_queue(dev, index)->sb_dev ? : dev;
@@ -1507,11 +1449,7 @@ static ssize_t xps_cpus_store(struct netdev_queue *queue,
 		return err;
 	}
 
-	if (!rtnl_trylock()) {
-		free_cpumask_var(mask);
-		return restart_syscall();
-	}
-
+	rtnl_lock();
 	err = netif_set_xps_queue(dev, mask, index);
 	rtnl_unlock();
 
@@ -1531,9 +1469,7 @@ static ssize_t xps_rxqs_show(struct netdev_queue *queue, char *buf)
 
 	index = get_netdev_queue_index(queue);
 
-	if (!rtnl_trylock())
-		return restart_syscall();
-
+	rtnl_lock();
 	tc = netdev_txq_to_tc(dev, index);
 	rtnl_unlock();
 	if (tc < 0)
@@ -1566,10 +1502,7 @@ static ssize_t xps_rxqs_store(struct netdev_queue *queue, const char *buf,
 		return err;
 	}
 
-	if (!rtnl_trylock()) {
-		bitmap_free(mask);
-		return restart_syscall();
-	}
+	rtnl_lock();
 
 	cpus_read_lock();
 	err = __netif_set_xps_queue(dev, mask, index, XPS_RXQS);
-- 
2.31.1

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ