All of lore.kernel.org
 help / color / mirror / Atom feed
* FAILED: patch "[PATCH] ipv6: prevent possible fib6 leaks" failed to apply to 4.14-stable tree
@ 2019-05-22 16:51 gregkh
  2019-05-22 16:52 ` David Ahern
  0 siblings, 1 reply; 5+ messages in thread
From: gregkh @ 2019-05-22 16:51 UTC (permalink / raw)
  To: edumazet, davem, dsahern, kafai, syzkaller, weiwan; +Cc: stable


The patch below does not apply to the 4.14-stable tree.
If someone wants it applied there, or to any other stable or longterm
tree, then please email the backport, including the original git commit
id to <stable@vger.kernel.org>.

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

From 61fb0d01680771f72cc9d39783fb2c122aaad51e Mon Sep 17 00:00:00 2001
From: Eric Dumazet <edumazet@google.com>
Date: Wed, 15 May 2019 19:39:52 -0700
Subject: [PATCH] ipv6: prevent possible fib6 leaks

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: 93531c674315 ("net/ipv6: separate handling of FIB entries from dst based routes")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: syzbot <syzkaller@googlegroups.com>
Cc: Wei Wang <weiwan@google.com>
Cc: David Ahern <dsahern@gmail.com>
Cc: Martin Lau <kafai@fb.com>
Acked-by: Wei Wang <weiwan@google.com>
Acked-by: Martin KaFai Lau <kafai@fb.com>
Reviewed-by: David Ahern <dsahern@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/include/net/ip6_fib.h b/include/net/ip6_fib.h
index 40105738e2f6..525f701653ca 100644
--- a/include/net/ip6_fib.h
+++ b/include/net/ip6_fib.h
@@ -167,7 +167,8 @@ struct fib6_info {
 					dst_nocount:1,
 					dst_nopolicy:1,
 					dst_host:1,
-					unused:3;
+					fib6_destroying:1,
+					unused:2;
 
 	struct fib6_nh			fib6_nh;
 	struct rcu_head			rcu;
diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
index 08e0390e001c..008421b550c6 100644
--- a/net/ipv6/ip6_fib.c
+++ b/net/ipv6/ip6_fib.c
@@ -904,6 +904,12 @@ static void fib6_drop_pcpu_from(struct fib6_info *f6i,
 {
 	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
 	 * all of its cached pcpu routes
 	 */
@@ -927,6 +933,9 @@ static void fib6_purge_rt(struct fib6_info *rt, struct fib6_node *fn,
 {
 	struct fib6_table *table = rt->fib6_table;
 
+	if (rt->rt6i_pcpu)
+		fib6_drop_pcpu_from(rt, table);
+
 	if (refcount_read(&rt->fib6_ref) != 1) {
 		/* This route is used as dummy address holder in some split
 		 * nodes. It is not leaked, but it still holds other resources,
@@ -948,9 +957,6 @@ static void fib6_purge_rt(struct fib6_info *rt, struct fib6_node *fn,
 			fn = rcu_dereference_protected(fn->parent,
 				    lockdep_is_held(&table->tb6_lock));
 		}
-
-		if (rt->rt6i_pcpu)
-			fib6_drop_pcpu_from(rt, table);
 	}
 }
 
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 23a20d62daac..27c0cc5d9d30 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -1295,6 +1295,13 @@ static struct rt6_info *rt6_make_pcpu_route(struct net *net,
 	prev = cmpxchg(p, NULL, pcpu_rt);
 	BUG_ON(prev);
 
+	if (res->f6i->fib6_destroying) {
+		struct fib6_info *from;
+
+		from = xchg((__force struct fib6_info **)&pcpu_rt->from, NULL);
+		fib6_info_release(from);
+	}
+
 	return pcpu_rt;
 }
 


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: FAILED: patch "[PATCH] ipv6: prevent possible fib6 leaks" failed to apply to 4.14-stable tree
  2019-05-22 16:51 FAILED: patch "[PATCH] ipv6: prevent possible fib6 leaks" failed to apply to 4.14-stable tree gregkh
@ 2019-05-22 16:52 ` David Ahern
  2019-05-22 17:29   ` Greg KH
  0 siblings, 1 reply; 5+ messages in thread
From: David Ahern @ 2019-05-22 16:52 UTC (permalink / raw)
  To: gregkh, edumazet, davem, kafai, syzkaller, weiwan; +Cc: stable

On 5/22/19 10:51 AM, gregkh@linuxfoundation.org wrote:
> 
> The patch below does not apply to the 4.14-stable tree.
> If someone wants it applied there, or to any other stable or longterm
> tree, then please email the backport, including the original git commit
> id to <stable@vger.kernel.org>.
> 
> thanks,
> 
> greg k-h
> 

As I recall it only needs to go back to 4.17.


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: FAILED: patch "[PATCH] ipv6: prevent possible fib6 leaks" failed to apply to 4.14-stable tree
  2019-05-22 16:52 ` David Ahern
@ 2019-05-22 17:29   ` Greg KH
  2019-05-22 17:36     ` David Ahern
  0 siblings, 1 reply; 5+ messages in thread
From: Greg KH @ 2019-05-22 17:29 UTC (permalink / raw)
  To: David Ahern; +Cc: edumazet, davem, kafai, syzkaller, weiwan, stable

On Wed, May 22, 2019 at 10:52:57AM -0600, David Ahern wrote:
> On 5/22/19 10:51 AM, gregkh@linuxfoundation.org wrote:
> > 
> > The patch below does not apply to the 4.14-stable tree.
> > If someone wants it applied there, or to any other stable or longterm
> > tree, then please email the backport, including the original git commit
> > id to <stable@vger.kernel.org>.
> > 
> > thanks,
> > 
> > greg k-h
> > 
> 
> As I recall it only needs to go back to 4.17.

Thanks, but someone backported the commit mentioned in the Fixes line:
	Fixes: 93531c674315 ("net/ipv6: separate handling of FIB entries from dst based routes")

to 4.14.y.  If it's really not relevant there, not a big deal, now
dropped.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: FAILED: patch "[PATCH] ipv6: prevent possible fib6 leaks" failed to apply to 4.14-stable tree
  2019-05-22 17:29   ` Greg KH
@ 2019-05-22 17:36     ` David Ahern
  2019-05-23  5:42       ` Greg KH
  0 siblings, 1 reply; 5+ messages in thread
From: David Ahern @ 2019-05-22 17:36 UTC (permalink / raw)
  To: Greg KH; +Cc: edumazet, davem, kafai, syzkaller, weiwan, stable

On 5/22/19 11:29 AM, Greg KH wrote:
> Thanks, but someone backported the commit mentioned in the Fixes line:
> 	Fixes: 93531c674315 ("net/ipv6: separate handling of FIB entries from dst based routes")
> 
> to 4.14.y.  If it's really not relevant there, not a big deal, now
> dropped.


Just checked and that commit is not in 4.14 line.

It is one of like 90 other patches that actually depend on other changes
in 4.15, 4.16. I can not imagine some poor soul backporting all of that
to 4.14.

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: FAILED: patch "[PATCH] ipv6: prevent possible fib6 leaks" failed to apply to 4.14-stable tree
  2019-05-22 17:36     ` David Ahern
@ 2019-05-23  5:42       ` Greg KH
  0 siblings, 0 replies; 5+ messages in thread
From: Greg KH @ 2019-05-23  5:42 UTC (permalink / raw)
  To: David Ahern; +Cc: edumazet, davem, kafai, syzkaller, weiwan, stable

On Wed, May 22, 2019 at 11:36:45AM -0600, David Ahern wrote:
> On 5/22/19 11:29 AM, Greg KH wrote:
> > Thanks, but someone backported the commit mentioned in the Fixes line:
> > 	Fixes: 93531c674315 ("net/ipv6: separate handling of FIB entries from dst based routes")
> > 
> > to 4.14.y.  If it's really not relevant there, not a big deal, now
> > dropped.
> 
> 
> Just checked and that commit is not in 4.14 line.
> 
> It is one of like 90 other patches that actually depend on other changes
> in 4.15, 4.16. I can not imagine some poor soul backporting all of that
> to 4.14.

Argh, my scripts caught that the string "93531c674315" was included in
the changelog for 4.14.72, but the context of that was:
	- fib6_info_release was introduced upstream in 93531c674315
	  ("net/ipv6: separate handling of FIB entries from dst based
	  routes"), but is not present in stable kernels; 4.14.y relies
	  on dst_release/ ip6_rt_put/dst_release_immediate.

So this was my mistake, thanks for pointing it out.

greg k-h

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2019-05-23  5:42 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-22 16:51 FAILED: patch "[PATCH] ipv6: prevent possible fib6 leaks" failed to apply to 4.14-stable tree gregkh
2019-05-22 16:52 ` David Ahern
2019-05-22 17:29   ` Greg KH
2019-05-22 17:36     ` David Ahern
2019-05-23  5:42       ` Greg KH

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.