During the creation of a new connection, there's a chance both key and rev_key end up having the same hash. This is more common in the case of all-zero snat with no collisions. In that case, once the connection is expired, but not cleaned up, if a new packet with the same 5-tuple is received, an assertion failure gets triggered in conn_update_state() because of a previous failure of retrieving a CT_CONN_TYPE_DEFAULT connection.
Fix it by releasing the nat_conn during the connection creation in the case of same hash for both key and rev_key. Reported-by: Michael Plato <michael.pl...@tu-berlin.de> Fixes: 61e48c2d1db2 ("conntrack: Handle SNAT with all-zero IP address.") Signed-off-by: Paolo Valerio <pvale...@redhat.com> --- In this thread [0] there are some more details. A similar approach here could be to avoid to add the nat_conn to the cmap and letting the sweeper release the memory for nat_conn once the whole connection gets freed. That approach could still be ok, but the drawback is that it could require a different patch for older branches that don't include 3d9c1b855a5f ("conntrack: Replace timeout based expiration lists with rculists."). It still worth to be considered. [0] https://mail.openvswitch.org/pipermail/ovs-discuss/2023-April/052339.html --- lib/conntrack.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/lib/conntrack.c b/lib/conntrack.c index 7e1fc4b1f..d2ee127d9 100644 --- a/lib/conntrack.c +++ b/lib/conntrack.c @@ -1007,14 +1007,19 @@ conn_not_found(struct conntrack *ct, struct dp_packet *pkt, } nat_packet(pkt, nc, false, ctx->icmp_related); - memcpy(&nat_conn->key, &nc->rev_key, sizeof nat_conn->key); - memcpy(&nat_conn->rev_key, &nc->key, sizeof nat_conn->rev_key); - nat_conn->conn_type = CT_CONN_TYPE_UN_NAT; - nat_conn->nat_action = 0; - nat_conn->alg = NULL; - nat_conn->nat_conn = NULL; - uint32_t nat_hash = conn_key_hash(&nat_conn->key, ct->hash_basis); - cmap_insert(&ct->conns, &nat_conn->cm_node, nat_hash); + uint32_t nat_hash = conn_key_hash(&nc->rev_key, ct->hash_basis); + if (nat_hash != ctx->hash) { + memcpy(&nat_conn->key, &nc->rev_key, sizeof nat_conn->key); + memcpy(&nat_conn->rev_key, &nc->key, sizeof nat_conn->rev_key); + nat_conn->conn_type = CT_CONN_TYPE_UN_NAT; + nat_conn->nat_action = 0; + nat_conn->alg = NULL; + nat_conn->nat_conn = NULL; + cmap_insert(&ct->conns, &nat_conn->cm_node, nat_hash); + } else { + free(nat_conn); + nat_conn = NULL; + } } nc->nat_conn = nat_conn; _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev