netdev - Re: [PATCH] bridge: Remove BR_PROXYARP flooding check code

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: <20141210113938.GA12093@jouni.qca.qualcomm.com>
Date:	Wed, 10 Dec 2014 13:39:38 +0200
From:	Jouni Malinen <jouni@...eaurora.org>
To:	Stephen Hemminger <stephen@...workplumber.org>
Cc:	David Miller <davem@...emloft.net>, netdev@...r.kernel.org,
	Kyeyoon Park <kyeyoonp@...eaurora.org>
Subject: Re: [PATCH] bridge: Remove BR_PROXYARP flooding check code

On Tue, Dec 09, 2014 at 02:21:58PM -0800, Stephen Hemminger wrote:
> On Mon,  8 Dec 2014 17:27:40 +0200
> Jouni Malinen <jouni@...eaurora.org> wrote:
> > diff --git a/net/bridge/br_forward.c b/net/bridge/br_forward.c
> > @@ -185,10 +185,6 @@ static void br_flood(struct net_bridge *br, struct sk_buff *skb,
> >  		if (unicast && !(p->flags & BR_FLOOD))
> >  			continue;
> >  
> > -		/* Do not flood to ports that enable proxy ARP */
> > -		if (p->flags & BR_PROXYARP)
> > -			continue;
> > -
> >  		prev = maybe_deliver(prev, p, skb, __packet_hook);

> Aren't you at risk of duplicate ARP responses in some cases.
> You can't assume user will run netfilter.

This is only for the case where BR_PROXYARP has been enabled by the
user, but yes, it would be convenient to handle cases better without
requiring netfilter and skip flooding to BR_PROXYARP port more
selectively here. Would there be some convenient means for
br_do_proxy_arp() to mark the skb that it has replied to in br_input.c
and then use that here in br_forward.c to not flood an ARP request that
has already been replied to? Or should this simply skip flooding of all
ARP packets with something like following?

 		if (unicast && !(p->flags & BR_FLOOD))
 			continue;
 
-		/* Do not flood to ports that enable proxy ARP */
-		if (p->flags & BR_PROXYARP)
+		/* Do not flood ARP to ports that enable proxy ARP */
+		if (p->flags & BR_PROXYARP &&
+		    skb->protocol == htons(ETH_P_ARP))
 			continue;
 
 		prev = maybe_deliver(prev, p, skb, __packet_hook);

-- 
Jouni Malinen                                            PGP id EFC895FA
--
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