Netfilter-Devel Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH net v3] Use consistent ct id hash calculation
@ 2019-08-08 20:57 Dirk Morris
  2019-08-08 22:04 ` Florian Westphal
  2019-08-13 15:54 ` Pablo Neira Ayuso
  0 siblings, 2 replies; 3+ messages in thread
From: Dirk Morris @ 2019-08-08 20:57 UTC (permalink / raw)
  To: netfilter-devel

Change ct id hash calculation to only use invariants.

Currently the ct id hash calculation is based on some fields that can
change in the lifetime on a conntrack entry in some corner cases. The
current hash uses the whole tuple which contains an hlist pointer
which will change when the conntrack is placed on the dying list
resulting in a ct id change.

This patch also removes the reply-side tuple and extension pointer
from the hash calculation so that the ct id will will not change from
initialization until confirmation.

Fixes: 3c79107631db1f7 ("netfilter: ctnetlink: don't use conntrack/expect object addresses as id")
Signed-off-by: Dirk Morris <dmorris@metaloft.com>
---
  nf_conntrack_core.c |   16 ++++++++--------
  1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
index ab73c5f..f86625f 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -302,13 +302,12 @@ EXPORT_SYMBOL_GPL(nf_ct_invert_tuple);
   * table location, we assume id gets exposed to userspace.
   *
   * Following nf_conn items do not change throughout lifetime
- * of the nf_conn after it has been committed to main hash table:
+ * of the nf_conn:
   *
   * 1. nf_conn address
- * 2. nf_conn->ext address
- * 3. nf_conn->master address (normally NULL)
- * 4. tuple
- * 5. the associated net namespace
+ * 2. nf_conn->master address (normally NULL)
+ * 3. the associated net namespace
+ * 4. the original direction tuple
   */
  u32 nf_ct_get_id(const struct nf_conn *ct)
  {
@@ -318,9 +317,10 @@ u32 nf_ct_get_id(const struct nf_conn *ct)
  	net_get_random_once(&ct_id_seed, sizeof(ct_id_seed));
  
  	a = (unsigned long)ct;
-	b = (unsigned long)ct->master ^ net_hash_mix(nf_ct_net(ct));
-	c = (unsigned long)ct->ext;
-	d = (unsigned long)siphash(&ct->tuplehash, sizeof(ct->tuplehash),
+	b = (unsigned long)ct->master;
+	c = (unsigned long)nf_ct_net(ct);
+	d = (unsigned long)siphash(&ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple,
+				   sizeof(ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple),
  				   &ct_id_seed);
  #ifdef CONFIG_64BIT
  	return siphash_4u64((u64)a, (u64)b, (u64)c, (u64)d, &ct_id_seed);

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH net v3] Use consistent ct id hash calculation
  2019-08-08 20:57 [PATCH net v3] Use consistent ct id hash calculation Dirk Morris
@ 2019-08-08 22:04 ` Florian Westphal
  2019-08-13 15:54 ` Pablo Neira Ayuso
  1 sibling, 0 replies; 3+ messages in thread
From: Florian Westphal @ 2019-08-08 22:04 UTC (permalink / raw)
  To: Dirk Morris; +Cc: netfilter-devel

Dirk Morris <dmorris@metaloft.com> wrote:
> Change ct id hash calculation to only use invariants.
> 
> Currently the ct id hash calculation is based on some fields that can
> change in the lifetime on a conntrack entry in some corner cases. The
> current hash uses the whole tuple which contains an hlist pointer
> which will change when the conntrack is placed on the dying list
> resulting in a ct id change.
> 
> This patch also removes the reply-side tuple and extension pointer
> from the hash calculation so that the ct id will will not change from
> initialization until confirmation.
> 
> Fixes: 3c79107631db1f7 ("netfilter: ctnetlink: don't use conntrack/expect object addresses as id")
> Signed-off-by: Dirk Morris <dmorris@metaloft.com>

Looks good, thanks Dirk.

Acked-by: Florian Westphal <fw@strlen.de>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH net v3] Use consistent ct id hash calculation
  2019-08-08 20:57 [PATCH net v3] Use consistent ct id hash calculation Dirk Morris
  2019-08-08 22:04 ` Florian Westphal
@ 2019-08-13 15:54 ` Pablo Neira Ayuso
  1 sibling, 0 replies; 3+ messages in thread
From: Pablo Neira Ayuso @ 2019-08-13 15:54 UTC (permalink / raw)
  To: Dirk Morris; +Cc: netfilter-devel

On Thu, Aug 08, 2019 at 01:57:51PM -0700, Dirk Morris wrote:
> Change ct id hash calculation to only use invariants.
> 
> Currently the ct id hash calculation is based on some fields that can
> change in the lifetime on a conntrack entry in some corner cases. The
> current hash uses the whole tuple which contains an hlist pointer
> which will change when the conntrack is placed on the dying list
> resulting in a ct id change.
> 
> This patch also removes the reply-side tuple and extension pointer
> from the hash calculation so that the ct id will will not change from
> initialization until confirmation.

Applied, thanks Dirk.

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-08 20:57 [PATCH net v3] Use consistent ct id hash calculation Dirk Morris
2019-08-08 22:04 ` Florian Westphal
2019-08-13 15:54 ` Pablo Neira Ayuso

Netfilter-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/netfilter-devel/0 netfilter-devel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 netfilter-devel netfilter-devel/ https://lore.kernel.org/netfilter-devel \
		netfilter-devel@vger.kernel.org netfilter-devel@archiver.kernel.org
	public-inbox-index netfilter-devel


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.netfilter-devel


AGPL code for this site: git clone https://public-inbox.org/ public-inbox