Re: [RFC] let mortals use ethtool

[Auke Kok]

Stephen Hemminger wrote:
There is no reason to not allow non-admin users to query network
statistics and settings.

--- linux-2.6.orig/net/core/ethtool.c
+++ linux-2.6/net/core/ethtool.c
@@ -806,13 +806,6 @@ int dev_ethtool(struct ifreq *ifr)
        int rc;
        unsigned long old_features;
 
-	/*
-        * XXX: This can be pushed down into the ethtool_* handlers that
-        * need it.  Keep existing behaviour for the moment.
-        */
-       if (!capable(CAP_NET_ADMIN))
-               return -EPERM;
-
        if (!dev || !netif_device_present(dev))
                return -ENODEV;
 
@@ -822,6 +815,33 @@ int dev_ethtool(struct ifreq *ifr)
        if (copy_from_user(&ethcmd, useraddr, sizeof (ethcmd)))
                return -EFAULT;
 
+	/* Allow some commands to be done by anyone */
+       switch(ethcmd) {
+       case ETHTOOL_GSET:
+       case ETHTOOL_GDRVINFO:
+       case ETHTOOL_GREGS:
+       case ETHTOOL_GWOL:
+       case ETHTOOL_GMSGLVL:
+       case ETHTOOL_GLINK:
+       case ETHTOOL_GCOALESCE:
+       case ETHTOOL_GRINGPARAM:
+       case ETHTOOL_GPAUSEPARAM:
+       case ETHTOOL_GRXCSUM:
+       case ETHTOOL_GTXCSUM:
+       case ETHTOOL_GSG:
+       case ETHTOOL_GSTRINGS:
+       case ETHTOOL_PHYS_ID:

PHYS_ID pokes in hardware and makes it jump through hoops, in the case of some hardware 
this could create a local DOS attack (e1000 suffers fromt his, probably more if not all) 
where the NIC might stop receiving packets, or the big lock is help indefinately.

Not a good idea

The other ones are fine I think.

Auke


+       case ETHTOOL_GSTATS:
+       case ETHTOOL_GTSO:
+       case ETHTOOL_GPERMADDR:
+       case ETHTOOL_GUFO:
+       case ETHTOOL_GGSO:
+               break;
+       default:
+               if (!capable(CAP_NET_ADMIN))
+                       return -EPERM;
+       }
+
        if(dev->ethtool_ops->begin)
                if ((rc = dev->ethtool_ops->begin(dev)) < 0)
                        return rc;
@@ -947,6 +967,10 @@ int dev_ethtool(struct ifreq *ifr)
        return rc;
 
  ioctl:
+       /* Keep existing behaviour for the moment.       */
+       if (!capable(CAP_NET_ADMIN))
+               return -EPERM;
+
        if (dev->do_ioctl)
                return dev->do_ioctl(dev, ifr, SIOCETHTOOL);
        return -EOPNOTSUPP;
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html