diff --git a/include/net/netfilter/nf_conntrack.h b/include/net/netfilter/nf_conntrack.h index bc42dd0e10e6..c39425e54d87 100644 --- a/include/net/netfilter/nf_conntrack.h +++ b/include/net/netfilter/nf_conntrack.h @@ -185,6 +185,11 @@ static inline void nf_ct_put(struct nf_conn *ct) nf_ct_destroy(&ct->ct_general); } +static inline bool nf_ct_shared(const struct nf_conn *ct) +{ + return refcount_read(&ct->ct_general.use) > 1; +} + /* load module; enable/disable conntrack in this namespace */ int nf_ct_netns_get(struct net *net, u8 nfproto); void nf_ct_netns_put(struct net *net, u8 nfproto); diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c index 0f433688e17b..d3326edcabf7 100644 --- a/net/openvswitch/conntrack.c +++ b/net/openvswitch/conntrack.c @@ -734,6 +734,18 @@ static int __ovs_ct_lookup(struct net *net, struct sw_flow_key *key, enum ip_conntrack_info ctinfo; struct nf_conn *ct; + /* If the ct entry is not confirmed and shared with some other skb, + * e.g., a cloned one, we can't just modify it with the commit as we + * must not modify the extension set. Reset. + */ + if (cached && info->commit) { + ct = nf_ct_get(skb, &ctinfo); + if (ct && !nf_ct_is_confirmed(ct) && nf_ct_shared(ct)) { + nf_reset_ct(skb); + cached = false; + } + } + if (!cached) { struct nf_hook_state state = { .hook = NF_INET_PRE_ROUTING, @@ -766,8 +778,6 @@ static int __ovs_ct_lookup(struct net *net, struct sw_flow_key *key, ct = nf_ct_get(skb, &ctinfo); if (ct) { - bool add_helper = false; - /* Packets starting a new connection must be NATted before the * helper, so that the helper knows about the NAT. We enforce * this by delaying both NAT and helper calls for unconfirmed @@ -799,7 +809,6 @@ static int __ovs_ct_lookup(struct net *net, struct sw_flow_key *key, GFP_ATOMIC); if (err) return err; - add_helper = true; /* helper installed, add seqadj if NAT is required */ if (info->nat && !nfct_seqadj(ct)) { @@ -808,14 +817,14 @@ static int __ovs_ct_lookup(struct net *net, struct sw_flow_key *key, } } - /* Call the helper only if: - * - nf_conntrack_in() was executed above ("!cached") or a - * helper was just attached ("add_helper") for a confirmed - * connection, or - * - When committing an unconfirmed connection. + /* Call the helper only if nf_conntrack_in() was executed + * above ("!cached"). + * + * For unconfirmed connections it will be called later during + * commit as we need to have all the other extensions allocated + * before the call. */ - if ((nf_ct_is_confirmed(ct) ? !cached || add_helper : - info->commit)) { + if (nf_ct_is_confirmed(ct) && !cached) { int err = nf_ct_helper(skb, ct, ctinfo, info->family); err = verdict_to_errno(err); @@ -1019,6 +1028,14 @@ static int ovs_ct_commit(struct net *net, struct sw_flow_key *key, return err; nf_conn_act_ct_ext_add(skb, ct, ctinfo); + + /* Call the helpers now. We couldn't do this before as + * all the extensions must be allocated before the call. + */ + err = nf_ct_helper(skb, ct, ctinfo, info->family); + err = verdict_to_errno(err); + if (err) + return err; } else if (IS_ENABLED(CONFIG_NF_CONNTRACK_LABELS) && labels_nonzero(&info->labels.mask)) { err = ovs_ct_set_labels(ct, key, &info->labels.value, diff --git a/net/sched/act_ct.c b/net/sched/act_ct.c index 55f3521edb4c..411e3dd92d07 100644 --- a/net/sched/act_ct.c +++ b/net/sched/act_ct.c @@ -979,14 +979,13 @@ TC_INDIRECT_SCOPE int tcf_ct_act(struct sk_buff *skb, const struct tc_action *a, struct tcf_result *res) { struct net *net = dev_net(skb->dev); + bool cached, commit, clear, nat; enum ip_conntrack_info ctinfo; struct tcf_ct *c = to_ct(a); struct nf_conn *tmpl = NULL; struct nf_hook_state state; - bool cached, commit, clear; int nh_ofs, err, retval; struct tcf_ct_params *p; - bool add_helper = false; bool skb_is_ours = false; bool skip_add = false; bool defrag = false; @@ -998,6 +997,7 @@ TC_INDIRECT_SCOPE int tcf_ct_act(struct sk_buff *skb, const struct tc_action *a, retval = p->action; commit = p->ct_action & TCA_CT_ACT_COMMIT; clear = p->ct_action & TCA_CT_ACT_CLEAR; + nat = p->ct_action & TCA_CT_ACT_NAT; tmpl = p->tmpl; tcf_lastuse_update(&c->tcf_tm); @@ -1046,6 +1046,19 @@ TC_INDIRECT_SCOPE int tcf_ct_act(struct sk_buff *skb, const struct tc_action *a, * different zone. */ cached = tcf_ct_skb_nfct_cached(net, skb, p); + + /* If the ct entry is not confirmed and shared with some other skb, + * e.g., a cloned one, we can't just modify it with a commit or nat + * as we must not modify the extension set. Reset. + */ + if (cached && (commit || nat)) { + ct = nf_ct_get(skb, &ctinfo); + if (ct && !nf_ct_is_confirmed(ct) && nf_ct_shared(ct)) { + nf_reset_ct(skb); + cached = false; + } + } + if (!cached) { if (tcf_ct_flow_table_lookup(p, skb, family)) { skip_add = true; @@ -1082,26 +1095,32 @@ do_nat: err = __nf_ct_try_assign_helper(ct, p->tmpl, GFP_ATOMIC); if (err) goto drop; - add_helper = true; - if (p->ct_action & TCA_CT_ACT_NAT && !nfct_seqadj(ct)) { + + if (nat && !nfct_seqadj(ct)) { if (!nfct_seqadj_ext_add(ct)) goto drop; } } - if (nf_ct_is_confirmed(ct) ? ((!cached && !skip_add) || add_helper) : commit) { - err = nf_ct_helper(skb, ct, ctinfo, family); - if (err != NF_ACCEPT) - goto nf_error; - } - if (commit) { tcf_ct_act_set_mark(ct, p->mark, p->mark_mask); tcf_ct_act_set_labels(ct, p->labels, p->labels_mask); if (!nf_ct_is_confirmed(ct)) nf_conn_act_ct_ext_add(skb, ct, ctinfo); + } + /* Run helpers for the connection if nf_conntrack_in() was executed + * or if we're about to commit. This has to be done after all the + * extensions are already added. + */ + if (nf_ct_is_confirmed(ct) ? (!cached && !skip_add) : commit) { + err = nf_ct_helper(skb, ct, ctinfo, family); + if (err != NF_ACCEPT) + goto nf_error; + } + + if (commit) { /* This will take care of sending queued events * even if the connection is already confirmed. */