From patchwork Sat Oct 4 14:18:02 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Herbert Xu X-Patchwork-Id: 396521 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 539CB140139 for ; Sun, 5 Oct 2014 00:18:23 +1000 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751343AbaJDOST (ORCPT ); Sat, 4 Oct 2014 10:18:19 -0400 Received: from helcar.apana.org.au ([209.40.204.226]:39740 "EHLO helcar.apana.org.au" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750857AbaJDOSR (ORCPT ); Sat, 4 Oct 2014 10:18:17 -0400 Received: from gondolin.me.apana.org.au ([192.168.0.6]) by fornost.hengli.com.au with esmtp (Exim 4.80 #3 (Debian)) id 1XaQ9t-0004sO-Sb; Sun, 05 Oct 2014 00:18:06 +1000 Received: from herbert by gondolin.me.apana.org.au with local (Exim 4.80) (envelope-from ) id 1XaQ9q-0002qf-8d; Sat, 04 Oct 2014 22:18:02 +0800 Date: Sat, 4 Oct 2014 22:18:02 +0800 From: Herbert Xu To: Florian Westphal Cc: netfilter-devel@vger.kernel.org, bsd@redhat.com, stephen@networkplumber.org, netdev@vger.kernel.org, eric.dumazet@gmail.com, davidn@davidnewall.com, "David S. Miller" Subject: bridge: Do not compile options in br_parse_ip_options Message-ID: <20141004141802.GA10878@gondor.apana.org.au> References: <1412384670-17794-1-git-send-email-fw@strlen.de> <20141004035606.GA8228@gondor.apana.org.au> <20141004100413.GA1241@breakpoint.cc> <20141004135508.GA10705@gondor.apana.org.au> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20141004135508.GA10705@gondor.apana.org.au> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Sat, Oct 04, 2014 at 09:55:08PM +0800, Herbert Xu wrote: > > I'll try to create a patch that essentially reverts the patch > that led us here. Here is a patch that's only compile-tested: bridge: Do not compile options in br_parse_ip_options Commit 462fb2af9788a82a534f8184abfde31574e1cfa0 bridge : Sanitize skb before it enters the IP stack broke when IP options are actually used because it mangles the skb as if it entered the IP stack which is wrong because the bridge is supposed to operate below the IP stack. Since nobody has actually requested for parsing of IP options this patch fixes it by simply reverting to the previous approach of ignoring all IP options, i.e., zeroing the IPCB. If and when somebody who uses IP options and actually needs them to be parsed by the bridge complains then we can revisit this. Reported-by: David Newall Signed-off-by: Herbert Xu Cheers, Tested-by: Florian Westphal diff --git a/net/bridge/br_netfilter.c b/net/bridge/br_netfilter.c index a615264..c0fdb4d 100644 --- a/net/bridge/br_netfilter.c +++ b/net/bridge/br_netfilter.c @@ -260,7 +260,6 @@ static inline void nf_bridge_update_protocol(struct sk_buff *skb) static int br_parse_ip_options(struct sk_buff *skb) { - struct ip_options *opt; const struct iphdr *iph; struct net_device *dev = skb->dev; u32 len; @@ -269,7 +268,6 @@ static int br_parse_ip_options(struct sk_buff *skb) goto inhdr_error; iph = ip_hdr(skb); - opt = &(IPCB(skb)->opt); /* Basic sanity checks */ if (iph->ihl < 5 || iph->version != 4) @@ -295,23 +293,11 @@ static int br_parse_ip_options(struct sk_buff *skb) } memset(IPCB(skb), 0, sizeof(struct inet_skb_parm)); - if (iph->ihl == 5) - return 0; - - opt->optlen = iph->ihl*4 - sizeof(struct iphdr); - if (ip_options_compile(dev_net(dev), opt, skb)) - goto inhdr_error; - - /* Check correct handling of SRR option */ - if (unlikely(opt->srr)) { - struct in_device *in_dev = __in_dev_get_rcu(dev); - if (in_dev && !IN_DEV_SOURCE_ROUTE(in_dev)) - goto drop; - - if (ip_options_rcv_srr(skb)) - goto drop; - } - + /* We should really parse IP options here but until + * somebody who actually uses IP options complains to + * us we'll just silently ignore the options because + * we're lazy! + */ return 0; inhdr_error: