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

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

From: "Jason A. Donenfeld" <Jason@zx2c4.com>
Date: 2019-09-24 09:36:11
Also in: lkml

Hey Wei,

I meant to CC you but slipped up. Sorry about that. Take a look at
this thread if you have a chance.

Thanks,
Jason

On Tue, Sep 24, 2019 at 10:03 AM Jason A. Donenfeld [off-list ref] wrote:
quoted hunk ↗ jump to hunk
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(-)
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;
 }

--
2.21.0
On Tue, Sep 24, 2019 at 10:03 AM Jason A. Donenfeld [off-list ref] wrote:
quoted hunk ↗ jump to hunk
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(-)
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;
 }

--
2.21.0


-- 
Jason A. Donenfeld
Deep Space Explorer
fr: +33 6 51 90 82 66
us: +1 513 476 1200
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help