netdev - Re: [PATCH] ipv6: no addrconf for slave devices

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:	Fri, 16 Oct 2015 17:57:44 +0200
From:	Jan Blunck <jblunck@...radead.org>
To:	Jiri Pirko <jiri@...nulli.us>
Cc:	davem@...emloft.net, Alexey Kuznetsov <kuznet@....inr.ac.ru>,
	jmorris@...ei.org, yoshfuji@...ux-ipv6.org,
	Patrick McHardy <kaber@...sh.net>, netdev@...r.kernel.org,
	LKML <linux-kernel@...r.kernel.org>, fubar@...ibm.com
Subject: Re: [PATCH] ipv6: no addrconf for slave devices

On Fri, Oct 16, 2015 at 1:54 PM, Jiri Pirko <jiri@...nulli.us> wrote:
> Fri, Oct 16, 2015 at 12:21:51PM CEST, jblunck@...radead.org wrote:
>>If a device without the IFF_SLAVE flag set (e.g. team, bridge, openvswitch
>>vport, batman) is enslaved and IPv6 is active then addrconf will be
>>initiated and a link-local address is added to the slave interface.
>>
>>This patch alters the behavior so that addrconf will only run on the master
>>device itself. This is achieved by checking the device tree instead of
>>checking for a specific flag.
>>
>>Signed-off-by: Jan Blunck <jblunck@...radead.org>
>>---
>> net/ipv6/addrconf.c | 6 +++++-
>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>
>>diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
>>index 9001133..26d61f0 100644
>>--- a/net/ipv6/addrconf.c
>>+++ b/net/ipv6/addrconf.c
>>@@ -3141,8 +3141,12 @@ static int addrconf_notify(struct notifier_block *this, unsigned long event,
>>
>>       case NETDEV_UP:
>>       case NETDEV_CHANGE:
>>-              if (dev->flags & IFF_SLAVE)
>>+              /* If a master is set stop IPv6 on this interface */
>>+              if (netdev_master_upper_dev_get(dev)) {
>>+                      if (idev)
>>+                              addrconf_ifdown(dev, 1);
>
> This breaks teamd if it's using NS/NA ping link-watch on link-local addresses.
>
> What is the reason for this patch? Does it recolve any issue you are
> having?

I don't think that enslaved ports should get network layer addresses.
This is one example with a team device:

3: eth1: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast
master team0 state UP group default qlen 1000
    link/ether 52:54:00:ef:5f:a1 brd ff:ff:ff:ff:ff:ff
    inet6 fe80::5054:ff:feef:5fa1/64 scope link
       valid_lft forever preferred_lft forever
4: eth2: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc pfifo_fast
master team0 state UP group default qlen 1000
    link/ether 52:54:00:ef:5f:a1 brd ff:ff:ff:ff:ff:ff
    inet6 fe80::5054:ff:feef:5fa1/64 scope link
       valid_lft forever preferred_lft forever
6: team0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue
state UP group default
    link/ether 52:54:00:ef:5f:a1 brd ff:ff:ff:ff:ff:ff
    inet6 fe80::5054:ff:feef:5fa1/64 scope link
       valid_lft forever preferred_lft forever

All link-layer addresses are identical due to the fact that the link
aggregation group is syncing the MAC addresses. Having the IPv6
link-local address set in this case is pretty useless. The partner
device is unable to differentiate if the port is addressed or the team
device. Even if the addrconf started before the device was enslaved
(and therefore at least one port got a different IPv6 link-local
address than the link aggregation group) the partner device usually
learns the address for the aggregated link.

For LACP the standard states that one port should only bind to at most
one aggregator. The additional IPv6 link-local address allows the port
to be used by another stack besides the aggregator. Besides that, the
distribution of any user traffic (e.g. ICMPv6) is forbidden in LACP
before the partner aggregator signals being ready. So having the
link-local traffic on the wire is clearly a violation of that.

In other cases like openvswitch the link-local address is added to the
system but it is not usable since the bridge port stays in state
UNKNOWN.

Regards,
Jan
--
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