From patchwork Wed May 12 14:03:54 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Bart De Schuymer X-Patchwork-Id: 52385 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 B0C07B7E11 for ; Thu, 13 May 2010 00:09:09 +1000 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755837Ab0ELOJE (ORCPT ); Wed, 12 May 2010 10:09:04 -0400 Received: from juliette.telenet-ops.be ([195.130.137.74]:57656 "EHLO juliette.telenet-ops.be" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754157Ab0ELOJB (ORCPT ); Wed, 12 May 2010 10:09:01 -0400 X-Greylist: delayed 302 seconds by postgrey-1.27 at vger.kernel.org; Wed, 12 May 2010 10:09:01 EDT Received: from [192.168.1.101] ([84.198.245.80]) by juliette.telenet-ops.be with bizsmtp id Ge3u1e01P1koMCs06e3vmL; Wed, 12 May 2010 16:03:57 +0200 Message-ID: <4BEAB54A.2070203@pandora.be> Date: Wed, 12 May 2010 16:03:54 +0200 From: Bart De Schuymer User-Agent: Thunderbird 2.0.0.24 (X11/20100411) MIME-Version: 1.0 To: Patrick McHardy CC: Eric Dumazet , Stephen Hemminger , netdev@vger.kernel.org Subject: Re: [BUG] crashes with kvm/nat networking and net-next References: <20100511202544.267d33ee@nehalam> <1273649526.2621.3.camel@edumazet-laptop> <4BEA8E79.9000406@trash.net> In-Reply-To: <4BEA8E79.9000406@trash.net> X-Enigmail-Version: 0.96.0 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Patrick McHardy wrote: > Eric Dumazet wrote: >> Le mardi 11 mai 2010 à 20:25 -0700, Stephen Hemminger a écrit : >>> This is a regression that is showing up now in net-next, not sure what >>> changed recently in bridge netfilter that could be causing it? >>> >>> [ 4593.956206] BUG: unable to handle kernel NULL pointer dereference at 0000000000000018 >>> [ 4593.956219] IP: [] br_nf_forward_finish+0x154/0x170 [bridge] >> Not sure, but br_nf_forward_ip() has following check : >> >> if (!skb->nf_bridge) >> return NF_ACCEPT; >> >> while br_nf_forward_arp() missed this check ... >> >> So we can dereference null pointer later > > That looks correct to me, offset 0x18 would be nf_bridge_info->mask. > Bart, please review, thanks. > >> diff --git a/net/bridge/br_netfilter.c b/net/bridge/br_netfilter.c >> index 93f80fe..cd2e5f5 100644 >> --- a/net/bridge/br_netfilter.c >> +++ b/net/bridge/br_netfilter.c >> @@ -723,6 +723,9 @@ static unsigned int br_nf_forward_arp(unsigned int hook, struct sk_buff *skb, >> return NF_ACCEPT; >> #endif >> >> + if (!skb->nf_bridge) >> + return NF_ACCEPT; >> + >> if (skb->protocol != htons(ETH_P_ARP)) { >> if (!IS_VLAN_ARP(skb)) >> return NF_ACCEPT; That won't fix it since nf_bridge isn't used in the ARP case. The correct fix is below. Does anyone know why the null pointer dereference (skb->nf_bridge->mask & BRNF_8021Q) in nf_bridge_update_protocol() didn't cause my uml kernel to crash?? cheers, Bart Don't call nf_bridge_update_protocol() for ARP traffic as skb->nf_bridge isn't used in the ARP case. Signed-off-by: Bart De Schuymer Reported-by: Stephen Hemminger diff --git a/net/bridge/br_netfilter.c b/net/bridge/br_netfilter.c index 93f80fe..4442099 100644 --- a/net/bridge/br_netfilter.c +++ b/net/bridge/br_netfilter.c @@ -643,10 +643,10 @@ static int br_nf_forward_finish(struct sk_buff *skb) skb->pkt_type = PACKET_OTHERHOST; nf_bridge->mask ^= BRNF_PKT_TYPE; } + nf_bridge_update_protocol(skb); } else { in = *((struct net_device **)(skb->cb)); } - nf_bridge_update_protocol(skb); nf_bridge_push_encap_header(skb); NF_HOOK_THRESH(NFPROTO_BRIDGE, NF_BR_FORWARD, skb, in,