netdev - Re: [PATCHv3] netns: oops in ip[6]_frag

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20090318072658.GA4799@ff.dom.local>
Date:	Wed, 18 Mar 2009 07:26:58 +0000
From:	Jarek Poplawski <jarkao2@...il.com>
To:	jorge@...2.net
Cc:	netdev@...r.kernel.org
Subject: Re: [PATCHv3] netns: oops in ip[6]_frag_reasm incrementing stats

On 17-03-2009 14:54, Jorge Boncompte [DTI2] wrote:
> Jarek Poplawski escribió:
>> On Tue, Mar 17, 2009 at 12:55:42PM +0100, Jorge Boncompte [DTI2] wrote:
>>> dev can be NULL in ip[6]_frag_reasm for skb's coming from RAW sockets.
>>>
>>> Quagga's OSPFD sends fragmented packets on a RAW socket, when netfilter
>>> conntrack reassembles them on the OUTPUT path you hit this code path.
>>>
>>> You can test it with something like "hping2 -0 -d 2000 -f AA.BB.CC.DD"
>>>
>>> Changes from v2: (address comments from Jarek Poplawski)
>>> 	- Patch reworked to get the net pointer with container_of()
>>> 	  instead of passing it to function calls.
>>> 	- Fix IPv6 code
>>> Changes from v1:
>>> 	- Fixed description
>> I guess David will be interested only with the final state of changes,
>> so v1 & v2 are not necessary here...
>>
>> Anyway, ipv4 looks OK to me, but ipv6 looks like something is
>> different:
>>> +	IP6_INC_STATS_BH(net, __in6_dev_get(dev), IPSTATS_MIB_REASMFAILS);
>> It still depends on dev != NULL in __in6_dev_get(). I see there
>> is also used skb->dst for similar things in ip6_frag_queue(), so I
>> don't know: it needs rethinking, and maybe these patches should be
>> separated if you prefer.
> 
> 	Not my day! :-) I should not look at code at 2 am and write patches
> the day after, I confused _idev and idev in the check for != NULL in _DEVINC.
> 
> 	I think this bug was first introduced by patch "[IPV6]: Per-interface
> statistics support." from YOSHIFUJI Hideaki on Nov 4, 2006.
> 
> 	If someone with more knowledge could confirm that using something like...
> 
> "(skb->dev ? skb->dev : skb->dst->dev)"
> 
> ... here is fine I'll redo this part and resend. I do not have an IPv6 setup where
> I can test this.
> 

skb->dst->dev is used in ip6_frag_queue anyway, so it shouldn't be
worse if you do this similarly in ip6_frag_reasm. If you send it as
a separate patch, and write it's not tested, David will decide if he
wants it. Otherwise you can resend this ipv4 patch only.

Jarek P.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html