Message ID | 1408259994-31458-1-git-send-email-sebastien.barre@uclouvain.be |
---|---|
State | Accepted, archived |
Delegated to: | David Miller |
Headers | show |
From: Sébastien Barré <sebastien.barre@uclouvain.be> Date: Sun, 17 Aug 2014 09:19:54 +0200 > Commit 7a9bc9b81a5b ("ipv4: Elide fib_validate_source() completely when possible.") > introduced a short-circuit to avoid calling fib_validate_source when not > needed. That change took rp_filter into account, but not accept_local. > This resulted in a change of behaviour: with rp_filter and accept_local > off, incoming packets with a local address in the source field should be > dropped. > > Here is how to reproduce the change pre/post 7a9bc9b81a5b commit: > -configure the same IPv4 address on hosts A and B. > -try to send an ARP request from B to A. > -The ARP request will be dropped before that commit, but accepted and answered > after that commit. > > This adds a check for ACCEPT_LOCAL, to maintain full > fib validation in case it is 0. We also leave __fib_validate_source() earlier > when possible, based on the same check as fib_validate_source(), once the > accept_local stuff is verified. > > Cc: Gregory Detal <gregory.detal@uclouvain.be> > Cc: Christoph Paasch <christoph.paasch@uclouvain.be> > Cc: Hannes Frederic Sowa <hannes@redhat.com> > Cc: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> > Signed-off-by: Sébastien Barré <sebastien.barre@uclouvain.be> Applied. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hello, On Sun, 17 Aug 2014, Sébastien Barré wrote: > Commit 7a9bc9b81a5b ("ipv4: Elide fib_validate_source() completely when possible.") > introduced a short-circuit to avoid calling fib_validate_source when not > needed. That change took rp_filter into account, but not accept_local. > This resulted in a change of behaviour: with rp_filter and accept_local > off, incoming packets with a local address in the source field should be > dropped. > > Here is how to reproduce the change pre/post 7a9bc9b81a5b commit: > -configure the same IPv4 address on hosts A and B. > -try to send an ARP request from B to A. > -The ARP request will be dropped before that commit, but accepted and answered > after that commit. > > This adds a check for ACCEPT_LOCAL, to maintain full > fib validation in case it is 0. We also leave __fib_validate_source() earlier > when possible, based on the same check as fib_validate_source(), once the > accept_local stuff is verified. I remember there was related change for docs, may be commit c801e3cc1925e0 should be reverted ? > Cc: Gregory Detal <gregory.detal@uclouvain.be> > Cc: Christoph Paasch <christoph.paasch@uclouvain.be> > Cc: Hannes Frederic Sowa <hannes@redhat.com> > Cc: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> > Signed-off-by: Sébastien Barré <sebastien.barre@uclouvain.be> Regards -- Julian Anastasov <ja@ssi.bg>
diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c index 255aa99..23104a3 100644 --- a/net/ipv4/fib_frontend.c +++ b/net/ipv4/fib_frontend.c @@ -243,7 +243,7 @@ static int __fib_validate_source(struct sk_buff *skb, __be32 src, __be32 dst, u8 tos, int oif, struct net_device *dev, int rpf, struct in_device *idev, u32 *itag) { - int ret, no_addr, accept_local; + int ret, no_addr; struct fib_result res; struct flowi4 fl4; struct net *net; @@ -258,16 +258,17 @@ static int __fib_validate_source(struct sk_buff *skb, __be32 src, __be32 dst, no_addr = idev->ifa_list == NULL; - accept_local = IN_DEV_ACCEPT_LOCAL(idev); fl4.flowi4_mark = IN_DEV_SRC_VMARK(idev) ? skb->mark : 0; net = dev_net(dev); if (fib_lookup(net, &fl4, &res)) goto last_resort; - if (res.type != RTN_UNICAST) { - if (res.type != RTN_LOCAL || !accept_local) - goto e_inval; - } + if (res.type != RTN_UNICAST && + (res.type != RTN_LOCAL || !IN_DEV_ACCEPT_LOCAL(idev))) + goto e_inval; + if (!rpf && !fib_num_tclassid_users(dev_net(dev)) && + (dev->ifindex != oif || !IN_DEV_TX_REDIRECTS(idev))) + goto last_resort; fib_combine_itag(itag, &res); dev_match = false; @@ -321,6 +322,7 @@ int fib_validate_source(struct sk_buff *skb, __be32 src, __be32 dst, int r = secpath_exists(skb) ? 0 : IN_DEV_RPFILTER(idev); if (!r && !fib_num_tclassid_users(dev_net(dev)) && + IN_DEV_ACCEPT_LOCAL(idev) && (dev->ifindex != oif || !IN_DEV_TX_REDIRECTS(idev))) { *itag = 0; return 0;
Commit 7a9bc9b81a5b ("ipv4: Elide fib_validate_source() completely when possible.") introduced a short-circuit to avoid calling fib_validate_source when not needed. That change took rp_filter into account, but not accept_local. This resulted in a change of behaviour: with rp_filter and accept_local off, incoming packets with a local address in the source field should be dropped. Here is how to reproduce the change pre/post 7a9bc9b81a5b commit: -configure the same IPv4 address on hosts A and B. -try to send an ARP request from B to A. -The ARP request will be dropped before that commit, but accepted and answered after that commit. This adds a check for ACCEPT_LOCAL, to maintain full fib validation in case it is 0. We also leave __fib_validate_source() earlier when possible, based on the same check as fib_validate_source(), once the accept_local stuff is verified. Cc: Gregory Detal <gregory.detal@uclouvain.be> Cc: Christoph Paasch <christoph.paasch@uclouvain.be> Cc: Hannes Frederic Sowa <hannes@redhat.com> Cc: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com> Signed-off-by: Sébastien Barré <sebastien.barre@uclouvain.be> --- Changes : -Applied Sergei's comments (update of the patch description) -Applied Hannes' comments: leave __fib_validate_source() after the RTN_UNICAST check when possible. I also removed the accept_local variable, as it is used only once, and changed the nested if into its single-if equivalent, for readability. net/ipv4/fib_frontend.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-)