From patchwork Tue May 19 11:04:23 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nikolay Aleksandrov X-Patchwork-Id: 1293212 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming-netdev@ozlabs.org Delivered-To: patchwork-incoming-netdev@ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org (client-ip=23.128.96.18; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=pass (p=none dis=none) header.from=cumulusnetworks.com Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=cumulusnetworks.com header.i=@cumulusnetworks.com header.a=rsa-sha256 header.s=google header.b=L0ht4p/t; dkim-atps=neutral Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by ozlabs.org (Postfix) with ESMTP id 49RChf6S9Nz9sT4 for ; Tue, 19 May 2020 21:05:06 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728632AbgESLFG (ORCPT ); Tue, 19 May 2020 07:05:06 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:53012 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726121AbgESLFF (ORCPT ); Tue, 19 May 2020 07:05:05 -0400 Received: from mail-lj1-x242.google.com (mail-lj1-x242.google.com [IPv6:2a00:1450:4864:20::242]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 18B3EC061A0C for ; Tue, 19 May 2020 04:05:05 -0700 (PDT) Received: by mail-lj1-x242.google.com with SMTP id o14so13265355ljp.4 for ; Tue, 19 May 2020 04:05:05 -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:in-reply-to:references :mime-version:content-transfer-encoding; bh=eSIqIensD5kTG2OFw+qHkUDhInpNgbxwlYI4j1K7t18=; b=L0ht4p/tGakuoyM80/dCiPixROTO1LkEMU/rfKtruL9TSSa/OEvALjH5OcFP9gLyS6 vXDc2TY6eOC2C/Qubmyu2NJmZDLr3mVdo8OZldYQ2hZtNPN316RdJZVHMxvArAl1kMdP BY+RdhwsLBsF4pvhY4oRLCal5/tZ0oCDAMspc= 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:in-reply-to :references:mime-version:content-transfer-encoding; bh=eSIqIensD5kTG2OFw+qHkUDhInpNgbxwlYI4j1K7t18=; b=Jvj6y2c16aCqUuyfr762i9SZQwXO0mXaovwH9kgFlK7WPIx7LpdL+KBNQv2lN0Kedw LM5z+QoQiKpquI4jMGdvWyteNRosWwAdRDzqQ1o0bV0vPnsrl1pmALy2bIpvItFd3Ub/ k/4nGldTWqpNojVMfwyEpudq7lT52CL8KMOa4fNTx2afRlGX+pOizG+ZjfjLtcF0bvxs nfVtZbIaIhPZDtfj3tOrLtqCQPt7l2KvRDIID2UtgHDEx9gbo9xbPowp2LxiaCT5r9vl sjvnwOlkH3kEq19IhJj94wiMgRQG++5RkHFpHrejjnS2vNDwKcYBSTkWW4PnMg/OSWvK VnLA== X-Gm-Message-State: AOAM533Q7ny9R28i2bm228MO+CZGMNp4TV1zvgef89xjrYapWR1O1Ihv 5upyw0BEE4T/RC4a6PQv3EkWDKLWA3z4Yw== X-Google-Smtp-Source: ABdhPJyV9vk6xAZjpp5NbfbgbLPz+QzNGLOz508e6Ap0pM7WuVw2d3gO2DObIhccCgCKFYYTmokQSw== X-Received: by 2002:a2e:864f:: with SMTP id i15mr14036545ljj.179.1589886303224; Tue, 19 May 2020 04:05:03 -0700 (PDT) Received: from localhost.localdomain (84-238-136-197.ip.btc-net.bg. [84.238.136.197]) by smtp.gmail.com with ESMTPSA id t30sm7959443lfd.29.2020.05.19.04.04.59 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 19 May 2020 04:05:02 -0700 (PDT) From: Nikolay Aleksandrov To: netdev@vger.kernel.org Cc: roopa@cumulusnetworks.com, dsahern@gmail.com, Nikolay Aleksandrov Subject: [PATCH net 1/2] net: nexthop: dereference nh only once in nexthop_select_path Date: Tue, 19 May 2020 14:04:23 +0300 Message-Id: <20200519110424.2397623-2-nikolay@cumulusnetworks.com> X-Mailer: git-send-email 2.25.2 In-Reply-To: <20200519110424.2397623-1-nikolay@cumulusnetworks.com> References: <20200519110424.2397623-1-nikolay@cumulusnetworks.com> MIME-Version: 1.0 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org the ->nh pointer might become suddenly null while we're selecting the path and we may dereference it. Dereference it only once in the beginning and use that if it's not null, we rely on the refcounting and rcu to protect against use-after-free. Fixes: 430a049190de ("nexthop: Add support for nexthop groups") Signed-off-by: Nikolay Aleksandrov --- net/ipv4/nexthop.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/net/ipv4/nexthop.c b/net/ipv4/nexthop.c index 2a31c4af845e..a6ffdb067253 100644 --- a/net/ipv4/nexthop.c +++ b/net/ipv4/nexthop.c @@ -490,28 +490,33 @@ struct nexthop *nexthop_select_path(struct nexthop *nh, int hash) nhg = rcu_dereference(nh->nh_grp); for (i = 0; i < nhg->num_nh; ++i) { struct nh_grp_entry *nhge = &nhg->nh_entries[i]; + struct nexthop *nhge_nh; struct nh_info *nhi; if (hash > atomic_read(&nhge->upper_bound)) continue; + nhge_nh = READ_ONCE(nhge->nh); + if (unlikely(!nhge_nh)) + continue; + /* nexthops always check if it is good and does * not rely on a sysctl for this behavior */ - nhi = rcu_dereference(nhge->nh->nh_info); + nhi = rcu_dereference(nhge_nh->nh_info); switch (nhi->family) { case AF_INET: if (ipv4_good_nh(&nhi->fib_nh)) - return nhge->nh; + return nhge_nh; break; case AF_INET6: if (ipv6_good_nh(&nhi->fib6_nh)) - return nhge->nh; + return nhge_nh; break; } if (!rc) - rc = nhge->nh; + rc = nhge_nh; } return rc; From patchwork Tue May 19 11:04:24 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nikolay Aleksandrov X-Patchwork-Id: 1293213 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming-netdev@ozlabs.org Delivered-To: patchwork-incoming-netdev@ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org (client-ip=23.128.96.18; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=pass (p=none dis=none) header.from=cumulusnetworks.com Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=cumulusnetworks.com header.i=@cumulusnetworks.com header.a=rsa-sha256 header.s=google header.b=azLxVH2s; dkim-atps=neutral Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by ozlabs.org (Postfix) with ESMTP id 49RChh5YRDz9sT4 for ; Tue, 19 May 2020 21:05:08 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728737AbgESLFH (ORCPT ); Tue, 19 May 2020 07:05:07 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:53020 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726121AbgESLFH (ORCPT ); Tue, 19 May 2020 07:05:07 -0400 Received: from mail-lj1-x244.google.com (mail-lj1-x244.google.com [IPv6:2a00:1450:4864:20::244]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D48A6C061A0C for ; Tue, 19 May 2020 04:05:06 -0700 (PDT) Received: by mail-lj1-x244.google.com with SMTP id z6so2251103ljm.13 for ; Tue, 19 May 2020 04:05:06 -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:in-reply-to:references :mime-version:content-transfer-encoding; bh=uoyuPqJL+YpP54xcKRDlWfAKsafN7+WQZupPCP2W3fM=; b=azLxVH2sr8CvTXodq79b+mS5zP8WQoY9LT8avBEIPHUxpLK1uDgr1HDTFVIG7Uo1UP Xs5Mi7lBYsT3+OZGPIyW5U2sQTeT/TpKlq5ckm0p56RFdB2zUYQOdr2C29KR4KTvCSkw SBFRaoGMe7jANVcO/UmuiOHXsV5ZXX/VFQ66E= 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:in-reply-to :references:mime-version:content-transfer-encoding; bh=uoyuPqJL+YpP54xcKRDlWfAKsafN7+WQZupPCP2W3fM=; b=bf4KhammnVp7OUjuYlD1Ioic4rmOMRV8H0T4ZHBkY5sa0QIAH45mqYP+uxVobhAtVI /4DJGrTGVXkt9JNDCXYrFxf0tmrvb2fwGq9lM/O63iwnJWYjJBJ+NLZUXnKsrXl2hlQW 1mZdh7unnq1w/bmgw1sMLJt/3hUrKTvROHuRCZ/NJpp3IvWCe8Cyz1FYgdn8xbqwFBff O79mctDT9T03FBZ7s66U1IdFi/EAa9/dLbZzO59B0EKJi6uH2RCbJcUCbu2GhgY7pVRb kirizfi/PTogjws4g55+sxryqj+ynUh1dsYPG7jAdPxcaYDFQVqSQxYpMuLfQbDMhhr0 jP6Q== X-Gm-Message-State: AOAM530E+Ebo8XGfTlTpQYpcaN+Srrl4RVdWFZwPrZKQnmSQG0VoyryL FduzXsQLj+AP3c23igxGqTkeugfAPpll8w== X-Google-Smtp-Source: ABdhPJzu7/JBbxOqd2KjZOdy4hBG0Re50TMtfINCMPBRqRpoCEO4DHyvRjaPFt4jH28Rxk7NDwTG7Q== X-Received: by 2002:a2e:958d:: with SMTP id w13mr6820855ljh.207.1589886304714; Tue, 19 May 2020 04:05:04 -0700 (PDT) Received: from localhost.localdomain (84-238-136-197.ip.btc-net.bg. [84.238.136.197]) by smtp.gmail.com with ESMTPSA id t30sm7959443lfd.29.2020.05.19.04.05.03 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 19 May 2020 04:05:03 -0700 (PDT) From: Nikolay Aleksandrov To: netdev@vger.kernel.org Cc: roopa@cumulusnetworks.com, dsahern@gmail.com, Nikolay Aleksandrov Subject: [PATCH net 2/2] net: nexthop: check for null return by nexthop_select_path() Date: Tue, 19 May 2020 14:04:24 +0300 Message-Id: <20200519110424.2397623-3-nikolay@cumulusnetworks.com> X-Mailer: git-send-email 2.25.2 In-Reply-To: <20200519110424.2397623-1-nikolay@cumulusnetworks.com> References: <20200519110424.2397623-1-nikolay@cumulusnetworks.com> MIME-Version: 1.0 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org nexthop_select_path() may return null if either .nh is null or the number of nexthops is 0 (rc == NULL). We need to check its return value before use to avoid deferencing a null ptr. Fixes: 4c7e8084fd46 ("ipv4: Plumb support for nexthop object in a fib_info") Fixes: f88d8ea67fbd ("ipv6: Plumb support for nexthop object in a fib6_info") Signed-off-by: Nikolay Aleksandrov --- Could you please confirm that simply returning in the IPv6 case is ok? AFAICT it's fine, I've also tested it, but I'm a bit worried about ip6_pol_route_lookup -> ip6_create_rt_rcu and the second directly deferencing res->nh. I think rt6_device_match() should take care of that case, but I'd appreciate more eyes on that. :) include/net/nexthop.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/include/net/nexthop.h b/include/net/nexthop.h index c440ccc861fc..7cc4343cdbfc 100644 --- a/include/net/nexthop.h +++ b/include/net/nexthop.h @@ -203,6 +203,8 @@ static inline void nexthop_path_fib_result(struct fib_result *res, int hash) struct nexthop *nh; nh = nexthop_select_path(res->fi->nh, hash); + if (unlikely(!nh)) + return; nhi = rcu_dereference(nh->nh_info); res->nhc = &nhi->fib_nhc; } @@ -290,7 +292,8 @@ static inline void nexthop_path_fib6_result(struct fib6_result *res, int hash) struct nh_info *nhi; nh = nexthop_select_path(nh, hash); - + if (unlikely(!nh)) + return; nhi = rcu_dereference_rtnl(nh->nh_info); if (nhi->reject_nh) { res->fib6_type = RTN_BLACKHOLE;