netdev - Re: [Bugme-new] [Bug 9758] New: net_device refcnt bug when NFQUEUEing bridged packets

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]

Date:	Wed, 16 Jan 2008 05:54:13 +0100
From:	Patrick McHardy <kaber@...sh.net>
To:	jckn@....net,
	Netfilter Development Mailinglist 
	<netfilter-devel@...r.kernel.org>
CC:	Andrew Morton <akpm@...ux-foundation.org>, netdev@...r.kernel.org,
	bugme-daemon@...zilla.kernel.org
Subject: Re: [Bugme-new] [Bug 9758] New: net_device refcnt bug when NFQUEUEing
 bridged packets

Andrew Morton wrote:
> On Tue, 15 Jan 2008 15:28:31 -0800 (PST)
> bugme-daemon@...zilla.kernel.org wrote:
> 
>> http://bugzilla.kernel.org/show_bug.cgi?id=9758
>>
>> The bug is probably around since the combination bridge+NFQUEUE is possible,
>> and does not depend on distro or environment:
>>
>> Packets that are to be sent out over a bridge device are skb_clone()d in
>> br_loop() before traversing the appropriate (FORWARD/OUTPUT) NF chain.
>> The copies made by skb_clone() share their nf_bridge metadata with the
>> original, which is no problem usually.
>> If however one or more packets of a br_loop() run end up in a NFQUEUE,
>> their shared nf_bridge metadata causes trouble when they are about to be
>> reinjected: nf_reinject() decrements the net_device refcounts that were
>> previously upped when queueing the packet in __nf_queue(), but as
>> skb->nf_bridge->physoutdev points to the same device for all these
>> packets, most (if not all) of them will affect the wrong refcnt.
>>
>> (I originally encountered the bug on a Xen host because the hypervisor
>> refused to shutdown a virtual device with non-zero refcount... but it is
>> perfectly reproducible with a standard kernel, too, although it was a
>> bit more tedious to create a test scenario, involving a couple of UMLs.)
>>
>> I'd suggest to make a real copy of the nf_bridge member in br_loop() if
>> CONFIG_BRIDGE_NETFILTER is defined, remedying the entanglement. I'd go ahead
>> and create a patch, but I'm unsure as to where that logic should be
>> implemented.


Very nice catch, that explains quite a few bug reports about
refcnt leaks. Your patch looks correct and performs the copying
in the logically correct place, it would be nicer to keep this
crap limited to bridge netfilter however.

What should work is to perform the copying in br_netfilter.c
at the spots where phsyoutdev is assigned. As an optimization
we should be able to avoid the copying in most cases by
checking that the bridge info has a refcount above 1.

Could you test whether this patch also fixes the problem?



View attachment "x" of type "text/plain" (1546 bytes)