Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756145Ab2F2BvE (ORCPT ); Thu, 28 Jun 2012 21:51:04 -0400 Received: from mail-bk0-f46.google.com ([209.85.214.46]:43931 "EHLO mail-bk0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754992Ab2F2BvB convert rfc822-to-8bit (ORCPT ); Thu, 28 Jun 2012 21:51:01 -0400 MIME-Version: 1.0 In-Reply-To: References: <1340890587-8169-1-git-send-email-xtfeng@gmail.com> Date: Fri, 29 Jun 2012 09:50:59 +0800 Message-ID: Subject: Re: [RFC PATCH net-next] ipvs: add missing lock in ip_vs_ftp_init_conn() From: Xiaotian Feng To: Julian Anastasov Cc: netdev@vger.kernel.org, lvs-devel@vger.kernel.org, netfilter-devel@vger.kernel.org, netfilter@vger.kernel.org, coreteam@netfilter.org, linux-kernel@vger.kernel.org, Xiaotian Feng , Wensong Zhang , Simon Horman , Pablo Neira Ayuso , Patrick McHardy , "David S. Miller" Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4556 Lines: 105 On Fri, Jun 29, 2012 at 8:17 AM, Julian Anastasov wrote: > > Hello, > > On Thu, 28 Jun 2012, Xiaotian Feng wrote: > >> 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(). > > Hm, ip_vs_ftp_init_conn is called before 1st hashing, > from ip_vs_bind_app() in ip_vs_conn_new() before > ip_vs_conn_hash(). It should be another problem with > the flags. How different is IPVS in 2.6.32.43 compared to > recent kernels? If commit aea9d711 is present, I'm not > aware of other similar problems. ip_vs_bind_app() is also called by ip_vs_try_bind_dest(), which can be traced to ip_vs_proc_conn(). I've checked the changes in upstream, but nothing helps since aea9d711 has been taken into 2.6.32.28 kernel. > > May be you can post some details for your setup on > lvs-devel@vger.kernel.org. I assume FTP is used, > what about master-backup sync? Can you confirm that > this fix solves the problem or it happens in rare cases? Yes, ftp and WRR scheduler is used. Unfortunately, the oops is unreproducible :( > >> Signed-off-by: Xiaotian Feng >> Cc: Wensong Zhang >> Cc: Simon Horman >> Cc: Julian Anastasov >> Cc: Pablo Neira Ayuso >> Cc: Patrick McHardy >> Cc: "David S. Miller" >> --- >> net/netfilter/ipvs/ip_vs_ftp.c | 2 ++ >> 1 files changed, 2 insertions(+), 0 deletions(-) >> >> 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; >> } >> >> -- >> 1.7.1 > > Regards > > -- > Julian Anastasov -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/