netdev - Re: Yet another bridge netfilter crash

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <AANLkTiniy+APw27u3Ld8YrRWqP7wwkSEoU=u94nhkxPd@mail.gmail.com>
Date:	Thu, 5 Aug 2010 07:00:22 +0800
From:	Changli Gao <xiaosuo@...il.com>
To:	Patrick McHardy <kaber@...sh.net>
Cc:	Herbert Xu <herbert@...dor.apana.org.au>,
	Stephen Hemminger <shemminger@...tta.com>,
	netdev@...r.kernel.org
Subject: Re: Yet another bridge netfilter crash

On Thu, Aug 5, 2010 at 12:30 AM, Patrick McHardy <kaber@...sh.net> wrote:
> Am 23.07.2010 17:26, schrieb Herbert Xu:
>> On Fri, Jul 23, 2010 at 05:17:42PM +0200, Patrick McHardy wrote:
>>>
>>>> There's also the matter of fragments jumping between bridges.
>>>
>>> Conntrack zones can be used to avoid that, but that currently needs
>>> manual configuration.
>>
>> I think this is something that we need to fix.  Because as it
>> stands, it can still crash if you get the wrong nf_bridge.
>>
>> The reason is that skb->dev does not hold a ref count.  So the
>> reassembly code just throws it away and always uses the dev of
>> the last fragment.
>>
>> This breaks when two bridges combine to reassemble a single
>> packet, as the nf_bridge attribute of the reassembled packet
>> may come from an skb whose device is now dead.  This is then
>> used to fill in the skb->dev (via nf_bridge->physindev).
>
> We could perform a new device lookup on reassembly as we do
> when expiring a fragment queue, but we probably shouldn't even
> be reassembling fragments from different bridges. One way to
> avoid this would be to automatically assign each bridge device
> to a different conntrack zone, but conntrack zones are limited
> to 2^16 and this might also have other unwanted side-effects.
>
> Until we come up with something better the best fix seems to
> be to perform the device lookup based on the iif.

How about holding physindev and physoutdev when queueing the skbs into
frag queue, and take the skb->dev(bridge NIC) as a key when queueing
the skbs from bridges?

-- 
Regards,
Changli Gao(xiaosuo@...il.com)
--
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