From mboxrd@z Thu Jan  1 00:00:00 1970
From: "Denis V. Lunev" <den@openvz.org>
Subject: Re: 2.6.25-rc: Null dereference in ip_defrag
Date: Mon, 17 Mar 2008 20:57:42 +0300
Message-ID: <1205776662.27115.1.camel@iris.sw.ru>
References: <20080317170008.GA30338@linuxace.com>
	 <47DEACF7.10202@openvz.org>  <47DEADC4.4010609@trash.net>
Mime-Version: 1.0
Content-Type: text/plain
Content-Transfer-Encoding: 7bit
Cc: Pavel Emelyanov <xemul@openvz.org>,
	Phil Oester <kernel@linuxace.com>, netdev@vger.kernel.org
To: Patrick McHardy <kaber@trash.net>
Return-path: <netdev-owner@vger.kernel.org>
Received: from mailhub.sw.ru ([195.214.232.25]:44814 "EHLO relay.sw.ru"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1752524AbYCQR5q (ORCPT <rfc822;netdev@vger.kernel.org>);
	Mon, 17 Mar 2008 13:57:46 -0400
In-Reply-To: <47DEADC4.4010609@trash.net>
Sender: netdev-owner@vger.kernel.org
List-ID: <netdev.vger.kernel.org>

On Mon, 2008-03-17 at 18:43 +0100, Patrick McHardy wrote:
> Pavel Emelyanov wrote:
> > Phil Oester wrote:
> >> And the packets causing the problem are all multicast fragments being
> >> generated by Quagga's OSPFD (see debug output below).  Tried manually generating
> >> some multicast fragments with iperf, but couldn't reproduce it.  
> >>
> >> Any ideas?
> > 
> > This is the same as the problem fixed here:
> > 
> >     commit 4136cd523eb0c0bd53173e16fd7406d31d05824f
> >     [IPV4]: route: fix crash ip_route_input
> > 
> > The sk_buff does not have a valid dev sometimes in ip_defrag() :(
> > and you have to setup conntrack rules to make packets go this way.
> > But unlike the above problem we cannot even trust the skb->dst to
> > be not NULL...
> 
> 
> We can on output. Usually we don't even see fragments in conntrack
> on output since we've defer fragmentation until all netfilter hooks
> have been processed. Quagga is generating fragments using raw sockets
> and IP_HDRINCL though.
> 
>  > Can you check with this patch, please (untested, but should work)?
> 
> This is getting pretty ugly. Shouldn't
> 
> int ip_defrag(struct sk_buff *skb, u32 user)
> {
> ...
> -         net = skb->dev->nd_net;
> +         net = skb->dev ? skb->dev->nd_net : skb->dst->dev->nd_net;
>>From my POW the we can just get skb->dst->dev. Could we?

I think that on IP level dealing with fragments we always have the
destination entry. I'll recheck this tomorrow.