Commit 471bdd1c authored by Eric Dumazet's avatar Eric Dumazet Committed by Greg Kroah-Hartman

ipv6: prevent possible fib6 leaks

[ Upstream commit 61fb0d01 ]

At ipv6 route dismantle, fib6_drop_pcpu_from() is responsible
for finding all percpu routes and set their ->from pointer
to NULL, so that fib6_ref can reach its expected value (1).

The problem right now is that other cpus can still catch the
route being deleted, since there is no rcu grace period
between the route deletion and call to fib6_drop_pcpu_from()

This can leak the fib6 and associated resources, since no
notifier will take care of removing the last reference(s).

I decided to add another boolean (fib6_destroying) instead
of reusing/renaming exception_bucket_flushed to ease stable backports,
and properly document the memory barriers used to implement this fix.

This patch has been co-developped with Wei Wang.

Fixes: 93531c67 ("net/ipv6: separate handling of FIB entries from dst based routes")
Signed-off-by: default avatarEric Dumazet <edumazet@google.com>
Reported-by: default avatarsyzbot <syzkaller@googlegroups.com>
Cc: Wei Wang <weiwan@google.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Martin Lau <kafai@fb.com>
Acked-by: default avatarWei Wang <weiwan@google.com>
Acked-by: default avatarMartin KaFai Lau <kafai@fb.com>
Reviewed-by: default avatarDavid Ahern <dsahern@gmail.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent 9cbc590d
...@@ -171,7 +171,8 @@ struct fib6_info { ...@@ -171,7 +171,8 @@ struct fib6_info {
dst_nocount:1, dst_nocount:1,
dst_nopolicy:1, dst_nopolicy:1,
dst_host:1, dst_host:1,
unused:3; fib6_destroying:1,
unused:2;
struct fib6_nh fib6_nh; struct fib6_nh fib6_nh;
struct rcu_head rcu; struct rcu_head rcu;
......
...@@ -909,6 +909,12 @@ static void fib6_drop_pcpu_from(struct fib6_info *f6i, ...@@ -909,6 +909,12 @@ static void fib6_drop_pcpu_from(struct fib6_info *f6i,
{ {
int cpu; int cpu;
/* Make sure rt6_make_pcpu_route() wont add other percpu routes
* while we are cleaning them here.
*/
f6i->fib6_destroying = 1;
mb(); /* paired with the cmpxchg() in rt6_make_pcpu_route() */
/* release the reference to this fib entry from /* release the reference to this fib entry from
* all of its cached pcpu routes * all of its cached pcpu routes
*/ */
...@@ -932,6 +938,9 @@ static void fib6_purge_rt(struct fib6_info *rt, struct fib6_node *fn, ...@@ -932,6 +938,9 @@ static void fib6_purge_rt(struct fib6_info *rt, struct fib6_node *fn,
{ {
struct fib6_table *table = rt->fib6_table; struct fib6_table *table = rt->fib6_table;
if (rt->rt6i_pcpu)
fib6_drop_pcpu_from(rt, table);
if (atomic_read(&rt->fib6_ref) != 1) { if (atomic_read(&rt->fib6_ref) != 1) {
/* This route is used as dummy address holder in some split /* This route is used as dummy address holder in some split
* nodes. It is not leaked, but it still holds other resources, * nodes. It is not leaked, but it still holds other resources,
...@@ -953,9 +962,6 @@ static void fib6_purge_rt(struct fib6_info *rt, struct fib6_node *fn, ...@@ -953,9 +962,6 @@ static void fib6_purge_rt(struct fib6_info *rt, struct fib6_node *fn,
fn = rcu_dereference_protected(fn->parent, fn = rcu_dereference_protected(fn->parent,
lockdep_is_held(&table->tb6_lock)); lockdep_is_held(&table->tb6_lock));
} }
if (rt->rt6i_pcpu)
fib6_drop_pcpu_from(rt, table);
} }
} }
......
...@@ -1260,6 +1260,13 @@ static struct rt6_info *rt6_make_pcpu_route(struct net *net, ...@@ -1260,6 +1260,13 @@ static struct rt6_info *rt6_make_pcpu_route(struct net *net,
prev = cmpxchg(p, NULL, pcpu_rt); prev = cmpxchg(p, NULL, pcpu_rt);
BUG_ON(prev); BUG_ON(prev);
if (rt->fib6_destroying) {
struct fib6_info *from;
from = xchg((__force struct fib6_info **)&pcpu_rt->from, NULL);
fib6_info_release(from);
}
return pcpu_rt; return pcpu_rt;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment