From patchwork Tue Oct 12 17:40:45 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Eric Dumazet X-Patchwork-Id: 67602 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 14929B6EFF for ; Wed, 13 Oct 2010 04:41:37 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757954Ab0JLRlZ (ORCPT ); Tue, 12 Oct 2010 13:41:25 -0400 Received: from mail-ww0-f42.google.com ([74.125.82.42]:60614 "EHLO mail-ww0-f42.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757306Ab0JLRlY (ORCPT ); Tue, 12 Oct 2010 13:41:24 -0400 Received: by wwb22 with SMTP id 22so52059wwb.1 for ; Tue, 12 Oct 2010 10:41:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:received:received:subject:from:to:cc :in-reply-to:references:content-type:date:message-id:mime-version :x-mailer:content-transfer-encoding; bh=+ZTzJ7+CmOnCs0yG3A2JsqLkGuBJub1r+cOuEq4HzoI=; b=jRc+0302HLqGv570k27P9WF9a6bR4em0g3P1Yxw8RqwJ9kifpO9XIucYBOQOzJRItn AFPpdFHtVSDqFP+maIYGZ3MwBmYPbXvVOOV7C9KXPQFVjtTu/QnBOYd/2ssH5i47/E+c nMVw2PIzPC/VEemxN2tWA31TDJc5KR/O/1ES8= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=subject:from:to:cc:in-reply-to:references:content-type:date :message-id:mime-version:x-mailer:content-transfer-encoding; b=AuUnYPRWDfaofBkIexlWZ98Lgvpv1FiORefx7IQDZBHbY7evuSf3+HgWNlkWJPjBjH zcyMIQZn5NRjxuizfCWDVszi8ts32RVNugxhVXCD+iAr27KtWPPXKIBdHXhrh7t2AQoX cBOy34e59dWEVyPpP9l8WBB/SBmA84W2NQ3Tw= Received: by 10.216.17.130 with SMTP id j2mr155456wej.47.1286905264951; Tue, 12 Oct 2010 10:41:04 -0700 (PDT) Received: from [192.168.1.21] (167.144.72-86.rev.gaoland.net [86.72.144.167]) by mx.google.com with ESMTPS id n40sm5439536weq.5.2010.10.12.10.41.03 (version=SSLv3 cipher=RC4-MD5); Tue, 12 Oct 2010 10:41:03 -0700 (PDT) Subject: Re: kernel panic in fib_rules_lookup [2.6.27.7 vendor-patched] From: Eric Dumazet To: Joe Buehler Cc: netdev@vger.kernel.org In-Reply-To: References: Date: Tue, 12 Oct 2010 19:40:45 +0200 Message-ID: <1286905245.2703.3.camel@edumazet-laptop> Mime-Version: 1.0 X-Mailer: Evolution 2.30.3 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Le mardi 12 octobre 2010 à 17:14 +0000, Joe Buehler a écrit : > I am seeing a kernel panic (NULL pointer) in fib_rules_lookup. There > were some other reports for 2.6.32 back in March and May. It looks to > me as though "rules_list" is not in a good state when fib_rules_lookup > traverses it. > > My application is bringing TAP interfaces up and down and making > changes to associated routing tables at a fairly good clip (say, a few > times a second). That use case seems to be similar to a previously > reported crash case. > > This is a MIPS kernel (Cavium Octeon) running two CPUs SMP. I am > using 2.6.27.7 patched by Cavium for hardware support reasons. I > cannot upgrade because the vendor patches are non-trivial to > forward-port. > > Here is one stack trace: > > [] fib_rules_lookup+0x11c/0x1a8 > [] fib_lookup+0x2c/0x48 > [] __ip_route_output_key+0x918/0xf38 > [] ip_route_output_flow+0x38/0x2e8 > [] tcp_v4_connect+0x134/0x498 > [] inet_stream_connect+0xf8/0x2f0 > [] sys_connect+0xe0/0xf8 > [] handle_sys+0x12c/0x148 > > Here is another: > > [] fib_rules_lookup+0x11c/0x1a8 > [] fib_lookup+0x2c/0x48 > [] fib_validate_source+0xb0/0x4c0 > [] ip_route_input+0x11a4/0x13c0 > [] ip_rcv_finish+0x2f4/0x4c0 > [] process_backlog+0xa8/0x160 > [] net_rx_action+0x190/0x2e0 > [] __do_softirq+0xf0/0x218 > [] do_softirq+0x78/0x80 > [] plat_irq_dispatch+0x130/0x1e0 > [] ret_from_irq+0x0/0x4 > [] _cond_resched+0x34/0x50 > [] fpu_emulator_cop1Handler+0x90/0x1c80 > [] do_cpu+0x24c/0x360 > [] ret_from_exception+0x0/0x8 > > *IF* my reading of the disassembled code at point of panic is correct, > the "pos" pointer in list_for_each_entry_rcu appears to be NULL. > > Looking at the code in net/core/fib_rules.c I see some uses of the > "rules_list" using rcu and some apparently not. Has something simple > been overlooked? > > I need this fixed so will try adding a spinlock to protect rules_list > if necessary. 2.6.27 is a bit old, you might try : commit 7fa7cb7109d07c29ab28bb877bc7049a0150dbe5 Author: Eric Dumazet Date: Mon Sep 27 04:18:27 2010 +0000 fib: use atomic_inc_not_zero() in fib_rules_lookup It seems we dont use appropriate refcount increment in an rcu_read_lock() protected section. fib_rule_get() might increment a null refcount and bad things could happen. While fib_nl_delrule() respects an rcu grace period before calling fib_rule_put(), fib_rules_cleanup_ops() calls fib_rule_put() without a grace period. Note : after this patch, we might avoid the synchronize_rcu() call done in fib_nl_delrule() Signed-off-by: Eric Dumazet Signed-off-by: David S. Miller --- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/net/core/fib_rules.c b/net/core/fib_rules.c index 42e84e0..d078728 100644 --- a/net/core/fib_rules.c +++ b/net/core/fib_rules.c @@ -225,9 +225,11 @@ jumped: err = ops->action(rule, fl, flags, arg); if (err != -EAGAIN) { - fib_rule_get(rule); - arg->rule = rule; - goto out; + if (likely(atomic_inc_not_zero(&rule->refcnt))) { + arg->rule = rule; + goto out; + } + break; } }