netdev - Re: [PATCH net] bonding: fix arp requests sends with isolated routes

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]

Message-ID: <20140217093618.GA13038@redhat.com>
Date:	Mon, 17 Feb 2014 10:36:18 +0100
From:	Veaceslav Falico <vfalico@...hat.com>
To:	François Cachereul <f.cachereul@...halink.fr>
Cc:	Jay Vosburgh <fubar@...ibm.com>,
	Andy Gospodarek <andy@...yhouse.net>, netdev@...r.kernel.org
Subject: Re: [PATCH net] bonding: fix arp requests sends with isolated routes

On Fri, Feb 14, 2014 at 04:59:23PM +0100, François Cachereul wrote:
>Make arp_send_all() try to send arp packets through slave devices event
>if no route to arp_ip_target is found. This is useful when the route
>is in an isolated routing table with routing rule parameters like oif or
>iif in which case ip_route_output() return an error.
>Thus, the arp packet is send without vlan and with the bond ip address
>as sender.

I'm not sure I understand it completely, specifically I don't really
understand the term "isolated routing table". Do you mean that it's an
routing table different from local/main, which is enabled by
CONFIG_IP_MULTIPLE_TABLES=y ? I think they should be all 'catched' by
ip_route_output(), or am I missing something?

Anyway, with this fix bonding will send packets even if it doesn't find
route AND src addr (bond_confirm_addr() can return 0 if bond doesn't have
the required ip assigned), which isn't good at all.

If my assumption about the routing tables is correct and ip_route_output()
doesn't find addition tables, we should at least try to fix it via scanning
those tables.

Sorry if I didn't understand you correctly...

>
>Signed-off-by: François CACHEREUL <f.cachereul@...halink.fr>
>---
>This previously worked, the problem was added in 2.6.35 with vlan 0
>added by default when the module 8021q is loaded. Before that no route
>lookup was done if the bond device did not have any vlan. The problem
>now exists event if the module 8021q is not loaded.
>
> drivers/net/bonding/bond_main.c |    9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
>diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>index 8676649..300e5b8 100644
>--- a/drivers/net/bonding/bond_main.c
>+++ b/drivers/net/bonding/bond_main.c
>@@ -2168,17 +2168,19 @@ static void bond_arp_send_all(struct bonding *bond, struct slave *slave)
> 	for (i = 0; i < BOND_MAX_ARP_TARGETS && targets[i]; i++) {
> 		pr_debug("basa: target %pI4\n", &targets[i]);
>
>+		vlan_id = 0;
>+
> 		/* Find out through which dev should the packet go */
> 		rt = ip_route_output(dev_net(bond->dev), targets[i], 0,
> 				     RTO_ONLINK, 0);
> 		if (IS_ERR(rt)) {
> 			pr_debug("%s: no route to arp_ip_target %pI4\n",
> 				 bond->dev->name, &targets[i]);
>-			continue;
>+			/* no route found, trying with bond->dev */
>+			addr = bond_confirm_addr(bond->dev, targets[i], 0);
>+			goto rt_err_try;
> 		}
>
>-		vlan_id = 0;
>-
> 		/* bond device itself */
> 		if (rt->dst.dev == bond->dev)
> 			goto found;
>@@ -2232,6 +2234,7 @@ static void bond_arp_send_all(struct bonding *bond, struct slave *slave)
> found:
> 		addr = bond_confirm_addr(rt->dst.dev, targets[i], 0);
> 		ip_rt_put(rt);
>+rt_err_try:
> 		bond_arp_send(slave->dev, ARPOP_REQUEST, targets[i],
> 			      addr, vlan_id);
> 	}
>-- 
>1.7.10.4
>
--
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