From patchwork Wed Jul 11 00:19:22 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Horman X-Patchwork-Id: 170309 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 89FD82C0086 for ; Wed, 11 Jul 2012 10:20:03 +1000 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932130Ab2GKATx (ORCPT ); Tue, 10 Jul 2012 20:19:53 -0400 Received: from kirsty.vergenet.net ([202.4.237.240]:42033 "EHLO kirsty.vergenet.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755082Ab2GKATv (ORCPT ); Tue, 10 Jul 2012 20:19:51 -0400 Received: from ayumi.akashicho.tokyo.vergenet.net (p6117-ipbfp1901kobeminato.hyogo.ocn.ne.jp [114.172.117.117]) by kirsty.vergenet.net (Postfix) with ESMTP id 56BF325C005; Wed, 11 Jul 2012 10:19:50 +1000 (EST) Received: by ayumi.akashicho.tokyo.vergenet.net (Postfix, from userid 7100) id 239D6EDEAEF; Wed, 11 Jul 2012 09:19:46 +0900 (JST) From: Simon Horman To: Pablo Neira Ayuso Cc: lvs-devel@vger.kernel.org, netdev@vger.kernel.org, netfilter-devel@vger.kernel.org, Wensong Zhang , Julian Anastasov , Hans Schillstrom , Jesper Dangaard Brouer , Xiaotian Feng , Xiaotian Feng , Patrick McHardy , "David S. Miller" , Simon Horman Subject: [PATCH 2/3] ipvs: add missing lock in ip_vs_ftp_init_conn() Date: Wed, 11 Jul 2012 09:19:22 +0900 Message-Id: <1341965963-7275-3-git-send-email-horms@verge.net.au> X-Mailer: git-send-email 1.7.10.2.484.gcd07cc5 In-Reply-To: <1341965963-7275-1-git-send-email-horms@verge.net.au> References: <1341965963-7275-1-git-send-email-horms@verge.net.au> Sender: netfilter-devel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netfilter-devel@vger.kernel.org From: Xiaotian Feng We met a kernel panic in 2.6.32.43 kernel: [2680191.848044] IPVS: ip_vs_conn_hash(): request for already hashed, called from run_timer_softirq+0x175/0x1d0 [2680311.849009] general protection fault: 0000 [#1] SMP [2680311.853001] RIP: 0010:[] [] ip_vs_conn_expire+0xdc/0x2f0 [2680311.853001] RSP: 0018:ffff880028303e70 EFLAGS: 00010202 [2680311.853001] RAX: dead000000200200 RBX: ffff8801aad00b80 RCX: 0000000000001d90 [2680311.853001] RDX: dead000000100100 RSI: 000000004fd59800 RDI: ffff8801aad00c08 [2680311.853001] Call Trace: [2680311.853001] [2680311.853001] [] ? ip_vs_conn_expire+0x0/0x2f0 [2680311.853001] [] run_timer_softirq+0x175/0x1d0 [2680311.853001] [] ? lapic_next_event+0x18/0x20 [2680311.853001] [] __do_softirq+0xb3/0x150 [2680311.853001] [] call_softirq+0x1c/0x30 [2680311.853001] [] do_softirq+0x4a/0x80 [2680311.853001] [] irq_exit+0x77/0x80 [2680311.853001] [] smp_apic_timer_interrupt+0x6c/0xa0 [2680311.853001] [] apic_timer_interrupt+0x13/0x20 [2680311.853001] [2680311.853001] [] ? mwait_idle+0x52/0x70 [2680311.853001] [] ? enter_idle+0x20/0x30 [2680311.853001] [] ? cpu_idle+0x52/0x80 [2680311.853001] [] ? start_secondary+0x19d/0x280 rax and rdx is LIST_POISON1 and LIST_POISON2, so kernel is list_del() on an already deleted connection and result the general protect fault. The "request for already hashed" warning, told us someone might change the connection flags incorrectly, like described in commit aea9d711, it changes the connection flags, but doesn't put the connection back to the list. So ip_vs_conn_hash() throw a warning and return. Later, when ip_vs_conn_expire fire again, ip_vs_conn_unhash() will find the HASHED connection and list_del() it, then kernel panic happened. After code review, the only chance that kernel change connection flag without protection is in ip_vs_ftp_init_conn(). Signed-off-by: Xiaotian Feng Cc: Wensong Zhang Cc: Pablo Neira Ayuso Cc: Patrick McHardy Cc: "David S. Miller" Acked-by: Julian Anastasov Signed-off-by: Simon Horman --- net/netfilter/ipvs/ip_vs_ftp.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/net/netfilter/ipvs/ip_vs_ftp.c b/net/netfilter/ipvs/ip_vs_ftp.c index b20b29c..c2bc264 100644 --- a/net/netfilter/ipvs/ip_vs_ftp.c +++ b/net/netfilter/ipvs/ip_vs_ftp.c @@ -65,8 +65,10 @@ static int ip_vs_ftp_pasv; static int ip_vs_ftp_init_conn(struct ip_vs_app *app, struct ip_vs_conn *cp) { + spin_lock(&cp->lock); /* We use connection tracking for the command connection */ cp->flags |= IP_VS_CONN_F_NFCT; + spin_unlock(&cp->lock); return 0; }