From patchwork Tue May 24 17:40:22 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: stephen hemminger X-Patchwork-Id: 97188 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 D480BB6F90 for ; Wed, 25 May 2011 03:40:31 +1000 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755637Ab1EXRk0 (ORCPT ); Tue, 24 May 2011 13:40:26 -0400 Received: from mail.vyatta.com ([76.74.103.46]:34658 "EHLO mail.vyatta.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753777Ab1EXRkZ convert rfc822-to-8bit (ORCPT ); Tue, 24 May 2011 13:40:25 -0400 Received: from localhost (localhost.localdomain [127.0.0.1]) by mail.vyatta.com (Postfix) with ESMTP id 7C21618292E3; Tue, 24 May 2011 10:40:25 -0700 (PDT) X-Virus-Scanned: amavisd-new at tahiti.vyatta.com Received: from mail.vyatta.com ([127.0.0.1]) by localhost (mail.vyatta.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id icpczi4jQ1aJ; Tue, 24 May 2011 10:40:24 -0700 (PDT) Received: from nehalam (static-50-53-80-93.bvtn.or.frontiernet.net [50.53.80.93]) by mail.vyatta.com (Postfix) with ESMTPSA id 35E01182927A; Tue, 24 May 2011 10:40:24 -0700 (PDT) Date: Tue, 24 May 2011 10:40:22 -0700 From: Stephen Hemminger To: Eric Dumazet Cc: David Miller , Herbert Xu , netdev@vger.kernel.org Subject: Re: bridge netfilter output bug on 2.6.39 Message-ID: <20110524104022.212b0b71@nehalam> In-Reply-To: <1306255587.3026.76.camel@edumazet-laptop> References: <20110524074156.58eb30f8@nehalam> <1306251543.3026.57.camel@edumazet-laptop> <1306254457.3026.69.camel@edumazet-laptop> <1306255587.3026.76.camel@edumazet-laptop> Organization: Vyatta X-Mailer: Claws Mail 3.7.6 (GTK+ 2.22.0; x86_64-pc-linux-gnu) Mime-Version: 1.0 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Tue, 24 May 2011 18:46:27 +0200 Eric Dumazet wrote: > Le mardi 24 mai 2011 à 18:27 +0200, Eric Dumazet a écrit : > > Le mardi 24 mai 2011 à 17:39 +0200, Eric Dumazet a écrit : > > > > > I would say its more likely a problem with dst metrics changes > > > > > > In this crash, we dereference a NULL dst->_metrics 'pointer' in > > > dst_metric_raw(dst, RTAX_MTU); > > > > It seems bridge code uses one fake_rtable > > > > You probably want to properly init its _metric field. > > > > I can do the patch in one hour eventually, if nobody beats me. > > > > > > Here is the patch : > > [PATCH] bridge: initialize fake_rtable metrics > > bridge netfilter code uses a fake_rtable, and we must init its _metric > field or risk NULL dereference later. > > Ref: https://bugzilla.kernel.org/show_bug.cgi?id=35672 > > Signed-off-by: Eric Dumazet > CC: Stephen Hemminger > CC: Herbert Xu > --- > net/bridge/br_netfilter.c | 6 +++++- > 1 files changed, 5 insertions(+), 1 deletion(-) > > diff --git a/net/bridge/br_netfilter.c b/net/bridge/br_netfilter.c > index e1f5ec7..3fa1231 100644 > --- a/net/bridge/br_netfilter.c > +++ b/net/bridge/br_netfilter.c > @@ -117,6 +117,10 @@ static struct dst_ops fake_dst_ops = { > * ipt_REJECT needs it. Future netfilter modules might > * require us to fill additional fields. > */ > +static const u32 br_dst_default_metrics[RTAX_MAX] = { > + [RTAX_MTU - 1] = 1500, > +}; > + > void br_netfilter_rtable_init(struct net_bridge *br) > { > struct rtable *rt = &br->fake_rtable; > @@ -124,7 +128,7 @@ void br_netfilter_rtable_init(struct net_bridge *br) > atomic_set(&rt->dst.__refcnt, 1); > rt->dst.dev = br->dev; > rt->dst.path = &rt->dst; > - dst_metric_set(&rt->dst, RTAX_MTU, 1500); > + dst_init_metrics(&rt->dst, br_dst_default_metrics, true); > rt->dst.flags = DST_NOXFRM; > rt->dst.ops = &fake_dst_ops; > } This part is fine. Acked-by: Stephen Hemminger I think there should be BUG_ON any calls to dst_metric_set where dst has no metrics available. [PATCH] dst: catch uninitialized metrics Catch cases where dst_metric_set() and other functions are called but _metrics is NULL. Signed-off-by: Stephen Hemminger --- 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 --- a/include/net/dst.h 2011-05-24 10:36:07.597962703 -0700 +++ b/include/net/dst.h 2011-05-24 10:36:54.382509111 -0700 @@ -111,6 +111,8 @@ static inline u32 *dst_metrics_write_ptr { unsigned long p = dst->_metrics; + BUG_ON(!p); + if (p & DST_METRICS_READ_ONLY) return dst->ops->cow_metrics(dst, p); return __DST_METRICS_PTR(p);