netdev - RFC: netdev: allow ethtool physical id to drop rtnl

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-next>] [day] [month] [year] [list]

Message-ID: <20091030104201.6bb03b23@nehalam>
Date:	Fri, 30 Oct 2009 10:42:01 -0700
From:	Stephen Hemminger <shemminger@...tta.com>
To:	David Miller <davem@...emloft.net>
Cc:	netdev@...r.kernel.org
Subject: RFC: netdev: allow ethtool physical id to drop rtnl_lock

The ethtool operation to blink LED can take an indeterminately long time,
blocking out other operations (via rtnl_lock). This patch is an attempt
to work around the problem.

It does need more discussion, because it will mean that drivers that formerly
were protected from changes during blink aren't.  For example, user could
start device blinking, and then plug in cable causing change netlink event
to change state or pull cable and have device come down.

The other possibility is to do this on a driver by driver basis
which is more effort.

Signed-off-by: Stephen Hemminger <shemminger@...tta.com>


--- a/net/core/ethtool.c	2009-10-30 10:27:23.621917624 -0700
+++ b/net/core/ethtool.c	2009-10-30 10:35:53.787670774 -0700
@@ -17,6 +17,7 @@
 #include <linux/errno.h>
 #include <linux/ethtool.h>
 #include <linux/netdevice.h>
+#include <linux/rtnetlink.h>
 #include <asm/uaccess.h>
 
 /*
@@ -781,6 +782,8 @@ static int ethtool_get_strings(struct ne
 static int ethtool_phys_id(struct net_device *dev, void __user *useraddr)
 {
 	struct ethtool_value id;
+	int err;
+	static int busy;
 
 	if (!dev->ethtool_ops->phys_id)
 		return -EOPNOTSUPP;
@@ -788,7 +791,21 @@ static int ethtool_phys_id(struct net_de
 	if (copy_from_user(&id, useraddr, sizeof(id)))
 		return -EFAULT;
 
-	return dev->ethtool_ops->phys_id(dev, id.data);
+	if (busy)
+		return -EBUSY;
+
+	/* This operation may take a long time, drop lock */
+	busy = 1;
+	dev_hold(dev);
+	rtnl_unlock();
+
+	err = dev->ethtool_ops->phys_id(dev, id.data);
+
+	rtnl_lock();
+	dev_put(dev);
+	busy = 0;
+
+	return err;
 }
 
 static int ethtool_get_stats(struct net_device *dev, void __user *useraddr)
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html