netfilter-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Pablo Neira Ayuso <pablo@netfilter.org>
To: netfilter-devel@vger.kernel.org
Cc: davem@davemloft.net, netdev@vger.kernel.org
Subject: [PATCH 02/16] openvswitch: Delete conntrack entry clashing with an expectation.
Date: Wed,  3 May 2017 11:31:57 +0200	[thread overview]
Message-ID: <1493803931-2837-3-git-send-email-pablo@netfilter.org> (raw)
In-Reply-To: <1493803931-2837-1-git-send-email-pablo@netfilter.org>

From: Jarno Rajahalme <jarno@ovn.org>

Conntrack helpers do not check for a potentially clashing conntrack
entry when creating a new expectation.  Also, nf_conntrack_in() will
check expectations (via init_conntrack()) only if a conntrack entry
can not be found.  The expectation for a packet which also matches an
existing conntrack entry will not be removed by conntrack, and is
currently handled inconsistently by OVS, as OVS expects the
expectation to be removed when the connection tracking entry matching
that expectation is confirmed.

It should be noted that normally an IP stack would not allow reuse of
a 5-tuple of an old (possibly lingering) connection for a new data
connection, so this is somewhat unlikely corner case.  However, it is
possible that a misbehaving source could cause conntrack entries be
created that could then interfere with new related connections.

Fix this in the OVS module by deleting the clashing conntrack entry
after an expectation has been matched.  This causes the following
nf_conntrack_in() call also find the expectation and remove it when
creating the new conntrack entry, as well as the forthcoming reply
direction packets to match the new related connection instead of the
old clashing conntrack entry.

Fixes: 7f8a436eaa2c ("openvswitch: Add conntrack action")
Reported-by: Yang Song <yangsong@vmware.com>
Signed-off-by: Jarno Rajahalme <jarno@ovn.org>
Acked-by: Joe Stringer <joe@ovn.org>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/openvswitch/conntrack.c | 30 +++++++++++++++++++++++++++++-
 1 file changed, 29 insertions(+), 1 deletion(-)

diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c
index 7b2c2fce408a..c92a5795dcda 100644
--- a/net/openvswitch/conntrack.c
+++ b/net/openvswitch/conntrack.c
@@ -514,10 +514,38 @@ ovs_ct_expect_find(struct net *net, const struct nf_conntrack_zone *zone,
 		   u16 proto, const struct sk_buff *skb)
 {
 	struct nf_conntrack_tuple tuple;
+	struct nf_conntrack_expect *exp;
 
 	if (!nf_ct_get_tuplepr(skb, skb_network_offset(skb), proto, net, &tuple))
 		return NULL;
-	return __nf_ct_expect_find(net, zone, &tuple);
+
+	exp = __nf_ct_expect_find(net, zone, &tuple);
+	if (exp) {
+		struct nf_conntrack_tuple_hash *h;
+
+		/* Delete existing conntrack entry, if it clashes with the
+		 * expectation.  This can happen since conntrack ALGs do not
+		 * check for clashes between (new) expectations and existing
+		 * conntrack entries.  nf_conntrack_in() will check the
+		 * expectations only if a conntrack entry can not be found,
+		 * which can lead to OVS finding the expectation (here) in the
+		 * init direction, but which will not be removed by the
+		 * nf_conntrack_in() call, if a matching conntrack entry is
+		 * found instead.  In this case all init direction packets
+		 * would be reported as new related packets, while reply
+		 * direction packets would be reported as un-related
+		 * established packets.
+		 */
+		h = nf_conntrack_find_get(net, zone, &tuple);
+		if (h) {
+			struct nf_conn *ct = nf_ct_tuplehash_to_ctrack(h);
+
+			nf_ct_delete(ct, 0, 0);
+			nf_conntrack_put(&ct->ct_general);
+		}
+	}
+
+	return exp;
 }
 
 /* This replicates logic from nf_conntrack_core.c that is not exported. */
-- 
2.1.4


  parent reply	other threads:[~2017-05-03  9:32 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-05-03  9:31 [PATCH 00/16] Netfilter/IPVS/OVS fixes for net Pablo Neira Ayuso
2017-05-03  9:31 ` [PATCH 01/16] netfilter: xt_CT: fix refcnt leak on error path Pablo Neira Ayuso
2017-05-03  9:31 ` Pablo Neira Ayuso [this message]
2017-05-03  9:31 ` [PATCH 03/16] netfilter: nf_ct_helper: permit cthelpers with different names via nfnetlink Pablo Neira Ayuso
2017-05-03  9:31 ` [PATCH 04/16] netfilter: nft_set_bitmap: free dummy elements when destroy the set Pablo Neira Ayuso
2017-05-03  9:32 ` [PATCH 05/16] netfilter: ctnetlink: drop the incorrect cthelper module request Pablo Neira Ayuso
2017-05-03  9:32 ` [PATCH 06/16] netfilter: ctnetlink: fix deadlock due to acquire _expect_lock twice Pablo Neira Ayuso
2017-05-03  9:32 ` [PATCH 07/16] netfilter: ctnetlink: make it safer when updating ct->status Pablo Neira Ayuso
2017-05-03  9:32 ` [PATCH 08/16] netfilter: ctnetlink: acquire ct->lock before operating nf_ct_seqadj Pablo Neira Ayuso
2017-05-03  9:32 ` [PATCH 09/16] netfilter: xt_socket: Fix broken IPv6 handling Pablo Neira Ayuso
2017-05-03  9:32 ` [PATCH 10/16] bridge: ebtables: fix reception of frames DNAT-ed to bridge device/port Pablo Neira Ayuso
2017-05-03  9:32 ` [PATCH 11/16] netfilter: nft_dynset: continue to next expr if _OP_ADD succeeded Pablo Neira Ayuso
2017-05-03  9:32 ` [PATCH 12/16] netfilter: Wrong icmp6 checksum for ICMPV6_TIME_EXCEED in reverse SNATv6 path Pablo Neira Ayuso
2017-05-03  9:32 ` [PATCH 13/16] ipvs: explicitly forbid ipv6 service/dest creation if ipv6 mod is disabled Pablo Neira Ayuso
2017-05-03  9:32 ` [PATCH 14/16] netfilter: x_tables: unlock on error in xt_find_table_lock() Pablo Neira Ayuso
2017-05-03  9:32 ` [PATCH 15/16] netfilter: update MAINTAINERS file Pablo Neira Ayuso
2017-05-03  9:32 ` [PATCH 16/16] netfilter: nf_tables: check if same extensions are set when adding elements Pablo Neira Ayuso
2017-05-03 14:11 ` [PATCH 00/16] Netfilter/IPVS/OVS fixes for net David Miller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1493803931-2837-3-git-send-email-pablo@netfilter.org \
    --to=pablo@netfilter.org \
    --cc=davem@davemloft.net \
    --cc=netdev@vger.kernel.org \
    --cc=netfilter-devel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).