[PATCH nf,v3 4/4] netfilter: flowtable: let GC handle partially set up flow

If the reply tuple cannot be inserted into the rhashtable, let GC remove
the tuple in the original direction. Otherwise, the flowtable lockless
rcu iterator might be already walking over such flow entry and access
different recycled ct due to the typesafe rcu semantics.

The rhashtable_remove_fast() call in flow_offload_del() will just report
ENOENT for the reply tuple.

Let the GC clear the offload bit when setting up the

While at it, remove WARN_ON_ONCE() which is reachable in case of OOM
in tc act_ct.

Fixes: ac2a66665e23 ("netfilter: add generic flow table infrastructure")
Signed-off-by: Pablo Neira Ayuso <[email protected]>
---
v3: new in this series

 net/netfilter/nf_flow_table_core.c |  8 +++++---
 net/netfilter/nft_flow_offload.c   |  8 +-------
 net/sched/act_ct.c                 | 12 ++----------
 3 files changed, 8 insertions(+), 20 deletions(-)

diff --git a/net/netfilter/nf_flow_table_core.c b/net/netfilter/nf_flow_table_core.c
index 8c84f02c4dd3..27cbabc092d0 100644
--- a/net/netfilter/nf_flow_table_core.c
+++ b/net/netfilter/nf_flow_table_core.c
@@ -341,9 +341,11 @@ int flow_offload_add(struct nf_flowtable *flow_table, struct flow_offload *flow)
 				     &flow->tuplehash[FLOW_OFFLOAD_DIR_REPLY].node,
 				     nf_flow_offload_rhash_params);
 	if (err < 0) {
-		rhashtable_remove_fast(&flow_table->rhashtable,
-				       &flow->tuplehash[FLOW_OFFLOAD_DIR_ORIGINAL].node,
-				       nf_flow_offload_rhash_params);
+		/* Let GC handle this flow in partial state, confirm it so it
+		 * is handled as an expired flow. The rhashtable_remove_fast()
+		 * call will just report ENOENT for the reply tuple.
+		 */
+		set_bit(NF_FLOW_CONFIRMED, &flow->flags);
 		return err;
 	}
 
diff --git a/net/netfilter/nft_flow_offload.c b/net/netfilter/nft_flow_offload.c
index 32b4281038dd..54671006ce98 100644
--- a/net/netfilter/nft_flow_offload.c
+++ b/net/netfilter/nft_flow_offload.c
@@ -59,7 +59,6 @@ static void nft_flow_offload_eval(const struct nft_expr *expr,
 	struct flow_offload *flow;
 	enum ip_conntrack_dir dir;
 	struct nf_conn *ct;
-	int ret;
 
 	if (nft_flow_offload_skip(pkt->skb, nft_pf(pkt)))
 		goto out;
@@ -118,14 +117,9 @@ static void nft_flow_offload_eval(const struct nft_expr *expr,
 		flow_offload_ct_tcp(ct);
 
 	__set_bit(NF_FLOW_HW_BIDIRECTIONAL, &flow->flags);
-	ret = flow_offload_add(flowtable, flow);
-	if (ret < 0)
-		goto err_flow_add;
-
+	flow_offload_add(flowtable, flow);
 	return;
 
-err_flow_add:
-	flow_offload_free(flow);
 err_flow_alloc:
 	dst_release(route.tuple[dir].dst);
 	dst_release(route.tuple[!dir].dst);
diff --git a/net/sched/act_ct.c b/net/sched/act_ct.c
index e250969c84ac..31df7f9bce86 100644
--- a/net/sched/act_ct.c
+++ b/net/sched/act_ct.c
@@ -427,16 +427,13 @@ static void tcf_ct_flow_table_add(struct tcf_ct_flow_table *ct_ft,
 {
 	struct nf_conn_act_ct_ext *act_ct_ext;
 	struct flow_offload *entry;
-	int err;
 
 	if (test_and_set_bit(IPS_OFFLOAD_BIT, &ct->status))
 		return;
 
 	entry = flow_offload_alloc(ct);
-	if (!entry) {
-		WARN_ON_ONCE(1);
+	if (!entry)
 		goto err_alloc;
-	}
 
 	if (tcp) {
 		ct->proto.tcp.seen[0].flags |= IP_CT_TCP_FLAG_BE_LIBERAL;
@@ -451,14 +448,9 @@ static void tcf_ct_flow_table_add(struct tcf_ct_flow_table *ct_ft,
 		tcf_ct_flow_tc_ifidx(entry, act_ct_ext, FLOW_OFFLOAD_DIR_REPLY);
 	}
 
-	err = flow_offload_add(&ct_ft->nf_ft, entry);
-	if (err)
-		goto err_add;
-
+	flow_offload_add(&ct_ft->nf_ft, entry);
 	return;
 
-err_add:
-	flow_offload_free(entry);
 err_alloc:
 	clear_bit(IPS_OFFLOAD_BIT, &ct->status);
 }
-- 
2.47.3