Re: net: heap out-of-bounds in fib6_clean_node/rt6_fill_node/fib6_age/fib6_prune_clone

From: David Ahern
Date: Mon Mar 06 2017 - 18:50:22 EST


On 3/6/17 11:51 AM, Dmitry Vyukov wrote:
> We hit it several thousand times, but we get only several dozens of
> crashes per day on ~80 VMs. So if you try to reproduce it on a single
> machine it can take days for a single crash.
> If you are ready to go that route, here are some instructions on
> setting up syzkaller:
> https://github.com/google/syzkaller
> You also need kernel built with CONFIG_KASAN.

ack and I have it setup on ubuntu 16.10 which has a fairly new compiler.

> I am ready to help with resolving any issues.
>
> Another possible route is if you give me a patch with some additional
> WARNINGs. Then I can deploy it to bots and collect stacks.

try the attached.
diff --git a/include/net/dst.h b/include/net/dst.h
index 049af33da3b6..d164eb8ceab8 100644
--- a/include/net/dst.h
+++ b/include/net/dst.h
@@ -58,6 +58,7 @@ struct dst_entry {
#define DST_XFRM_TUNNEL 0x0080
#define DST_XFRM_QUEUE 0x0100
#define DST_METADATA 0x0200
+#define DST_IN_FIB 0x0400

short error;

diff --git a/net/core/dst.c b/net/core/dst.c
index 960e503b5a52..c98447fe8510 100644
--- a/net/core/dst.c
+++ b/net/core/dst.c
@@ -232,6 +232,9 @@ void __dst_free(struct dst_entry *dst)
{
spin_lock_bh(&dst_garbage.lock);
___dst_free(dst);
+if (dst->flags & DST_IN_FIB)
+ pr_warn("dst %p is marked as in fib\n", dst);
+//WARN_ON(dst->flags & DST_IN_FIB);
dst->next = dst_garbage.list;
dst_garbage.list = dst;
if (dst_garbage.timer_inc > DST_GC_INC) {
diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
index e4266746e4a2..a4d55ba00a43 100644
--- a/net/ipv6/ip6_fib.c
+++ b/net/ipv6/ip6_fib.c
@@ -155,6 +155,7 @@ static void node_free(struct fib6_node *fn)

static void rt6_rcu_free(struct rt6_info *rt)
{
+WARN_ON(rt->dst.flags & DST_IN_FIB);
call_rcu(&rt->dst.rcu_head, dst_rcu_free);
}

@@ -878,6 +879,7 @@ static int fib6_add_rt2node(struct fib6_node *fn, struct rt6_info *rt,
return err;

rt->dst.rt6_next = iter;
+ rt->dst.flags |= DST_IN_FIB;
*ins = rt;
rt->rt6i_node = fn;
atomic_inc(&rt->rt6i_ref);
@@ -907,6 +909,7 @@ static int fib6_add_rt2node(struct fib6_node *fn, struct rt6_info *rt,
*ins = rt;
rt->rt6i_node = fn;
rt->dst.rt6_next = iter->dst.rt6_next;
+ rt->dst.flags |= DST_IN_FIB;
atomic_inc(&rt->rt6i_ref);
if (!info->skip_notify)
inet6_rt_notify(RTM_NEWROUTE, rt, info, NLM_F_REPLACE);
@@ -974,6 +977,20 @@ int fib6_add(struct fib6_node *root, struct rt6_info *rt,
!atomic_read(&rt->dst.__refcnt)))
return -EINVAL;

+if (rt->dst.ops->family != AF_INET6) {
+ pr_warn("fib6_add: adding rt with family is %d dst flags %x\n",
+ rt->dst.ops->family, rt->dst.flags);
+
+ WARN_ON(1);
+}
+/* dst.next really should not be set at this point */
+if (rt->dst.next && rt->dst.next->ops->family != AF_INET6) {
+ pr_warn("fib6_add: adding rt with bad next -- family %d dst flags %x\n",
+ rt->dst.next->ops->family, rt->dst.next->flags);
+
+ WARN_ON(1);
+}
+
if (info->nlh) {
if (!(info->nlh->nlmsg_flags & NLM_F_CREATE))
allow_create = 0;
@@ -1444,6 +1461,7 @@ static void fib6_del_route(struct fib6_node *fn, struct rt6_info **rtp,
read_unlock(&net->ipv6.fib6_walker_lock);

rt->dst.rt6_next = NULL;
+ rt->dst.flags &= ~DST_IN_FIB;

/* If it was last route, expunge its radix tree node */
if (!fn->leaf) {
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 229bfcc451ef..e91d7871ccfc 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -1135,6 +1135,8 @@ struct rt6_info *ip6_pol_route(struct net *net, struct fib6_table *table,

dst_hold(&uncached_rt->dst);

+ uncached_rt->dst.flags &= ~DST_IN_FIB;
+
trace_fib6_table_lookup(net, uncached_rt, table->tb6_id, fl6);
return uncached_rt;

@@ -1160,6 +1162,7 @@ struct rt6_info *ip6_pol_route(struct net *net, struct fib6_table *table,
dst_release(&rt->dst);
}

+ pcpu_rt->dst.flags &= ~DST_IN_FIB;
trace_fib6_table_lookup(net, pcpu_rt, table->tb6_id, fl6);
return pcpu_rt;