netdev - Re: [PATCH 06/25] netns ct: per-netns conntrack hash

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] [day] [month] [year] [list]

Message-ID: <485F7959.9000901@trash.net>
Date:	Mon, 23 Jun 2008 12:22:17 +0200
From:	Patrick McHardy <kaber@...sh.net>
To:	Alexey Dobriyan <adobriyan@...il.com>
CC:	netdev@...r.kernel.org, netfilter-devel@...r.kernel.org,
	den@...nvz.org, xemul@...nvz.org, ebiederm@...ssion.com,
	benjamin.thery@...l.net, dlezcano@...ibm.com
Subject: Re: [PATCH 06/25] netns ct: per-netns conntrack hash

Alexey Dobriyan wrote:
> netns is given personal conntrack hash. Another way is to have one hash
> and give tuplehashes ->ct_net pointer. I tried that at some point, it's
> more ugly and more non-obvious.

I think is makes more sense your way, otherwise eviction becomes much
more complicated or people can easily DoS other namespaces.

> Functions that search by tuple (numerical data) get netns argument
> to know where to search as well as conntrack flush functions gets netns
> argument propagated.
> 
> Everybody is stubbed to init_net, except trivial places.
> 
> --- a/include/net/netns/conntrack.h
> +++ b/include/net/netns/conntrack.h
> @@ -5,5 +5,7 @@
>  
>  struct netns_ct {
>  	atomic_t	count;
> +	struct hlist_head	*hash;
> +	int			hash_vmalloc;

Shouldn't the lock also be per namespace?

> --- a/net/netfilter/nf_conntrack_core.c
> +++ b/net/netfilter/nf_conntrack_core.c
> @@ -49,15 +49,11 @@ EXPORT_SYMBOL_GPL(nf_conntrack_htable_size);
>  int nf_conntrack_max __read_mostly;
>  EXPORT_SYMBOL_GPL(nf_conntrack_max);
>  
> -struct hlist_head *nf_conntrack_hash __read_mostly;
> -EXPORT_SYMBOL_GPL(nf_conntrack_hash);
> -
>  struct nf_conn nf_conntrack_untracked __read_mostly;
>  EXPORT_SYMBOL_GPL(nf_conntrack_untracked);

Just a general comments, maybe its done in a later patch. But
for nf_conntrack_cleanup(), you also need to put the untrack
entry in a namespace, otherwise different use in different
namespaces will prevent cleanup from completing.
--
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