Thread (9 messages) 9 messages, 4 authors, 2019-09-26

Re: [PATCH v2] ipv6: do not free rt if FIB_LOOKUP_NOREF is set on suppress rule

From: Wei Wang <hidden>
Date: 2019-09-24 17:02:57
Also in: lkml, netdev

On Tue, Sep 24, 2019 at 7:01 AM Jason A. Donenfeld [off-list ref] wrote:
Commit 7d9e5f422150 removed references from certain dsts, but accounting
for this never translated down into the fib6 suppression code. This bug
was triggered by WireGuard users who use wg-quick(8), which uses the
"suppress-prefix" directive to ip-rule(8) for routing all of their
internet traffic without routing loops. The test case added here
causes the reference underflow by causing packets to evaluate a suppress
rule.

Cc: stable@vger.kernel.org
Fixes: 7d9e5f422150 ("ipv6: convert major tx path to use RT6_LOOKUP_F_DST_NOREF")
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
Acked-by: Wei Wang <redacted>

Good catch. Thanks for the fix.
quoted hunk ↗ jump to hunk
 net/ipv6/fib6_rules.c                    |  3 ++-
 tools/testing/selftests/net/fib_tests.sh | 17 ++++++++++++++++-
 2 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/net/ipv6/fib6_rules.c b/net/ipv6/fib6_rules.c
index d22b6c140f23..f9e8fe3ff0c5 100644
--- a/net/ipv6/fib6_rules.c
+++ b/net/ipv6/fib6_rules.c
@@ -287,7 +287,8 @@ static bool fib6_rule_suppress(struct fib_rule *rule, struct fib_lookup_arg *arg
        return false;

 suppress_route:
-       ip6_rt_put(rt);
+       if (!(arg->flags & FIB_LOOKUP_NOREF))
+               ip6_rt_put(rt);
        return true;
 }
diff --git a/tools/testing/selftests/net/fib_tests.sh b/tools/testing/selftests/net/fib_tests.sh
index 4465fc2dae14..c2c5f2bf0f95 100755
--- a/tools/testing/selftests/net/fib_tests.sh
+++ b/tools/testing/selftests/net/fib_tests.sh
@@ -9,7 +9,7 @@ ret=0
 ksft_skip=4

 # all tests in this script. Can be overridden with -t option
-TESTS="unregister down carrier nexthop ipv6_rt ipv4_rt ipv6_addr_metric ipv4_addr_metric ipv6_route_metrics ipv4_route_metrics ipv4_route_v6_gw rp_filter"
+TESTS="unregister down carrier nexthop suppress ipv6_rt ipv4_rt ipv6_addr_metric ipv4_addr_metric ipv6_route_metrics ipv4_route_metrics ipv4_route_v6_gw rp_filter"

 VERBOSE=0
 PAUSE_ON_FAIL=no
@@ -614,6 +614,20 @@ fib_nexthop_test()
        cleanup
 }

+fib_suppress_test()
+{
+       $IP link add dummy1 type dummy
+       $IP link set dummy1 up
+       $IP -6 route add default dev dummy1
+       $IP -6 rule add table main suppress_prefixlength 0
+       ping -f -c 1000 -W 1 1234::1 || true
+       $IP -6 rule del table main suppress_prefixlength 0
+       $IP link del dummy1
+
+       # If we got here without crashing, we're good.
+       return 0
+}
+
 ################################################################################
 # Tests on route add and replace
@@ -1591,6 +1605,7 @@ do
        fib_carrier_test|carrier)       fib_carrier_test;;
        fib_rp_filter_test|rp_filter)   fib_rp_filter_test;;
        fib_nexthop_test|nexthop)       fib_nexthop_test;;
+       fib_suppress_test|suppress)     fib_suppress_test;;
        ipv6_route_test|ipv6_rt)        ipv6_route_test;;
        ipv4_route_test|ipv4_rt)        ipv4_route_test;;
        ipv6_addr_metric)               ipv6_addr_metric_test;;
--
2.21.0
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help