netdev - Re: Regression in bonding driver

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:	Wed, 4 Nov 2015 18:45:43 +1100
From:	Toby Corkindale <tjc@...trmute.net>
To:	netdev@...r.kernel.org
Subject: Re: Regression in bonding driver - devices without set_mac

On 4 November 2015 at 14:49, Toby Corkindale <tjc@...trmute.net> wrote:
> Hi,
> I've subscribed to this list to try and sort out a regression in the
> bonding network driver.
> I originally reported this in Dec 2014, as:
> https://bugzilla.kernel.org/show_bug.cgi?id=89161
>
> The bug is still present in the 4.2 Linux kernel.
>
> If you look at the code, here:
> https://github.com/torvalds/linux/commit/f54424412b6b2f64cae4d7c39d981ca14ce0052c
>
> Then you see that the intention is for devices without set_mac support
> to be supported. Although in older code they DID work, and that
> ability died sometime near this commit. And has never returned since.
>
> The code path in current kernels does allow devices through that block
> of code, but it fails somewhere else -- the devices are not
> successfully added as slaves, but the only error printed is the
> warning about not supporting MAC address setting.
>
> Is there anything I can do to try and sort this regression out, with you?
>
> I can explain how to set up two virtual machines with PPPoE client and
> server in order to test, if you like? Or I can try out patches and
> give feedback.
>
> Cheers
> Toby


The following patch fixes the regression, for me, although I suspect
it's not suitable for direct application.
But maybe knowing that it fixes the issue, helps you create something
appropriate?

diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 771a449..3f44875 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1460,8 +1460,8 @@ int bond_enslave(struct net_device *bond_dev,
struct net_device *slave_dev
)
       addr.sa_family = slave_dev->type;
       res = dev_set_mac_address(slave_dev, &addr);
       if (res) {
-           netdev_dbg(bond_dev, "Error %d calling set_mac_address\n", res);
-           goto err_restore_mtu;
+           netdev_warn(bond_dev, "Error %d calling set_mac_address\n", res);
       }
   }

diff --git a/drivers/net/bonding/bond_netlink.c
b/drivers/net/bonding/bond_netlink.c
index db760e8..e4ffc94 100644
--- a/drivers/net/bonding/bond_netlink.c
+++ b/drivers/net/bonding/bond_netlink.c
@@ -49,9 +49,8 @@ static int bond_fill_slave_info(struct sk_buff *skb,
           slave->link_failure_count))
       goto nla_put_failure;

-   if (nla_put(skb, IFLA_BOND_SLAVE_PERM_HWADDR,
-           slave_dev->addr_len, slave->perm_hwaddr))
-       goto nla_put_failure;
+   nla_put(skb, IFLA_BOND_SLAVE_PERM_HWADDR, slave_dev->addr_len,
slave->perm_hwaddr);

   if (nla_put_u16(skb, IFLA_BOND_SLAVE_QUEUE_ID, slave->queue_id))
       goto nla_put_failure;
--
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