diff mbox series

[ovs-dev,v6,2/3] conntrack: Lookup only 'UNNAT conns' in 'nat_clean()'.

Message ID 1552687280-40867-2-git-send-email-dlu998@gmail.com
State Accepted
Commit 901a0dad38207d2ddae14cda6fe0b6d1bb126c8d
Headers show
Series [ovs-dev,v6,1/3] conntrack: Fix race for NAT cleanup. | expand

Commit Message

Darrell Ball March 15, 2019, 10:01 p.m. UTC
When freeing 'UNNAT conns', lookup only 'UNNAT conns' to
protect against possible address overlap with 'default
conns' during a DOS attempt.  This is very unlikely, but
protection is simple.

Fixes: 286de2729955 ("dpdk: Userspace Datapath: Introduce NAT Support.")
Signed-off-by: Darrell Ball <dlu998@gmail.com>
---

This patch is targeted for earlier releases as new RCU patches
inherently don't have this race.

Backport to 2.8.

v6: Changed comment to lock annotation.
v5: Add fixes tag.
v1->v4: No changes to this patch.

 lib/conntrack.c | 25 +++++++++++++++++++++----
 1 file changed, 21 insertions(+), 4 deletions(-)

Comments

Ben Pfaff March 15, 2019, 10:58 p.m. UTC | #1
On Fri, Mar 15, 2019 at 03:01:19PM -0700, Darrell Ball wrote:
> When freeing 'UNNAT conns', lookup only 'UNNAT conns' to
> protect against possible address overlap with 'default
> conns' during a DOS attempt.  This is very unlikely, but
> protection is simple.
> 
> Fixes: 286de2729955 ("dpdk: Userspace Datapath: Introduce NAT Support.")
> Signed-off-by: Darrell Ball <dlu998@gmail.com>
> ---
> 
> This patch is targeted for earlier releases as new RCU patches
> inherently don't have this race.
> 
> Backport to 2.8.

Applied to master, 2.11, 2.10.
diff mbox series

Patch

diff --git a/lib/conntrack.c b/lib/conntrack.c
index dd6e19b..5235690 100644
--- a/lib/conntrack.c
+++ b/lib/conntrack.c
@@ -776,6 +776,22 @@  conn_lookup_def(const struct conn_key *key,
     return conn;
 }
 
+static struct conn *
+conn_lookup_unnat(const struct conn_key *key,
+                  const struct conntrack_bucket *ctb, uint32_t hash)
+    OVS_REQUIRES(ctb->lock)
+{
+    struct conn *conn = NULL;
+
+    HMAP_FOR_EACH_WITH_HASH (conn, node, hash, &ctb->connections) {
+        if (!conn_key_cmp(&conn->key, key)
+            && conn->conn_type == CT_CONN_TYPE_UN_NAT) {
+            break;
+        }
+    }
+    return conn;
+}
+
 static void
 conn_seq_skew_set(struct conntrack *ct, const struct conn_key *key,
                   long long now, int seq_skew, bool seq_skew_dir)
@@ -799,12 +815,13 @@  nat_clean(struct conntrack *ct, struct conn *conn,
     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));
+    uint32_t hash = conn_key_hash(&conn->rev_key, ct->hash_basis);
+    unsigned bucket_rev_conn = hash_to_bucket(hash);
     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_unnat(&conn->rev_key,
+                                              &ct->buckets[bucket_rev_conn],
+                                              hash);
     struct nat_conn_key_node *nat_conn_key_node =
         nat_conn_keys_lookup(&ct->nat_conn_keys, &conn->rev_key,
                              ct->hash_basis);