From patchwork Mon Feb 14 16:24:37 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Patrick McHardy X-Patchwork-Id: 83115 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 6BBCAB71A2 for ; Tue, 15 Feb 2011 03:24:45 +1100 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755891Ab1BNQYk (ORCPT ); Mon, 14 Feb 2011 11:24:40 -0500 Received: from stinky.trash.net ([213.144.137.162]:56030 "EHLO stinky.trash.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753073Ab1BNQYj (ORCPT ); Mon, 14 Feb 2011 11:24:39 -0500 Received: from x100e.localnet (localhost [127.0.0.1]) by stinky.trash.net (Postfix) with ESMTP id 5D5B8B2C50; Mon, 14 Feb 2011 17:24:37 +0100 (MET) Message-ID: <4D595745.7070505@trash.net> Date: Mon, 14 Feb 2011 17:24:37 +0100 From: Patrick McHardy User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.13) Gecko/20101209 Fedora/3.1.7-0.35.b3pre.fc14 Thunderbird/3.1.7 MIME-Version: 1.0 To: Eric Dumazet CC: Jan Engelhardt , Avi Kivity , netfilter-devel@vger.kernel.org, Marcelo Tosatti , nicolas prochazka , KVM list , netdev Subject: Re: Possible netfilter-related memory corruption in 2.6.37 References: <4D594313.4050009@redhat.com> <1297696283.2996.33.camel@edumazet-laptop> <1297698641.2996.38.camel@edumazet-laptop> In-Reply-To: <1297698641.2996.38.camel@edumazet-laptop> Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Am 14.02.2011 16:50, schrieb Eric Dumazet: > Le lundi 14 février 2011 à 16:18 +0100, Jan Engelhardt a écrit : >> On Monday 2011-02-14 16:11, Eric Dumazet wrote: >> >>> Le lundi 14 février 2011 à 16:58 +0200, Avi Kivity a écrit : >>>> We see severe memory corruption in kvm while used in conjunction with >>>> bridge/netfilter. Enabling slab debugging points the finger at a >>>> netfilter chain invoked from the bridge code. >>>> >>>> Can someone take a look? >>>> >>>> https://bugzilla.kernel.org/show_bug.cgi?id=27052 >> >> Maybe looks familiar to https://lkml.org/lkml/2011/2/3/147 > > Are you sure Jan ? > > IMHO it looks like in your case, a NULL ->hook() is called, from > nf_iterate() > > BTW, list_for_each_continue_rcu() really should be converted to > list_for_each_entry_continue_rcu() > > This is a bit ugly : > > list_for_each_continue_rcu(*i, head) { > struct nf_hook_ops *elem = (struct nf_hook_ops *)*i; > > Also, I wonder if RCU rules are respected in nf_iterate(). > For example this line is really suspicious : > > *i = (*i)->prev; Yeah, that definitely looks wrong. How about this instead? Acked-by: Eric Dumazet diff --git a/net/netfilter/core.c b/net/netfilter/core.c index 1e00bf7..899b71c 100644 --- a/net/netfilter/core.c +++ b/net/netfilter/core.c @@ -133,6 +133,7 @@ unsigned int nf_iterate(struct list_head *head, /* Optimization: we don't need to hold module reference here, since function can't sleep. --RR */ +repeat: verdict = elem->hook(hook, skb, indev, outdev, okfn); if (verdict != NF_ACCEPT) { #ifdef CONFIG_NETFILTER_DEBUG @@ -145,7 +146,7 @@ unsigned int nf_iterate(struct list_head *head, #endif if (verdict != NF_REPEAT) return verdict; - *i = (*i)->prev; + goto repeat; } } return NF_ACCEPT;