netdev - Re: Oops with latest (netfilter) nf-next tree, when unloading iptable

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:	Thu, 20 Sep 2012 19:06:52 +0200 (MEST)
From:	Patrick McHardy <kaber@...sh.net>
To:	Pablo Neira Ayuso <pablo@...monks.org>
cc:	Jesper Dangaard Brouer <brouer@...hat.com>,
	Florian Westphal <fw@...len.de>,
	netfilter-devel <netfilter-devel@...r.kernel.org>,
	netdev <netdev@...r.kernel.org>, yongjun_wei@...ndmicro.com.cn
Subject: Re: Oops with latest (netfilter) nf-next tree, when unloading
 iptable_nat

On Thu, 20 Sep 2012, Patrick McHardy wrote:

>>> diff --git a/net/netfilter/nf_conntrack_core.c 
>>> b/net/netfilter/nf_conntrack_core.c
>>> index dcb2791..0f241be 100644
>>> --- a/net/netfilter/nf_conntrack_core.c
>>> +++ b/net/netfilter/nf_conntrack_core.c
>>> @@ -1224,6 +1224,8 @@ get_next_corpse(struct net *net, int (*iter)(struct 
>>> nf_conn *i, void *data),
>>>  	spin_lock_bh(&nf_conntrack_lock);
>>>  	for (; *bucket < net->ct.htable_size; (*bucket)++) {
>>>  		hlist_nulls_for_each_entry(h, n, &net->ct.hash[*bucket], 
>>> hnnode) {
>>> +			if (NF_CT_DIRECTION(h) != IP_CT_DIR_ORIGINAL)
>>> +				continue;
>> 
>> I think this will make the deletion of entries via `conntrack -F'
>> slowier as we'll have to iterate over more entries (we won't delete
>> entries for the reply tuple).
>
> Slightly maybe, but I doubt it makes much of a difference.
>
>> I think I prefer Florian's patch, it's fairly small and it does not
>> change the current nf_ct_iterate behaviour or adding some
>> nf_nat_iterate cleanup.
>
> I don't think I've received it. Could you forward it to me please?

Florian forwarded the patch to me. While it fixes the problem, it
is a workaround and it certainly is inelegant to do the
list_del_rcu_init() and memset up to *four* times for a single conntrack.

The correct thing IMO is to invoke the callbacks exactly once per
conntrack, either through my nf_ct_iterate_cleanup() change or through
a new iteration function for callers that don't kill conntracks. As
soon as we start generating events for NAT section cleanup this will be
needed in any case.

Unless I'm missing something, conntrack flushing is also a really rare 
operation anyways and for large tables where this might make a small
difference will take a quite large time anyway.
--
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