[PATCH nf,v3 2/4] netfilter: flowtable: do not expose expired flows via lookup function

The lookup function currently relies on the teardown bit to skip flow
entries that will be removed soon by the GC. However, datapath can still
reach a flow that has expired without the teardown bit set on. This
includes a flow that is being set up at this time whose timeout is still
not fully initially.

Although the new NF_FLOW_CONFIRMED bit could be used to skip this flow
entry in lookups, skipping a expired flow in the lookup is convenient.
While it is safe to access such expired flow entry from datapath because
of the rcu semantics, the flow->ct pointer for a released flow via rcu
might point to a different conntrack object due to the typesafe rcu
semantics. Note that nf_ct_put() called from flow_offload_free()
releases a conntrack inmmediately.

This patch adds the missing READ_ONCE/WRITE_ONCE to update/access the
flow entry timeout. Then, skip expired flow entries from the flow lookup
function so they become stale inmediately even though the GC did not set
on the teardown bit yet.

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 | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/net/netfilter/nf_flow_table_core.c b/net/netfilter/nf_flow_table_core.c
index cbb56ca94717..675d470ea581 100644
--- a/net/netfilter/nf_flow_table_core.c
+++ b/net/netfilter/nf_flow_table_core.c
@@ -172,7 +172,7 @@ EXPORT_SYMBOL_GPL(flow_offload_route_init);
 
 static inline bool nf_flow_has_expired(const struct flow_offload *flow)
 {
-	return nf_flow_timeout_delta(flow->timeout) <= 0;
+	return nf_flow_timeout_delta(READ_ONCE(flow->timeout)) <= 0;
 }
 
 static void flow_offload_fixup_tcp(struct nf_conn *ct, u8 tcp_state)
@@ -329,7 +329,7 @@ int flow_offload_add(struct nf_flowtable *flow_table, struct flow_offload *flow)
 {
 	int err;
 
-	flow->timeout = nf_flowtable_time_stamp + flow_offload_get_timeout(flow);
+	WRITE_ONCE(flow->timeout, nf_flowtable_time_stamp);
 
 	err = rhashtable_insert_fast(&flow_table->rhashtable,
 				     &flow->tuplehash[FLOW_OFFLOAD_DIR_ORIGINAL].node,
@@ -352,6 +352,7 @@ int flow_offload_add(struct nf_flowtable *flow_table, struct flow_offload *flow)
 	if (nf_flowtable_hw_offload(flow_table))
 		nf_flow_offload_add(flow_table, flow);
 
+	WRITE_ONCE(flow->timeout, flow->timeout + flow_offload_get_timeout(flow));
 	smp_mb__before_atomic();
 	set_bit(NF_FLOW_CONFIRMED, &flow->flags);
 
@@ -414,10 +415,9 @@ flow_offload_lookup(struct nf_flowtable *flow_table,
 
 	dir = tuplehash->tuple.dir;
 	flow = container_of(tuplehash, struct flow_offload, tuplehash[dir]);
-	if (test_bit(NF_FLOW_TEARDOWN, &flow->flags))
-		return NULL;
-
-	if (unlikely(nf_ct_is_dying(flow->ct)))
+	if (nf_flow_has_expired(flow) ||
+	    test_bit(NF_FLOW_TEARDOWN, &flow->flags) ||
+	    unlikely(nf_ct_is_dying(flow->ct)))
 		return NULL;
 
 	return tuplehash;
-- 
2.47.3