From patchwork Mon Feb 8 09:24:11 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: "Schmid, Carsten" X-Patchwork-Id: 379025 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-12.8 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI, PDS_BAD_THREAD_QP_64, SPF_HELO_NONE, SPF_PASS, URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 08FAFC433DB for ; Mon, 8 Feb 2021 09:28:21 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 910DE64E7B for ; Mon, 8 Feb 2021 09:28:20 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231526AbhBHJ1i (ORCPT ); Mon, 8 Feb 2021 04:27:38 -0500 Received: from esa1.mentor.iphmx.com ([68.232.129.153]:40810 "EHLO esa1.mentor.iphmx.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230239AbhBHJZa (ORCPT ); Mon, 8 Feb 2021 04:25:30 -0500 IronPort-SDR: hB1Nvw8g4LQrxTCq/pK8/XsC+hJJ3/WH491QQc2C+afwk9h0mm0rDbBcnmn/KQZSsOZw7r4tyW l62a1B0rvKpz4I51ct4IblpMHGGqJbYmEVAypr2VgUK6CvVNzzyGAHgGAd7czJI+b3yK9ChzMP cuzCDUw5qTNOYfzl6eI5stgnYRP8LBwttWKjoyjCpqkQYIrc/Dq2lDqaoDye8J/M7cYwrnzNvT tHBxRvuKPp/UK2XGliFFVoimJiOMD5vwffjj5lBRzE5X/FOCfq11xT7nAl0+itfsfc/fsHhXs0 wl8= X-IronPort-AV: E=Sophos;i="5.81,161,1610438400"; d="scan'208,223";a="60168842" Received: from orw-gwy-01-in.mentorg.com ([192.94.38.165]) by esa1.mentor.iphmx.com with ESMTP; 08 Feb 2021 01:24:17 -0800 IronPort-SDR: YyXUU93KmTI2N2Up/kJyjsKZcvO36P3q4bCEdg/cj2VXg5/kQNgnhV+Dc6+ncsmoNjrde9Hnk7 N08aPXP9x2JH9zv9SBGwqZ0J7wrx+DcKnkbzKHQ9sitU3CXJ6ew+0vdnFW6uX18LkabMgEuzKd 7TTmQj7q0moSQAL4PwTUINr5xBjZre9jbNix1VuE1BXyr8jzhcsmHUe+wmXiQoKlybfxANnLjN ppn5GvLlLmOSaFl1pn+q62be1Ckm4z686besYcsQVjUetRYREy43Hl9KWTFoTnYSWEbkB/3pJ+ 7xY= From: "Schmid, Carsten" To: Greg KH CC: "netdev@vger.kernel.org" , "weiwan@google.com" , "idosch@idosch.org" , "jesse@mbuki-mvuki.org" , "kafai@fb.com" , "dsahern@gmail.com" , "davem@davemloft.net" Subject: AW: Where is this patch buried? Thread-Topic: Where is this patch buried? Thread-Index: AQHW/fIkZ019VAXYJE+A3ji1EQtU3KpN8pgAgAAFj1U= Date: Mon, 8 Feb 2021 09:24:11 +0000 Message-ID: <1612776251858.30522@mentor.com> References: <7953a4158fd14aabbcfbad8365231961@SVR-IES-MBX-03.mgc.mentorg.com>, In-Reply-To: Accept-Language: de-DE, en-IE, en-US Content-Language: de-DE X-MS-Has-Attach: yes X-MS-TNEF-Correlator: x-ms-exchange-messagesentrepresentingtype: 1 x-ms-exchange-transport-fromentityheader: Hosted x-originating-ip: [137.202.0.90] MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org >> Hi Greg, >> >> in kernel 4.14 i have seen a NULL pointer deref in >> [65064.613465] RIP: 0010:ip_route_output_key_hash_rcu+0x755/0x850 >> (i have a core dump and detailed analysis) >> >> That looks like this patch could have prevented it: >> >> https://www.spinics.net/lists/stable-commits/msg133055.html >> >> this one was queued for 4.14, but i can't find it in git tree? >> Any idea who/what buried this one? >> >> In 4.19 it is present. >> >> Because our customer uses 4.14 (going to 4.14.212 in a few days) i kindly want to >> ask why this patch hasn't entered 4.14. > > Because it breaks the build? Try it yourself and see what happens :) yep. rt_add_uncached_list is implemented _after_ the call :-( > > I will gladly take a working backport if you can submit it. > Please find it attached - i needed to move rt_add_uncached_list before the rt_cache_route, nothing else changed. This is for 4.14 only, as other kernels do have this patch. I can't reproduce the crash at will, but at least i could compile and flash it on my target. And the target comes up .. hopefully the testing in other/newer kernels done by the developers of the patch is also valid for 4.14. > thanks, > > greg k-h Thanks, Carsten ----------------- Mentor Graphics (Deutschland) GmbH, Arnulfstrasse 201, 80634 München Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Frank Thürauf >From 667f814207fc0aa9b3e620c733d1754ec522c447 Mon Sep 17 00:00:00 2001 From: Wei Wang Date: Wed, 16 Oct 2019 12:03:15 -0700 Subject: [PATCH] ipv4: fix race condition between route lookup and invalidation [ upstream commit 5018c59607a511cdee743b629c76206d9c9e6d7b ] Jesse and Ido reported the following race condition: - Received packet A is forwarded and cached dst entry is taken from the nexthop ('nhc->nhc_rth_input'). Calls skb_dst_set() - Given Jesse has busy routers ("ingesting full BGP routing tables from multiple ISPs"), route is added / deleted and rt_cache_flush() is called - Received packet B tries to use the same cached dst entry from t0, but rt_cache_valid() is no longer true and it is replaced in rt_cache_route() by the newer one. This calls dst_dev_put() on the original dst entry which assigns the blackhole netdev to 'dst->dev' - dst_input(skb) is called on packet A and it is dropped due to 'dst->dev' being the blackhole netdev There are 2 issues in the v4 routing code: 1. A per-netns counter is used to do the validation of the route. That means whenever a route is changed in the netns, users of all routes in the netns needs to redo lookup. v6 has an implementation of only updating fn_sernum for routes that are affected. 2. When rt_cache_valid() returns false, rt_cache_route() is called to throw away the current cache, and create a new one. This seems unnecessary because as long as this route does not change, the route cache does not need to be recreated. To fully solve the above 2 issues, it probably needs quite some code changes and requires careful testing, and does not suite for net branch. So this patch only tries to add the deleted cached rt into the uncached list, so user could still be able to use it to receive packets until it's done. Fixes: 95c47f9cf5e0 ("ipv4: call dst_dev_put() properly") Signed-off-by: Wei Wang Reported-by: Ido Schimmel Reported-by: Jesse Hathaway Tested-by: Jesse Hathaway Acked-by: Martin KaFai Lau Cc: David Ahern Reviewed-by: Ido Schimmel Signed-off-by: David S. Miller Signed-off-by: Carsten Schmid --- net/ipv4/route.c | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/net/ipv4/route.c b/net/ipv4/route.c index c25b3be78e02..7eb8f3048c86 100644 --- a/net/ipv4/route.c +++ b/net/ipv4/route.c @@ -1432,6 +1432,24 @@ static bool rt_bind_exception(struct rtable *rt, struct fib_nh_exception *fnhe, return ret; } +struct uncached_list { + spinlock_t lock; + struct list_head head; +}; + +static DEFINE_PER_CPU_ALIGNED(struct uncached_list, rt_uncached_list); + +static void rt_add_uncached_list(struct rtable *rt) +{ + struct uncached_list *ul = raw_cpu_ptr(&rt_uncached_list); + + rt->rt_uncached_list = ul; + + spin_lock_bh(&ul->lock); + list_add_tail(&rt->rt_uncached, &ul->head); + spin_unlock_bh(&ul->lock); +} + static bool rt_cache_route(struct fib_nh *nh, struct rtable *rt) { struct rtable *orig, *prev, **p; @@ -1451,7 +1469,7 @@ static bool rt_cache_route(struct fib_nh *nh, struct rtable *rt) prev = cmpxchg(p, orig, rt); if (prev == orig) { if (orig) { - dst_dev_put(&orig->dst); + rt_add_uncached_list(orig); dst_release(&orig->dst); } } else { @@ -1462,24 +1480,6 @@ static bool rt_cache_route(struct fib_nh *nh, struct rtable *rt) return ret; } -struct uncached_list { - spinlock_t lock; - struct list_head head; -}; - -static DEFINE_PER_CPU_ALIGNED(struct uncached_list, rt_uncached_list); - -static void rt_add_uncached_list(struct rtable *rt) -{ - struct uncached_list *ul = raw_cpu_ptr(&rt_uncached_list); - - rt->rt_uncached_list = ul; - - spin_lock_bh(&ul->lock); - list_add_tail(&rt->rt_uncached, &ul->head); - spin_unlock_bh(&ul->lock); -} - static void ipv4_dst_destroy(struct dst_entry *dst) { struct dst_metrics *p = (struct dst_metrics *)DST_METRICS_PTR(dst); -- 2.17.1