diff options
author | Darrell Ball <dlu998@gmail.com> | 2017-09-25 20:51:44 -0700 |
---|---|---|
committer | Ben Pfaff <blp@ovn.org> | 2017-09-26 15:54:54 -0700 |
commit | d8682ee522b06326d05fce5f86fa6332be60d115 (patch) | |
tree | 8235af1cb16fbf399939f08ec9f4e1d1ab17fcf3 /lib | |
parent | 66f400f59b6efa7dfd40a69e22a6ca1021e56a3d (diff) | |
download | openvswitch-d8682ee522b06326d05fce5f86fa6332be60d115.tar.gz |
conntrack: Tighten handling of alg reverse conns.
Close a theoretical race delete/create corner case for alg
reverse conns and add debugging around this that may point to
an intentional exploit, unintentional problem or just a rare
condition. The solution is to keep track of reverse conn via
nat_conn_keys and avoid deleting the reverse conn when it has been
recreated.
Fixes: bd5e81a0e596 ("Userspace Datapath: Add ALG infra and FTP.")
Signed-off-by: Darrell Ball <dlu998@gmail.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
Diffstat (limited to 'lib')
-rw-r--r-- | lib/conntrack.c | 34 |
1 files changed, 28 insertions, 6 deletions
diff --git a/lib/conntrack.c b/lib/conntrack.c index 3c9f34400..406d7a553 100644 --- a/lib/conntrack.c +++ b/lib/conntrack.c @@ -67,9 +67,6 @@ enum ct_alg_mode { CT_TFTP_MODE, }; -void ct_print_conn_info(struct conn *c, char *log_msg, enum vlog_level vll, - bool force, bool rl_on); - static bool conn_key_extract(struct conntrack *, struct dp_packet *, ovs_be16 dl_type, struct conn_lookup_ctx *, uint16_t zone); @@ -226,7 +223,7 @@ conn_key_cmp(const struct conn_key *key1, const struct conn_key *key2) return 1; } -void +static void ct_print_conn_info(struct conn *c, char *log_msg, enum vlog_level vll, bool force, bool rl_on) { @@ -835,6 +832,18 @@ conn_not_found(struct conntrack *ct, struct dp_packet *pkt, nc->nat_info->nat_action = NAT_ACTION_DST; } *conn_for_un_nat_copy = *nc; + ct_rwlock_wrlock(&ct->resources_lock); + bool new_insert = nat_conn_keys_insert(&ct->nat_conn_keys, + conn_for_un_nat_copy, + ct->hash_basis); + ct_rwlock_unlock(&ct->resources_lock); + if (!new_insert) { + char *log_msg = xasprintf("Pre-existing alg " + "nat_conn_key"); + ct_print_conn_info(conn_for_un_nat_copy, log_msg, VLL_INFO, + true, false); + free(log_msg); + } } else { *conn_for_un_nat_copy = *nc; ct_rwlock_wrlock(&ct->resources_lock); @@ -936,8 +945,16 @@ create_un_nat_conn(struct conntrack *ct, struct conn *conn_for_un_nat_copy, struct conn *rev_conn = conn_lookup(ct, &nc->key, now); if (alg_un_nat) { - hmap_insert(&ct->buckets[un_nat_conn_bucket].connections, - &nc->node, un_nat_hash); + if (!rev_conn) { + hmap_insert(&ct->buckets[un_nat_conn_bucket].connections, + &nc->node, un_nat_hash); + } else { + char *log_msg = xasprintf("Unusual condition for un_nat conn " + "create for alg: rev_conn %p", rev_conn); + ct_print_conn_info(nc, log_msg, VLL_INFO, true, false); + free(log_msg); + free(nc); + } } else { ct_rwlock_rdlock(&ct->resources_lock); @@ -949,6 +966,11 @@ create_un_nat_conn(struct conntrack *ct, struct conn *conn_for_un_nat_copy, hmap_insert(&ct->buckets[un_nat_conn_bucket].connections, &nc->node, un_nat_hash); } else { + char *log_msg = xasprintf("Unusual condition for un_nat conn " + "create: nat_conn_key_node/rev_conn " + "%p/%p", nat_conn_key_node, rev_conn); + ct_print_conn_info(nc, log_msg, VLL_INFO, true, false); + free(log_msg); free(nc); } ct_rwlock_unlock(&ct->resources_lock); |