[ovs-dev,v2,4/6] conntrack: Tighten nat_clean.

Message ID 1536124490-86810-4-git-send-email-dlu998@gmail.com
State New
Headers show
Series
  • [ovs-dev,v2,1/6] conntrack: Handle self nat case.
Related show

Commit Message

Darrell Ball Sept. 5, 2018, 5:14 a.m.
nat_clean has a defunct optimization for calculating a hash outside the
scope of a bucket lock which potentially 'might' lead to referencing freed
memory.  There is little point to this optimization, so just remove it.
Needs backporting to 2.8.

Signed-off-by: Darrell Ball <dlu998@gmail.com>
---
 lib/conntrack.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

Patch

diff --git a/lib/conntrack.c b/lib/conntrack.c
index 15e0a62..065c337 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -778,20 +778,22 @@  nat_clean(struct conntrack *ct, struct conn *conn,
 {
     ct_rwlock_wrlock(&ct->resources_lock);
     nat_conn_keys_remove(&ct->nat_conn_keys, &conn->rev_key, ct->hash_basis);
-    ct_rwlock_unlock(&ct->resources_lock);
-    ct_lock_unlock(&ctb->lock);
     unsigned bucket_rev_conn =
         hash_to_bucket(conn_key_hash(&conn->rev_key, ct->hash_basis));
+    struct conn_key rev_key = conn->rev_key;
+    ct_rwlock_unlock(&ct->resources_lock);
+    ct_lock_unlock(&ctb->lock);
+
     ct_lock_lock(&ct->buckets[bucket_rev_conn].lock);
     ct_rwlock_wrlock(&ct->resources_lock);
     long long now = time_msec();
-    struct conn *rev_conn = conn_lookup(ct, &conn->rev_key, now);
+    struct conn *rev_conn = conn_lookup(ct, &rev_key, now);
     struct nat_conn_key_node *nat_conn_key_node =
-        nat_conn_keys_lookup(&ct->nat_conn_keys, &conn->rev_key,
+        nat_conn_keys_lookup(&ct->nat_conn_keys, &rev_key,
                              ct->hash_basis);
 
-    /* In the unlikely event, rev conn was recreated, then skip
-     * rev_conn cleanup. */
+    /* In the unlikely event, 'rev_conn' was recreated, then skip
+     * 'rev_conn' cleanup. */
     if (rev_conn && (!nat_conn_key_node ||
                      conn_key_cmp(&nat_conn_key_node->value,
                                   &rev_conn->rev_key))) {