From patchwork Tue May 9 23:50:05 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Dumazet X-Patchwork-Id: 760368 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 3wMx1v3Glfz9ryQ for ; Wed, 10 May 2017 09:50:11 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="gzIeGOcm"; dkim-atps=neutral Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1750926AbdEIXuJ (ORCPT ); Tue, 9 May 2017 19:50:09 -0400 Received: from mail-pg0-f67.google.com ([74.125.83.67]:36114 "EHLO mail-pg0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750733AbdEIXuI (ORCPT ); Tue, 9 May 2017 19:50:08 -0400 Received: by mail-pg0-f67.google.com with SMTP id 64so1774991pgb.3 for ; Tue, 09 May 2017 16:50:07 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=message-id:subject:from:to:cc:date:in-reply-to:references :mime-version:content-transfer-encoding; bh=vonv2MiM3ZQUCNp2oChPKPE3E6HyH1nIURuDysvuksU=; b=gzIeGOcmUm4L1IMqfQ5mBr0IY7kMxqpwUijXOwb0Hk2ZH/U0A9LuFuwEkpYkHCMyyX H0Nr/VebQH3kiADUaQNpM9oSe766eRMkKMtHKrlkH9JczND01Bg3uzsy8muPQaxyXH2C /f2uVqSQ+lAE1sL3B8f7jenqJEUpM/AVJP0L4lCh4vw46PrJ/W66dfWjqmexA7fjAQfD DlFX/q1R0Spjiihn1vnuheAiH9KZNREc7ntn1ZD7HPc3J0eDFyhNNt+PXBm9IAFlfhsx e5o1VD5cp4AWso4gMPIErfkNENOJKn6yDCt6bmYwWzoWLMgfOYgxbwXXVJ03+nkK3PTh mwrA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:message-id:subject:from:to:cc:date:in-reply-to :references:mime-version:content-transfer-encoding; bh=vonv2MiM3ZQUCNp2oChPKPE3E6HyH1nIURuDysvuksU=; b=n8iDPwgAqQwmzBhaLwZIQMyCdoSGlyKrHfPBAtGEzmHt5X0Ewd76lsiETVACILnoqA zdN+poSXcUQSxb8qRG15BpCRUH7tLF2nqH1Al1xrEjod+CbiodE2EZhJGPN8JTfyzww1 bvYAMw7wTAscr/Ry8mWyceV/WTGcLjbC/3ovqro/jfUtul5V7D8Ppeon4S2IZi+tq8NA dT1LcHOF07MBe5jU/OvDuEh0gQ2ujJEu2MrU9Cn4j4t7LkjJCW7bi4S78KkLhgLg9oGw 52kNID7T251uwB07tb7Zv0HftP4a6Kuo0UT8nOsoFMfh5CtYVUrq/KpAu7QWqp/bwpMz kevA== X-Gm-Message-State: AODbwcAK1CyxfaoAiWUGDMp/CUDHKgk+57DGyzCKBaPXSJuOZfP1vOr8 Itv2nbUfvMWF1w== X-Received: by 10.84.194.37 with SMTP id g34mr3921764pld.182.1494373807417; Tue, 09 May 2017 16:50:07 -0700 (PDT) Received: from ?IPv6:2620:15c:2c1:100:c4f2:a756:d300:5869? ([2620:15c:2c1:100:c4f2:a756:d300:5869]) by smtp.googlemail.com with ESMTPSA id 12sm1724875pgb.35.2017.05.09.16.50.06 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 09 May 2017 16:50:06 -0700 (PDT) Message-ID: <1494373805.7796.98.camel@edumazet-glaptop3.roam.corp.google.com> Subject: Re: [Patch net] ipv4: restore rt->fi for reference counting From: Eric Dumazet To: Cong Wang Cc: David Miller , Linux Kernel Network Developers , Andrey Konovalov , Eric Dumazet Date: Tue, 09 May 2017 16:50:05 -0700 In-Reply-To: References: <1493934857-6693-1-git-send-email-xiyou.wangcong@gmail.com> <20170508.143557.105629611489969352.davem@davemloft.net> <1494288080.7796.59.camel@edumazet-glaptop3.roam.corp.google.com> <20170508.212211.1291611254198273979.davem@davemloft.net> <1494296302.7796.61.camel@edumazet-glaptop3.roam.corp.google.com> <1494348962.7796.88.camel@edumazet-glaptop3.roam.corp.google.com> <1494370367.7796.92.camel@edumazet-glaptop3.roam.corp.google.com> <1494370451.7796.93.camel@edumazet-glaptop3.roam.corp.google.com> <1494371348.7796.95.camel@edumazet-glaptop3.roam.corp.google.com> X-Mailer: Evolution 3.10.4-0ubuntu2 Mime-Version: 1.0 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Tue, 2017-05-09 at 16:35 -0700, Cong Wang wrote: > All of them take RCU read lock, so, as I explained in the code comment, > they all should be fine because of synchronize_net() on unregister path. > Do you see anything otherwise? They might take rcu lock, but compiler is still allowed to read fi->fib_dev multiple times, and crashes might happen. You will need to audit all code and fix it, using proper rcu_dereference() or similar code ensuring compiler wont do stupid things. Like : diff --git a/net/ipv4/fib_trie.c b/net/ipv4/fib_trie.c index 1201409ba1dcb18ee028003b065410b87bf4a602..ab69517befbb5f300af785fbb20071a3d1086593 100644 --- a/net/ipv4/fib_trie.c +++ b/net/ipv4/fib_trie.c @@ -2666,11 +2666,13 @@ static int fib_route_seq_show(struct seq_file *seq, void *v) seq_setwidth(seq, 127); - if (fi) + if (fi) { + struct net_device *dev = rcu_dereference(fi->fib_dev); + seq_printf(seq, "%s\t%08X\t%08X\t%04X\t%d\t%u\t" "%d\t%08X\t%d\t%u\t%u", - fi->fib_dev ? fi->fib_dev->name : "*", + dev ? dev->name : "*", prefix, fi->fib_nh->nh_gw, flags, 0, 0, fi->fib_priority, @@ -2679,13 +2681,13 @@ static int fib_route_seq_show(struct seq_file *seq, void *v) fi->fib_advmss + 40 : 0), fi->fib_window, fi->fib_rtt >> 3); - else + } else { seq_printf(seq, "*\t%08X\t%08X\t%04X\t%d\t%u\t" "%d\t%08X\t%d\t%u\t%u", prefix, 0, flags, 0, 0, 0, mask, 0, 0, 0); - + } seq_pad(seq, '\n'); }