[PATCH net v2] netfilter: Use consistent ct id hash calculation

[PATCH net v2] netfilter: Use consistent ct id hash calculation

[Dirk Morris]

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. This
results on the ct id change after the conntrack has been confirmed.
This changes the hash to be based on attributes which should never
change. Now the ct id hash is also consistent from initialization to
conntrack confirmation either even though it is unconfirmed.

Signed-off-by: Dirk Morris <dmorris@metaloft.com>
---
  nf_conntrack_core.c |    7 ++++---
  1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
index ab73c5f..dad6868 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -318,9 +318,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);

Re: [PATCH net v2] netfilter: Use consistent ct id hash calculation

[Florian Westphal]

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. This
> results on the ct id change after the conntrack has been confirmed.
> This changes the hash to be based on attributes which should never
> change. Now the ct id hash is also consistent from initialization to
> conntrack confirmation either even though it is unconfirmed.

Looks good, but can you also fix up the comment at the top of this
function?  (Alternatively, delete those things that are not relevant
anymore).

Also, please add following Tag:

Fixes: 3c79107631db1f7 ("netfilter: ctnetlink: don't use conntrack/expect object addresses as id")

perhaps also mention that hashing the full tuplehash includes
the hlist pointer address, which will change when a conntrack is placed
on the dying list -- this affects reliable delete event delivery --
on redelivery, the id will be different, so your commit is not
just an improvement, it also fixes a bug.

Thanks!

Re: [PATCH net v2] netfilter: Use consistent ct id hash calculation

[Dirk Morris]

On 8/8/19 1:28 PM, Florian Westphal wrote:


> Looks good, but can you also fix up the comment at the top of this
> function?  (Alternatively, delete those things that are not relevant
> anymore).

doh! Sorry should have thought of that.
Yes will also make the other changes and submit v3.