[ovs-dev] [PATCH v2] conntrack: Reset ct_state when entering a new zone.

Dumitru Ceara dceara at redhat.com
Thu Mar 5 11:28:55 UTC 2020

When a new conntrack zone is entered, the ct_state field is zeroed in
order to avoid using state information from different zones.

One such scenario is when a packet is double NATed. Assuming two zones
and 3 flows performing the following actions in order on the packet:
1. ct(zone=5,nat), recirc
2. ct(zone=1), recirc
3. ct(zone=1,nat)

If at step #1 the packet matches an existing NAT entry, it will get
translated and pkt->md.ct_state is set to CS_DST_NAT or CS_SRC_NAT.
At step #2 the new tuple might match an existing connection and
pkt->md.ct_zone is set to 1.
If at step #3 the packet matches an existing NAT entry in zone 1,
handle_nat() will be called to perform the translation but it will
return early because the packet's zone matches the conntrack zone and
the ct_state field still contains CS_DST_NAT or CS_SRC_NAT from the
translations in zone 5.

In order to reliably detect when a packet enters a new conntrack zone
we also need to make sure that the pkt->md.ct_zone is properly
initialized if pkt->md.ct_state is non-zero. This already happens for
most cases. The only exception is when matched conntrack connection is
of type CT_CONN_TYPE_UN_NAT and the master connection is missing. To
cover this path we now call write_ct_md() in that case too. Remove
setting the CS_TRACKED flag as in this case as it will be done by the
new call to write_ct_md().

Also, the error path above seems hard to hit as it would've caused a
crash due to dereferencing a NULL pointer when calling:
'ct_print_conn_info(conn, log_msg, VLL_INFO, true, true)'. This patch
changes the call to log the 'rev_conn' info instead.

CC: Darrell Ball <dlu998 at gmail.com>
Fixes: 967bb5c5cd90 ("conntrack: Add rcu support.")
Signed-off-by: Dumitru Ceara <dceara at redhat.com>

- Address Ilya's comments:
    - revert changes to pkt_metadata_init().
    - update ct_state in process_one() only if ct_state is already
- Make sure pkt->md.ct_zone is always initialized when pkt->md.ct_state
  is non-zero.
- Fix NULL pointer dereference in process_one() if conn_type is
  CT_CONN_TYPE_UN_NAT and master conn is not found.
 lib/conntrack.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/lib/conntrack.c b/lib/conntrack.c
index ff5a894..0cbc8f6 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -1277,6 +1277,11 @@ process_one(struct conntrack *ct, struct dp_packet *pkt,
             const struct nat_action_info_t *nat_action_info,
             ovs_be16 tp_src, ovs_be16 tp_dst, const char *helper)
+    /* Reset ct_state whenever entering a new zone. */
+    if (pkt->md.ct_state && pkt->md.ct_zone != zone) {
+        pkt->md.ct_state = 0;
+    }
     bool create_new_conn = false;
     conn_key_lookup(ct, &ctx->key, ctx->hash, now, &ctx->conn, &ctx->reply);
     struct conn *conn = ctx->conn;
@@ -1300,9 +1305,10 @@ process_one(struct conntrack *ct, struct dp_packet *pkt,
             conn_key_lookup(ct, &ctx->key, hash, now, &conn, &ctx->reply);
             if (!conn) {
-                pkt->md.ct_state |= CS_TRACKED | CS_INVALID;
+                pkt->md.ct_state |= CS_INVALID;
+                write_ct_md(pkt, zone, NULL, NULL, NULL);
                 char *log_msg = xasprintf("Missing master conn %p", rev_conn);
-                ct_print_conn_info(conn, log_msg, VLL_INFO, true, true);
+                ct_print_conn_info(rev_conn, log_msg, VLL_INFO, true, true);

More information about the dev mailing list