From patchwork Tue Mar 7 18:43:01 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Ahern X-Patchwork-Id: 736328 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3vd5J170Pvz9sNC for ; Wed, 8 Mar 2017 05:47:45 +1100 (AEDT) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=cumulusnetworks.com header.i=@cumulusnetworks.com header.b="Poa0vvgK"; dkim-atps=neutral Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755850AbdCGSnq (ORCPT ); Tue, 7 Mar 2017 13:43:46 -0500 Received: from mail-pg0-f43.google.com ([74.125.83.43]:36403 "EHLO mail-pg0-f43.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754950AbdCGSnn (ORCPT ); Tue, 7 Mar 2017 13:43:43 -0500 Received: by mail-pg0-f43.google.com with SMTP id 187so3441661pgb.3 for ; Tue, 07 Mar 2017 10:43:04 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cumulusnetworks.com; s=google; h=subject:to:references:cc:from:message-id:date:user-agent :mime-version:in-reply-to; bh=5RiPKpchKfKnvVrkKy7P+vJOJGMl+xcWVpKHiwVMJtI=; b=Poa0vvgK0/XIviz4c3utZhz6pCUCuwa6DXxtL8Zwv6rwxJbuVsxlvsdvvkzfw2Z73S 3Lq2FWRsJ7QXP07ICueHj07HKRzqul2ddVQqVdDXCRlNDv93dywFYpj5kSeinbLRmoGZ /OWXqnV4cviRTtllWUXW99VleOFKCqXJLR67o= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:cc:from:message-id:date :user-agent:mime-version:in-reply-to; bh=5RiPKpchKfKnvVrkKy7P+vJOJGMl+xcWVpKHiwVMJtI=; b=LidsWFEKptc+8BuQGtbEwG97uEL1X0yf9I5r7vf5itBM4LG45FA64JntHKO15NvJll ZaAHF5qQzcEqfQJasR97DA79rA07btC7/INP8FLuZVa7fM6v/DOXgeWh9rzo7V7YxBby xkWxcAfh9Sj/K58FojSL9UNsNoK4F+uN8mvI3yIpQHe5qTIielyb+Wp6TsCuiZaJLCtW 1BvwSTdqcE1+eiiMAFkDFLcbyU+oIlpdP3ivsIZoWHPZTLYtKQxhPcV+h9d3qIsKreLb ooSFZT3EH7WVmiL/LImnxmX07h6GZJuaJRXoUltoWV3rgbdP4pMy0id16wT93OIGpaKP Bd/w== X-Gm-Message-State: AMke39kULYuhe1vj7breTlBkBTZpciOL0AMyvMrNPGSomcMqYp/xE0nAv1hpMKLyvUsUAQrS X-Received: by 10.84.248.79 with SMTP id e15mr2438431pln.133.1488912184407; Tue, 07 Mar 2017 10:43:04 -0800 (PST) Received: from [172.27.227.238] ([216.129.126.118]) by smtp.googlemail.com with ESMTPSA id q64sm1233445pfi.69.2017.03.07.10.43.02 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 07 Mar 2017 10:43:03 -0800 (PST) Subject: Re: net: heap out-of-bounds in fib6_clean_node/rt6_fill_node/fib6_age/fib6_prune_clone To: Dmitry Vyukov References: <1488658514.9415.356.camel@edumazet-glaptop3.roam.corp.google.com> <4e4d7d51-82de-b21e-cb5d-d804f7b88999@cumulusnetworks.com> <14c01aea-6c2f-6ba5-6aee-52c55f410da7@cumulusnetworks.com> <2b60b1b8-4766-0e36-f6fb-79914bf1925d@cumulusnetworks.com> Cc: Eric Dumazet , Mahesh Bandewar , Eric Dumazet , David Miller , Alexey Kuznetsov , James Morris , Hideaki YOSHIFUJI , Patrick McHardy , netdev , LKML , Cong Wang , syzkaller From: David Ahern Message-ID: <328b1fa7-2d97-6ae3-3b87-e33a0d564ad9@cumulusnetworks.com> Date: Tue, 7 Mar 2017 11:43:01 -0700 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.12; rv:45.0) Gecko/20100101 Thunderbird/45.7.1 MIME-Version: 1.0 In-Reply-To: Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On 3/7/17 11:13 AM, Dmitry Vyukov wrote: >> on this warning: >> >> /* 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); >> } >> >> You should have seen the pr_warn in the log preceding the WARN_ON dump. > > Right. They all have the same "IPv6: fib6_add: adding rt with bad next > -- family 2 dst flags 6" remove the previous changes and 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/include/net/dst_ops.h b/include/net/dst_ops.h index c84b3287e38b..cd0df8f76420 100644 --- a/include/net/dst_ops.h +++ b/include/net/dst_ops.h @@ -15,6 +15,7 @@ struct dst_ops { unsigned short family; unsigned int gc_thresh; + void (*dump)(struct dst_entry *); int (*gc)(struct dst_ops *ops); struct dst_entry * (*check)(struct dst_entry *, __u32 cookie); unsigned int (*default_advmss)(const struct dst_entry *); 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..d4539d9a463e 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); @@ -916,6 +919,7 @@ static int fib6_add_rt2node(struct fib6_node *fn, struct rt6_info *rt, } nsiblings = iter->rt6i_nsiblings; fib6_purge_rt(iter, fn, info->nl_net); + iter->dst.flags &= ~DST_IN_FIB; rt6_release(iter); if (nsiblings) { @@ -926,6 +930,7 @@ static int fib6_add_rt2node(struct fib6_node *fn, struct rt6_info *rt, if (rt6_qualify_for_ecmp(iter)) { *ins = iter->dst.rt6_next; fib6_purge_rt(iter, fn, info->nl_net); + iter->dst.flags &= ~DST_IN_FIB; rt6_release(iter); nsiblings--; } else { @@ -974,6 +979,21 @@ 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); + + if (rt->dst.ops->dump) + rt->dst.ops->dump(&rt->dst); +} + if (info->nlh) { if (!(info->nlh->nlmsg_flags & NLM_F_CREATE)) allow_create = 0; @@ -1444,6 +1464,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..e83b5ef7fbcd 100644 --- a/net/ipv6/route.c +++ b/net/ipv6/route.c @@ -238,10 +238,22 @@ static void ip6_confirm_neigh(const struct dst_entry *dst, const void *daddr) __ipv6_confirm_neigh(dev, daddr); } +static void ip6_dst_dump(struct dst_entry *dst) +{ + struct rt6_info *rt = (struct rt6_info *) dst; + + pr_warn("rt %p: dev %s gw %pI6c dst %pI6c/%d src %pI6c prefsrc %pI6c flags %x rt6i_nsiblings %u\n", + rt, rt->rt6i_idev ? rt->rt6i_idev->dev->name : "", + &rt->rt6i_gateway, &rt->rt6i_dst.addr, rt->rt6i_dst.plen, + &rt->rt6i_src.addr, &rt->rt6i_prefsrc.addr, + rt->rt6i_flags, rt->rt6i_nsiblings); +} + static struct dst_ops ip6_dst_ops_template = { .family = AF_INET6, .gc = ip6_dst_gc, .gc_thresh = 1024, + .dump = ip6_dst_dump, .check = ip6_dst_check, .default_advmss = ip6_default_advmss, .mtu = ip6_mtu, @@ -1135,6 +1147,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 +1174,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;