netdev - [NET/USB] ax88179_set_mac_addr: return only error or zero

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

Message-ID: <CALgSy_dy8=WwTJ-GAkGY-DV=NeOOKEP7nMNO=0XyaP2wGwbkdA@mail.gmail.com>
Date:	Wed, 15 Oct 2014 09:49:19 -0400
From:	Ian Morgan <imorgan@...mordial.ca>
To:	netdev@...r.kernel.org, linux-usb@...r.kernel.org
Subject: [NET/USB] ax88179_set_mac_addr: return only error or zero

(It's about a USB NIC, so not sure if this is suitable to the netdev
mailing list or the linux-usb mailing list, so you both get it.)

I had an issue with bonding one of these USB3 Gigabit NIC devices
because set_mac_addr was returning a positive value (6), and the
bonding driver thinks that any non-zero value is an error.

My proposed fix was to (I thought correctly) check for negative return
codes in bond_enslave() instead of any non-zero value.

I was told by the bonding driver maintainers that my proposed fix was
incorrect, and that the device driver should return only zero on
success, as other drivers do this and that assumption has been made.
I'm really not sure why the more vigilant check in the bonding driver
was "wrong" though. We all know the rule about assumptions ;-) But I
defer to their superior knowledge of kernel interfaces.

Anyhow, it would seem that the following patch is the "more
acceptable" solution, and is in line with what other drivers do:

--- linux-3.17/drivers/net/usb/ax88179_178a.c.orig  2014-10-05
15:23:04.000000000 -0400
+++ linux-3.17/drivers/net/usb/ax88179_178a.c   2014-10-15
09:07:31.217810217 -0400
@@ -937,6 +937,7 @@ static int ax88179_set_mac_addr(struct n
 {
        struct usbnet *dev = netdev_priv(net);
        struct sockaddr *addr = p;
+       int ret;

        if (netif_running(net))
                return -EBUSY;
@@ -946,8 +947,12 @@ static int ax88179_set_mac_addr(struct n
        memcpy(net->dev_addr, addr->sa_data, ETH_ALEN);

        /* Set the MAC address */
-       return ax88179_write_cmd(dev, AX_ACCESS_MAC, AX_NODE_ID, ETH_ALEN,
+       ret = ax88179_write_cmd(dev, AX_ACCESS_MAC, AX_NODE_ID, ETH_ALEN,
                                 ETH_ALEN, net->dev_addr);
+       if (ret < 0)
+               return ret;
+
+       return 0;
 }

 static const struct net_device_ops ax88179_netdev_ops = {

(non-tab-mangled patch file attached)

Your feedback is duly appreciated. Please CC me, as I am not
subscribed to these mailing lists.

--
Ian Morgan

View attachment "ax88719_setmacaddr_rc.patch" of type "text/x-patch" (775 bytes)