Skip to content

Commit a4e6a10

Browse files
Florian Westphalummakynes
authored andcommitted
netfilter: conntrack: add clash resolution for reverse collisions
Given existing entry: ORIGIN: a:b -> c:d REPLY: c:d -> a:b And colliding entry: ORIGIN: c:d -> a:b REPLY: a:b -> c:d The colliding ct (and the associated skb) get dropped on insert. Permit this by checking if the colliding entry matches the reply direction. Happens when both ends send packets at same time, both requests are picked up as NEW, rather than NEW for the 'first' and 'ESTABLISHED' for the second packet. This is an esoteric condition, as ruleset must permit NEW connections in either direction and both peers must already have a bidirectional traffic flow at the time conntrack gets enabled. Allow the 'reverse' skb to pass and assign the existing (clashing) entry. While at it, also drop the extra 'dying' check, this is already tested earlier by the calling function. Signed-off-by: Florian Westphal <[email protected]> Signed-off-by: Pablo Neira Ayuso <[email protected]>
1 parent d8f84a9 commit a4e6a10

File tree

1 file changed

+51
-5
lines changed

1 file changed

+51
-5
lines changed

net/netfilter/nf_conntrack_core.c

Lines changed: 51 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -988,6 +988,56 @@ static void __nf_conntrack_insert_prepare(struct nf_conn *ct)
988988
tstamp->start = ktime_get_real_ns();
989989
}
990990

991+
/**
992+
* nf_ct_match_reverse - check if ct1 and ct2 refer to identical flow
993+
* @ct1: conntrack in hash table to check against
994+
* @ct2: merge candidate
995+
*
996+
* returns true if ct1 and ct2 happen to refer to the same flow, but
997+
* in opposing directions, i.e.
998+
* ct1: a:b -> c:d
999+
* ct2: c:d -> a:b
1000+
* for both directions. If so, @ct2 should not have been created
1001+
* as the skb should have been picked up as ESTABLISHED flow.
1002+
* But ct1 was not yet committed to hash table before skb that created
1003+
* ct2 had arrived.
1004+
*
1005+
* Note we don't compare netns because ct entries in different net
1006+
* namespace cannot clash to begin with.
1007+
*
1008+
* @return: true if ct1 and ct2 are identical when swapping origin/reply.
1009+
*/
1010+
static bool
1011+
nf_ct_match_reverse(const struct nf_conn *ct1, const struct nf_conn *ct2)
1012+
{
1013+
u16 id1, id2;
1014+
1015+
if (!nf_ct_tuple_equal(&ct1->tuplehash[IP_CT_DIR_ORIGINAL].tuple,
1016+
&ct2->tuplehash[IP_CT_DIR_REPLY].tuple))
1017+
return false;
1018+
1019+
if (!nf_ct_tuple_equal(&ct1->tuplehash[IP_CT_DIR_REPLY].tuple,
1020+
&ct2->tuplehash[IP_CT_DIR_ORIGINAL].tuple))
1021+
return false;
1022+
1023+
id1 = nf_ct_zone_id(nf_ct_zone(ct1), IP_CT_DIR_ORIGINAL);
1024+
id2 = nf_ct_zone_id(nf_ct_zone(ct2), IP_CT_DIR_REPLY);
1025+
if (id1 != id2)
1026+
return false;
1027+
1028+
id1 = nf_ct_zone_id(nf_ct_zone(ct1), IP_CT_DIR_REPLY);
1029+
id2 = nf_ct_zone_id(nf_ct_zone(ct2), IP_CT_DIR_ORIGINAL);
1030+
1031+
return id1 == id2;
1032+
}
1033+
1034+
static int nf_ct_can_merge(const struct nf_conn *ct,
1035+
const struct nf_conn *loser_ct)
1036+
{
1037+
return nf_ct_match(ct, loser_ct) ||
1038+
nf_ct_match_reverse(ct, loser_ct);
1039+
}
1040+
9911041
/* caller must hold locks to prevent concurrent changes */
9921042
static int __nf_ct_resolve_clash(struct sk_buff *skb,
9931043
struct nf_conntrack_tuple_hash *h)
@@ -999,11 +1049,7 @@ static int __nf_ct_resolve_clash(struct sk_buff *skb,
9991049

10001050
loser_ct = nf_ct_get(skb, &ctinfo);
10011051

1002-
if (nf_ct_is_dying(ct))
1003-
return NF_DROP;
1004-
1005-
if (((ct->status & IPS_NAT_DONE_MASK) == 0) ||
1006-
nf_ct_match(ct, loser_ct)) {
1052+
if (nf_ct_can_merge(ct, loser_ct)) {
10071053
struct net *net = nf_ct_net(ct);
10081054

10091055
nf_conntrack_get(&ct->ct_general);

0 commit comments

Comments
 (0)