From patchwork Wed Mar 13 15:48:54 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Darrell Ball X-Patchwork-Id: 1056118 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=openvswitch.org (client-ip=140.211.169.12; helo=mail.linuxfoundation.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="WDAw87UY"; dkim-atps=neutral Received: from mail.linuxfoundation.org (mail.linuxfoundation.org [140.211.169.12]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 44KGW91Fxcz9s4Y for ; Thu, 14 Mar 2019 02:49:57 +1100 (AEDT) Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id BE95FCA0; Wed, 13 Mar 2019 15:49:12 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@mail.linuxfoundation.org Received: from smtp1.linuxfoundation.org (smtp1.linux-foundation.org [172.17.192.35]) by mail.linuxfoundation.org (Postfix) with ESMTPS id 3E0CFC6A for ; Wed, 13 Mar 2019 15:49:10 +0000 (UTC) X-Greylist: whitelisted by SQLgrey-1.7.6 Received: from mail-pg1-f196.google.com (mail-pg1-f196.google.com [209.85.215.196]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id A3781836 for ; Wed, 13 Mar 2019 15:49:09 +0000 (UTC) Received: by mail-pg1-f196.google.com with SMTP id h34so1788612pgh.11 for ; Wed, 13 Mar 2019 08:49:09 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:subject:date:message-id; bh=O5KQwJcP7DDzRCjLlEXEwbZNQQxu3phFYRLffKu/CyM=; b=WDAw87UYfLRObVfFmkAgSz9SnZnZDYd5+dugi0pYf2iLoSQsXje5raWyJzHJ4CDjSC igr+Zl/92q8lPpIEuZCJ9smVpMOrkJdwEVH/va5DFXPW/1HqkZIfJBigoeL+hoMkbNlL WMoMgLcBva+oJSlq9dPCxKZGr+3DDu8fJBvcYX2d/QyFFIvQ8/ZUsToL7GWfWBL0Rhb2 k9xQLUbPs1lhSOIyvJ4bTcbmBbYVuWjkBntL9PXE3nEY0pOIxbTrMYTt+RGCmHh2WGWW IhixJfU06kjzQ6FDcfioy2zeMiZiScyl9qmCQ/4GRY3gimMKs9RKKSIY4w1X5/7QlrQi hz2w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:subject:date:message-id; bh=O5KQwJcP7DDzRCjLlEXEwbZNQQxu3phFYRLffKu/CyM=; b=I2XtTgOe/oshQ6al9YSsnolWin3drpRZEqWDLB69bYYAKp93uJZ8IhJROzuhnvy5dS FIjM4p2IKYvT3SRZyPNSg2y1cxo4O8qSDT6FTBr54idn4xzMHG7mzrN4I/cHqx8NIDsM cilEBbCtF9X6G6YqPdZUZHJ2xReqSkT42P/lQgxGVM0seQJZWy333baPj3dB5gj/Evz6 yjoAPAGZXCw52jvVzU5eiZz6IiRZKB+/m6+zA29piomKaSS/Qdefc+d+qAGfd2yIT0BF G6WloEUBBufjZxFRLbpvu3KlwUSk6ZZZMrpt474BVqTziJGHhzDagNJ5yrnTCgFX2wV8 pkgQ== X-Gm-Message-State: APjAAAVZKZxFJKNVCDgP0FokLJCt1mZbUNiFx93LYWMuhcLsD0IwZul+ E9kDV6iJcz7lfvHdTZ+KI3s= X-Google-Smtp-Source: APXvYqxRhLndmojOzKI6njJ8hpalbLiXjurwkxd81SQskFaIw32YfBx6aLAyA/dVn7kE7rLmWlKYrA== X-Received: by 2002:a17:902:2f:: with SMTP id 44mr41186381pla.139.1552492147817; Wed, 13 Mar 2019 08:49:07 -0700 (PDT) Received: from ubuntu.localdomain (c-76-102-76-212.hsd1.ca.comcast.net. [76.102.76.212]) by smtp.gmail.com with ESMTPSA id i4sm23001061pfo.158.2019.03.13.08.49.05 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Wed, 13 Mar 2019 08:49:07 -0700 (PDT) From: Darrell Ball To: dlu998@gmail.com, dev@openvswitch.org Date: Wed, 13 Mar 2019 08:48:54 -0700 Message-Id: <1552492135-39574-1-git-send-email-dlu998@gmail.com> X-Mailer: git-send-email 1.9.1 X-Spam-Status: No, score=-1.7 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_ENVFROM_END_DIGIT,FREEMAIL_FROM, RCVD_IN_DNSWL_NONE autolearn=no version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on smtp1.linux-foundation.org Subject: [ovs-dev] [patch v2 1/2] conntrack: Fix race for NAT cleanup. X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: ovs-dev-bounces@openvswitch.org Errors-To: ovs-dev-bounces@openvswitch.org Reference lists are not fully protected during cleanup of NAT connections where the bucket lock is transiently not held during list traversal. This can lead to referencing freed memory during cleaning from multiple contexts. Fix this by protecting with the existing 'cleanup' mutex in the missed cases where 'conn_clean()' is called. 'conntrack_flush()' is converted to expiry list traversal to support the proper bucket level protection with the 'cleanup' mutex. Fixes: 286de2729955 ("dpdk: Userspace Datapath: Introduce NAT Support.") Reported-by: solomon Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2019-March/357056.html Tested-by: solomon Signed-off-by: Darrell Ball --- v2: Fix typo - if (OVS_LIKELY(force && ctx->reply && conn)) { + if (OVS_UNLIKELY(force && ctx->reply && conn)) { lib/conntrack.c | 96 +++++++++++++++++++++++++++++++++++++++++---------------- 1 file changed, 70 insertions(+), 26 deletions(-) diff --git a/lib/conntrack.c b/lib/conntrack.c index 691782c..ea50963 100644 --- a/lib/conntrack.c +++ b/lib/conntrack.c @@ -753,6 +753,24 @@ conn_lookup(struct conntrack *ct, const struct conn_key *key, long long now) return ctx.conn; } +/* This function is called with the bucket lock held. */ +static struct conn * +conn_lookup_any(const struct conn_key *key, + const struct conntrack_bucket *ctb, uint32_t hash) +{ + struct conn *conn = NULL; + + HMAP_FOR_EACH_WITH_HASH (conn, node, hash, &ctb->connections) { + if (!conn_key_cmp(&conn->key, key)) { + break; + } + if (!conn_key_cmp(&conn->rev_key, key)) { + 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) @@ -823,6 +841,20 @@ conn_clean(struct conntrack *ct, struct conn *conn, } } +static void +conn_clean_safe(struct conntrack *ct, struct conn *conn, + struct conntrack_bucket *ctb, uint32_t hash) +{ + ovs_mutex_lock(&ctb->cleanup_mutex); + ct_lock_lock(&ctb->lock); + conn = conn_lookup_any(&conn->key, ctb, hash); + if (conn) { + conn_clean(ct, conn, ctb); + } + ct_lock_unlock(&ctb->lock); + ovs_mutex_unlock(&ctb->cleanup_mutex); +} + static bool ct_verify_helper(const char *helper, enum ct_alg_ctl_type ct_alg_ctl) { @@ -969,6 +1001,7 @@ conn_update_state(struct conntrack *ct, struct dp_packet *pkt, OVS_REQUIRES(ct->buckets[bucket].lock) { bool create_new_conn = false; + struct conn lconn; if (ctx->icmp_related) { pkt->md.ct_state |= CS_RELATED; @@ -995,7 +1028,10 @@ conn_update_state(struct conntrack *ct, struct dp_packet *pkt, pkt->md.ct_state = CS_INVALID; break; case CT_UPDATE_NEW: - conn_clean(ct, *conn, &ct->buckets[bucket]); + memcpy(&lconn, *conn, sizeof lconn); + ct_lock_unlock(&ct->buckets[bucket].lock); + conn_clean_safe(ct, &lconn, &ct->buckets[bucket], ctx->hash); + ct_lock_lock(&ct->buckets[bucket].lock); create_new_conn = true; break; default: @@ -1184,8 +1220,12 @@ process_one(struct conntrack *ct, struct dp_packet *pkt, conn = ctx->conn; /* Delete found entry if in wrong direction. 'force' implies commit. */ - if (conn && force && ctx->reply) { - conn_clean(ct, conn, &ct->buckets[bucket]); + if (OVS_UNLIKELY(force && ctx->reply && conn)) { + struct conn lconn; + memcpy(&lconn, conn, sizeof lconn); + ct_lock_unlock(&ct->buckets[bucket].lock); + conn_clean_safe(ct, &lconn, &ct->buckets[bucket], ctx->hash); + ct_lock_lock(&ct->buckets[bucket].lock); conn = NULL; } @@ -1391,19 +1431,17 @@ sweep_bucket(struct conntrack *ct, struct conntrack_bucket *ctb, for (unsigned i = 0; i < N_CT_TM; i++) { LIST_FOR_EACH_SAFE (conn, next, exp_node, &ctb->exp_lists[i]) { - if (conn->conn_type == CT_CONN_TYPE_DEFAULT) { - if (!conn_expired(conn, now) || count >= limit) { - min_expiration = MIN(min_expiration, conn->expiration); - if (count >= limit) { - /* Do not check other lists. */ - COVERAGE_INC(conntrack_long_cleanup); - return min_expiration; - } - break; + if (!conn_expired(conn, now) || count >= limit) { + min_expiration = MIN(min_expiration, conn->expiration); + if (count >= limit) { + /* Do not check other lists. */ + COVERAGE_INC(conntrack_long_cleanup); + return min_expiration; } - conn_clean(ct, conn, ctb); - count++; + break; } + conn_clean(ct, conn, ctb); + count++; } } return min_expiration; @@ -2547,16 +2585,19 @@ int conntrack_flush(struct conntrack *ct, const uint16_t *zone) { for (unsigned i = 0; i < CONNTRACK_BUCKETS; i++) { - struct conn *conn, *next; - - ct_lock_lock(&ct->buckets[i].lock); - HMAP_FOR_EACH_SAFE (conn, next, node, &ct->buckets[i].connections) { - if ((!zone || *zone == conn->key.zone) && - (conn->conn_type == CT_CONN_TYPE_DEFAULT)) { - conn_clean(ct, conn, &ct->buckets[i]); + struct conntrack_bucket *ctb = &ct->buckets[i]; + ovs_mutex_lock(&ctb->cleanup_mutex); + ct_lock_lock(&ctb->lock); + for (unsigned j = 0; j < N_CT_TM; j++) { + struct conn *conn, *next; + LIST_FOR_EACH_SAFE (conn, next, exp_node, &ctb->exp_lists[j]) { + if (!zone || *zone == conn->key.zone) { + conn_clean(ct, conn, ctb); + } } } - ct_lock_unlock(&ct->buckets[i].lock); + ct_lock_unlock(&ctb->lock); + ovs_mutex_unlock(&ctb->cleanup_mutex); } return 0; @@ -2573,16 +2614,19 @@ conntrack_flush_tuple(struct conntrack *ct, const struct ct_dpif_tuple *tuple, tuple_to_conn_key(tuple, zone, &ctx.key); ctx.hash = conn_key_hash(&ctx.key, ct->hash_basis); unsigned bucket = hash_to_bucket(ctx.hash); + struct conntrack_bucket *ctb = &ct->buckets[bucket]; - ct_lock_lock(&ct->buckets[bucket].lock); - conn_key_lookup(&ct->buckets[bucket], &ctx, time_msec()); + ovs_mutex_lock(&ctb->cleanup_mutex); + ct_lock_lock(&ctb->lock); + conn_key_lookup(ctb, &ctx, time_msec()); if (ctx.conn && ctx.conn->conn_type == CT_CONN_TYPE_DEFAULT) { - conn_clean(ct, ctx.conn, &ct->buckets[bucket]); + conn_clean(ct, ctx.conn, ctb); } else { VLOG_WARN("Must flush tuple using the original pre-NATed tuple"); error = ENOENT; } - ct_lock_unlock(&ct->buckets[bucket].lock); + ct_lock_unlock(&ctb->lock); + ovs_mutex_unlock(&ctb->cleanup_mutex); return error; } From patchwork Wed Mar 13 15:48:55 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Darrell Ball X-Patchwork-Id: 1056115 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=openvswitch.org (client-ip=140.211.169.12; helo=mail.linuxfoundation.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="pggSvqIs"; dkim-atps=neutral Received: from mail.linuxfoundation.org (mail.linuxfoundation.org [140.211.169.12]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 44KGVM1Zsxz9s3q for ; Thu, 14 Mar 2019 02:49:13 +1100 (AEDT) Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id DB1C4B8A; Wed, 13 Mar 2019 15:49:09 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@mail.linuxfoundation.org Received: from smtp1.linuxfoundation.org (smtp1.linux-foundation.org [172.17.192.35]) by mail.linuxfoundation.org (Postfix) with ESMTPS id 61669A70 for ; Wed, 13 Mar 2019 15:49:09 +0000 (UTC) X-Greylist: whitelisted by SQLgrey-1.7.6 Received: from mail-pg1-f196.google.com (mail-pg1-f196.google.com [209.85.215.196]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id 22717836 for ; Wed, 13 Mar 2019 15:49:09 +0000 (UTC) Received: by mail-pg1-f196.google.com with SMTP id m2so1812572pgl.5 for ; Wed, 13 Mar 2019 08:49:09 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:subject:date:message-id:in-reply-to:references; bh=Tgp+i7XIrUH4S0CypzW20hhsXj5bW2ATM8F2e9tDIgg=; b=pggSvqIszjSJIxT2A2eIsYhNSXvysEsRYLWK4QwE9PRNmmUblKicIpa/QkfPFR9nVj uUOwkdXNnb8MvWkThz8L/HVPo9quPhSk8UHWOG4GuAoIbtTqx2puq380Rkq0jhd4nzSy cWzkjn6IjDJvI9XchqTkZVA844JgxFJpun4mMfVeXePZ4+wdykWcWBNW3O1TXsuPxWYc QVZwJUFxmQP5x/bOd/XL3Rzdam3TqkmosdRA3R2oWevEmju48odfJqMqyzollm8Da3bb 7xwV5uvmARSOKgd2YTTo4kZL0C2HMKFp4kK+grYf/FOZ7lanjTOCwq6OdPUfzQ5aqBq+ HZNA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:subject:date:message-id:in-reply-to :references; bh=Tgp+i7XIrUH4S0CypzW20hhsXj5bW2ATM8F2e9tDIgg=; b=SlMd2ptBq6TIkTBX3nZLNjYCp/q6d0EDvx3LDxcbPW8lJhNgNrgTbWjxL1v0YvyGGB zt2EPiZMP+1vX914vgt+eK2lxmslbJUMLaKp0OphvbyKEyzgbCgLtCC7H4Rmx+m6aNei 2qf1vsAeQSPfT/BvEjanPwJ4Mk+jaUanMhT3B8L1soAymSTH5pYcvh7RyzFt5qKqj3Pi GhDqKzJTyxOo4Vt7oakONw78/z63fjIkf38cK2XMDWVHtBln55Ffsmb1RQPfIKl7n0+V isTmLzMxCRx9FG90kM13MEnv2sBtaZ6RjTOSK74RecR8n1MKhcvmqExl5NMMgKbhAs5T Pziw== X-Gm-Message-State: APjAAAV2KRGhRAQmMkX4Ec83PLUOqfqYo3UUqq5o1QNmn7W7wpir7B/r BZALAZ4+Q+MwSVQZQZ6p0n8= X-Google-Smtp-Source: APXvYqxfZ8kInBf57KYfuFuZCPPQyuVjyqum8Crpw1jxlwiIKkFRJzBx8bdWgyME3+HDunsutnlhBw== X-Received: by 2002:a63:4e57:: with SMTP id o23mr38468442pgl.368.1552492148577; Wed, 13 Mar 2019 08:49:08 -0700 (PDT) Received: from ubuntu.localdomain (c-76-102-76-212.hsd1.ca.comcast.net. [76.102.76.212]) by smtp.gmail.com with ESMTPSA id i4sm23001061pfo.158.2019.03.13.08.49.07 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Wed, 13 Mar 2019 08:49:08 -0700 (PDT) From: Darrell Ball To: dlu998@gmail.com, dev@openvswitch.org Date: Wed, 13 Mar 2019 08:48:55 -0700 Message-Id: <1552492135-39574-2-git-send-email-dlu998@gmail.com> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1552492135-39574-1-git-send-email-dlu998@gmail.com> References: <1552492135-39574-1-git-send-email-dlu998@gmail.com> X-Spam-Status: No, score=-1.7 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_ENVFROM_END_DIGIT,FREEMAIL_FROM, RCVD_IN_DNSWL_NONE autolearn=no version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on smtp1.linux-foundation.org Subject: [ovs-dev] [patch v2 2/2] conntrack: Lookup only 'UNNAT conns' in 'nat_clean()'. X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: ovs-dev-bounces@openvswitch.org Errors-To: ovs-dev-bounces@openvswitch.org 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. Signed-off-by: Darrell Ball --- v2: No change to this patch. lib/conntrack.c | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/lib/conntrack.c b/lib/conntrack.c index ea50963..07ee242 100644 --- a/lib/conntrack.c +++ b/lib/conntrack.c @@ -771,6 +771,22 @@ conn_lookup_any(const struct conn_key *key, return conn; } +/* This function is called with the bucket lock held. */ +static struct conn * +conn_lookup_unnat(const struct conn_key *key, + const struct conntrack_bucket *ctb, uint32_t hash) +{ + 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) @@ -794,12 +810,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);