| Message ID | 20190924073615.31704-1-Jason@zx2c4.com |
|---|---|
| State | New |
| Headers | show |
| Series | ipv6: do not free rt if FIB_LOOKUP_NOREF is set on suppress rule | expand |
On Tue, Sep 24, 2019 at 2:53 PM David Miller <davem@davemloft.net> wrote: > Please make such test cases integratabe into the selftests area for networking > and submit it along with this fix. That link is for a WireGuard test-case. When we get that upstream, those will all live in selftests/ all the same as you'd like. For now, it's running for every kernel on https://build.wireguard.com/ which in turn runs for every new commit.
From: "Jason A. Donenfeld" <Jason@zx2c4.com> Date: Tue, 24 Sep 2019 14:55:04 +0200 > On Tue, Sep 24, 2019 at 2:53 PM David Miller <davem@davemloft.net> wrote: >> Please make such test cases integratabe into the selftests area for networking >> and submit it along with this fix. > > That link is for a WireGuard test-case. When we get that upstream, > those will all live in selftests/ all the same as you'd like. For now, > it's running for every kernel on https://build.wireguard.com/ which in > turn runs for every new commit. I'm asking you to make a non-wireguard test that triggers the problem. Or would you like a situation you're interested in to break from time to time. Jason, please don't be difficult about this and write a proper test case just like I would ask anyone else fixing bugs like this to write. Thank you.
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; }
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 in the link of this commit reliably triggers various crashes due to the use-after-free caused by the reference underflow. Cc: stable@vger.kernel.org Fixes: 7d9e5f422150 ("ipv6: convert major tx path to use RT6_LOOKUP_F_DST_NOREF") Test-case: https://git.zx2c4.com/WireGuard/commit/?id=ad66532000f7a20b149e47c5eb3a957362c8e161 Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> --- net/ipv6/fib6_rules.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) -- 2.21.0