From patchwork Tue Apr 25 16:17:29 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Ahern X-Patchwork-Id: 754913 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 3wC7fK1FkSz9s8Q for ; Wed, 26 Apr 2017 02:17:45 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=cumulusnetworks.com header.i=@cumulusnetworks.com header.b="CYHhRGUL"; dkim-atps=neutral Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1951438AbdDYQRl (ORCPT ); Tue, 25 Apr 2017 12:17:41 -0400 Received: from mail-io0-f180.google.com ([209.85.223.180]:33052 "EHLO mail-io0-f180.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1950391AbdDYQRi (ORCPT ); Tue, 25 Apr 2017 12:17:38 -0400 Received: by mail-io0-f180.google.com with SMTP id k87so220121215ioi.0 for ; Tue, 25 Apr 2017 09:17:38 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cumulusnetworks.com; s=google; h=from:to:cc:subject:date:message-id; bh=rKLhTTpzmbH2qGTe9bdHtybkv48AXuzNBJxyi8B/Gos=; b=CYHhRGULsLq382lx929BNJzbvozwDHrV7m9zxeDpsngEh3T7igGavMFoYN2KjygkqK HkLia1k8sf9RZoY1jVqnpCfLzpvNH31ejnqtXuHMk9m05DLH9IOrW6UC5k9rcA0a4TPV +3KCOqpm2wWG/adVAVc+TuQ3z1rl0nwYoKOuY= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=rKLhTTpzmbH2qGTe9bdHtybkv48AXuzNBJxyi8B/Gos=; b=fM3STu6BwX0ZKBei2Rnc09SWlk/4dVj1tup/4az400/xcT+EuVgV5pcqQjNgMXHNUc wDMBbB3xPzm8RmVCUDn20SG8rDMHrhatQKU7tpiVv1ZZWCzFU3EfFRVSYyUFk3x1e8d9 akB5jTA27HcTm8R22ouJrk9NPfs2CstXTmACC2bQlFA3qGwI56qGeAuGnNdy7nBGwlOh X4lBecOE2KAAQajoF+adSEH2NnGVkbT+1IFlEzeeftPNXEXRndbSCRYvlFPi2CRD7ns4 Ao8KfKaxsfdVq/AkbzDUiZHG0xBqSkHhWlqPXShraYUvDEXn1lJzL04DLfpaK/+Zd6Sn mzXA== X-Gm-Message-State: AN3rC/7HyU/sxzbRlnc3AX0X3Fe+nZBgueVAyidirpy5xpCfR18S1yu7 aiQb1f07+FmXpDNk X-Received: by 10.107.197.5 with SMTP id v5mr18151924iof.175.1493137057303; Tue, 25 Apr 2017 09:17:37 -0700 (PDT) Received: from kenny.it.cumulusnetworks.com. ([216.129.126.126]) by smtp.googlemail.com with ESMTPSA id a10sm2234728itj.1.2017.04.25.09.17.36 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Tue, 25 Apr 2017 09:17:36 -0700 (PDT) From: David Ahern To: netdev@vger.kernel.org Cc: dvyukov@google.com, andreyknvl@google.com, mmanning@brocade.com, kafai@fb.com, eric.dumazet@gmail.com, David Ahern Subject: [PATCH v4 net] net: ipv6: regenerate host route if moved to gc list Date: Tue, 25 Apr 2017 09:17:29 -0700 Message-Id: <1493137049-16465-1-git-send-email-dsa@cumulusnetworks.com> X-Mailer: git-send-email 2.1.4 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Taking down the loopback device wreaks havoc on IPv6 routing. By extension, taking down a VRF device wreaks havoc on its table. Dmitry and Andrey both reported heap out-of-bounds reports in the IPv6 FIB code while running syzkaller fuzzer. The root cause is a dead dst that is on the garbage list gets reinserted into the IPv6 FIB. While on the gc (or perhaps when it gets added to the gc list) the dst->next is set to an IPv4 dst. A subsequent walk of the ipv6 tables causes the out-of-bounds access. Andrey's reproducer was the key to getting to the bottom of this. With IPv6, host routes for an address have the dst->dev set to the loopback device. When the 'lo' device is taken down, rt6_ifdown initiates a walk of the fib evicting routes with the 'lo' device which means all host routes are removed. That process moves the dst which is attached to an inet6_ifaddr to the gc list and marks it as dead. The recent change to keep global IPv6 addresses added a new function, fixup_permanent_addr, that is called on admin up. That function restarts dad for an inet6_ifaddr and when it completes the host route attached to it is inserted into the fib. Since the route was marked dead and moved to the gc list, re-inserting the route causes the reported out-of-bounds accesses. If the device with the address is taken down or the address is removed, the WARN_ON in fib6_del is triggered. All of those faults are fixed by regenerating the host route if the existing one has been moved to the gc list, something that can be determined by checking if the rt6i_ref counter is 0. Fixes: f1705ec197e7 ("net: ipv6: Make address flushing on ifdown optional") Reported-by: Dmitry Vyukov Reported-by: Andrey Konovalov Signed-off-by: David Ahern Acked-by: Martin KaFai Lau Acked-by: Eric Dumazet --- v4 - move 'prev = ifp->rt;' under spinlock as requested by Eric v3 - removed 'if (prev)' and just call ip6_rt_put; added comment about spinlock v2 - change ifp->rt under spinlock vs cmpxchg - add comment about rt6i_ref == 0 net/ipv6/addrconf.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c index 80ce478c4851..0ea96c4d334d 100644 --- a/net/ipv6/addrconf.c +++ b/net/ipv6/addrconf.c @@ -3271,14 +3271,24 @@ static void addrconf_gre_config(struct net_device *dev) static int fixup_permanent_addr(struct inet6_dev *idev, struct inet6_ifaddr *ifp) { - if (!ifp->rt) { - struct rt6_info *rt; + /* rt6i_ref == 0 means the host route was removed from the + * FIB, for example, if 'lo' device is taken down. In that + * case regenerate the host route. + */ + if (!ifp->rt || !atomic_read(&ifp->rt->rt6i_ref)) { + struct rt6_info *rt, *prev; rt = addrconf_dst_alloc(idev, &ifp->addr, false); if (unlikely(IS_ERR(rt))) return PTR_ERR(rt); + /* ifp->rt can be accessed outside of rtnl */ + spin_lock(&ifp->lock); + prev = ifp->rt; ifp->rt = rt; + spin_unlock(&ifp->lock); + + ip6_rt_put(prev); } if (!(ifp->flags & IFA_F_NOPREFIXROUTE)) {