public inbox for netfilter-devel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH nf] netfilter: nf_tables: reject requests exceeding NF_FLOW_RULE_ACTION_MAX actions
@ 2026-03-25 16:41 Florian Westphal
  2026-03-26 12:03 ` Fernando Fernandez Mancera
  2026-03-26 21:49 ` Pablo Neira Ayuso
  0 siblings, 2 replies; 3+ messages in thread
From: Florian Westphal @ 2026-03-25 16:41 UTC (permalink / raw)
  To: netfilter-devel; +Cc: Florian Westphal, Hyunwoo Kim

nf_flow_offload_rule_alloc() allocates space for NF_FLOW_RULE_ACTION_MAX
entries.  Make sure userspace passes more entries to us.

Reported-by: Hyunwoo Kim <imv4bel@gmail.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
---
 Can also route via nf-next if thats deemed the better tree.

 include/net/netfilter/nf_flow_table.h | 2 ++
 net/netfilter/nf_flow_table_offload.c | 2 --
 net/netfilter/nf_tables_offload.c     | 5 +++--
 3 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/include/net/netfilter/nf_flow_table.h b/include/net/netfilter/nf_flow_table.h
index b09c11c048d5..0b2fb1467b3f 100644
--- a/include/net/netfilter/nf_flow_table.h
+++ b/include/net/netfilter/nf_flow_table.h
@@ -13,6 +13,8 @@
 #include <linux/if_pppox.h>
 #include <linux/ppp_defs.h>
 
+#define NF_FLOW_RULE_ACTION_MAX	16
+
 struct nf_flowtable;
 struct nf_flow_rule;
 struct flow_offload;
diff --git a/net/netfilter/nf_flow_table_offload.c b/net/netfilter/nf_flow_table_offload.c
index 9b677e116487..11463682bbfa 100644
--- a/net/netfilter/nf_flow_table_offload.c
+++ b/net/netfilter/nf_flow_table_offload.c
@@ -727,8 +727,6 @@ int nf_flow_rule_route_ipv6(struct net *net, struct flow_offload *flow,
 }
 EXPORT_SYMBOL_GPL(nf_flow_rule_route_ipv6);
 
-#define NF_FLOW_RULE_ACTION_MAX	16
-
 static struct nf_flow_rule *
 nf_flow_offload_rule_alloc(struct net *net,
 			   const struct flow_offload_work *offload,
diff --git a/net/netfilter/nf_tables_offload.c b/net/netfilter/nf_tables_offload.c
index 9101b1703b52..a2f7966bc201 100644
--- a/net/netfilter/nf_tables_offload.c
+++ b/net/netfilter/nf_tables_offload.c
@@ -88,10 +88,11 @@ static void nft_flow_rule_transfer_vlan(struct nft_offload_ctx *ctx,
 struct nft_flow_rule *nft_flow_rule_create(struct net *net,
 					   const struct nft_rule *rule)
 {
+	unsigned int num_actions = 0;
 	struct nft_offload_ctx *ctx;
 	struct nft_flow_rule *flow;
-	int num_actions = 0, err;
 	struct nft_expr *expr;
+	int err;
 
 	expr = nft_expr_first(rule);
 	while (nft_expr_more(rule, expr)) {
@@ -102,7 +103,7 @@ struct nft_flow_rule *nft_flow_rule_create(struct net *net,
 		expr = nft_expr_next(expr);
 	}
 
-	if (num_actions == 0)
+	if (num_actions == 0 || num_actions > NF_FLOW_RULE_ACTION_MAX)
 		return ERR_PTR(-EOPNOTSUPP);
 
 	flow = nft_flow_rule_alloc(num_actions);
-- 
2.52.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH nf] netfilter: nf_tables: reject requests exceeding NF_FLOW_RULE_ACTION_MAX actions
  2026-03-25 16:41 [PATCH nf] netfilter: nf_tables: reject requests exceeding NF_FLOW_RULE_ACTION_MAX actions Florian Westphal
@ 2026-03-26 12:03 ` Fernando Fernandez Mancera
  2026-03-26 21:49 ` Pablo Neira Ayuso
  1 sibling, 0 replies; 3+ messages in thread
From: Fernando Fernandez Mancera @ 2026-03-26 12:03 UTC (permalink / raw)
  To: Florian Westphal, netfilter-devel; +Cc: Hyunwoo Kim

On 3/25/26 5:41 PM, Florian Westphal wrote:
> nf_flow_offload_rule_alloc() allocates space for NF_FLOW_RULE_ACTION_MAX
> entries.  Make sure userspace passes more entries to us.
> 

nit: shouldn't this be "Make sure userspace does not pass more entries 
to us"?

Other than that, LGTM.

Thanks.

> Reported-by: Hyunwoo Kim <imv4bel@gmail.com>
> Signed-off-by: Florian Westphal <fw@strlen.de>
> ---
>   Can also route via nf-next if thats deemed the better tree.
> 
>   include/net/netfilter/nf_flow_table.h | 2 ++
>   net/netfilter/nf_flow_table_offload.c | 2 --
>   net/netfilter/nf_tables_offload.c     | 5 +++--
>   3 files changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/include/net/netfilter/nf_flow_table.h b/include/net/netfilter/nf_flow_table.h
> index b09c11c048d5..0b2fb1467b3f 100644
> --- a/include/net/netfilter/nf_flow_table.h
> +++ b/include/net/netfilter/nf_flow_table.h
> @@ -13,6 +13,8 @@
>   #include <linux/if_pppox.h>
>   #include <linux/ppp_defs.h>
>   
> +#define NF_FLOW_RULE_ACTION_MAX	16
> +
>   struct nf_flowtable;
>   struct nf_flow_rule;
>   struct flow_offload;
> diff --git a/net/netfilter/nf_flow_table_offload.c b/net/netfilter/nf_flow_table_offload.c
> index 9b677e116487..11463682bbfa 100644
> --- a/net/netfilter/nf_flow_table_offload.c
> +++ b/net/netfilter/nf_flow_table_offload.c
> @@ -727,8 +727,6 @@ int nf_flow_rule_route_ipv6(struct net *net, struct flow_offload *flow,
>   }
>   EXPORT_SYMBOL_GPL(nf_flow_rule_route_ipv6);
>   
> -#define NF_FLOW_RULE_ACTION_MAX	16
> -
>   static struct nf_flow_rule *
>   nf_flow_offload_rule_alloc(struct net *net,
>   			   const struct flow_offload_work *offload,
> diff --git a/net/netfilter/nf_tables_offload.c b/net/netfilter/nf_tables_offload.c
> index 9101b1703b52..a2f7966bc201 100644
> --- a/net/netfilter/nf_tables_offload.c
> +++ b/net/netfilter/nf_tables_offload.c
> @@ -88,10 +88,11 @@ static void nft_flow_rule_transfer_vlan(struct nft_offload_ctx *ctx,
>   struct nft_flow_rule *nft_flow_rule_create(struct net *net,
>   					   const struct nft_rule *rule)
>   {
> +	unsigned int num_actions = 0;
>   	struct nft_offload_ctx *ctx;
>   	struct nft_flow_rule *flow;
> -	int num_actions = 0, err;
>   	struct nft_expr *expr;
> +	int err;
>   
>   	expr = nft_expr_first(rule);
>   	while (nft_expr_more(rule, expr)) {
> @@ -102,7 +103,7 @@ struct nft_flow_rule *nft_flow_rule_create(struct net *net,
>   		expr = nft_expr_next(expr);
>   	}
>   
> -	if (num_actions == 0)
> +	if (num_actions == 0 || num_actions > NF_FLOW_RULE_ACTION_MAX)
>   		return ERR_PTR(-EOPNOTSUPP);
>   
>   	flow = nft_flow_rule_alloc(num_actions);


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH nf] netfilter: nf_tables: reject requests exceeding NF_FLOW_RULE_ACTION_MAX actions
  2026-03-25 16:41 [PATCH nf] netfilter: nf_tables: reject requests exceeding NF_FLOW_RULE_ACTION_MAX actions Florian Westphal
  2026-03-26 12:03 ` Fernando Fernandez Mancera
@ 2026-03-26 21:49 ` Pablo Neira Ayuso
  1 sibling, 0 replies; 3+ messages in thread
From: Pablo Neira Ayuso @ 2026-03-26 21:49 UTC (permalink / raw)
  To: Florian Westphal; +Cc: netfilter-devel, Hyunwoo Kim

On Wed, Mar 25, 2026 at 05:41:27PM +0100, Florian Westphal wrote:
> nf_flow_offload_rule_alloc() allocates space for NF_FLOW_RULE_ACTION_MAX
> entries.  Make sure userspace passes more entries to us.

While the flowtable hardware offload uses a fixed maximum number of
actions NF_FLOW_RULE_ACTION_MAX for simplicity.

But nf_tables hardware offload allocates the number of actions
dynamically from nft_flow_rule_create(), such function iterates to
check if there is .offload_action is true, the increments the array of
actions by one for each.

Possible actions (note payload mangling is currently not supported
in nf_tables hardware offload).

This is fragile, because advancing the action array is opencoded:

        entry = &flow->rule->action.entries[ctx->num_actions++];

I can make a patch for nf-next to add a helper, but I don't see any
issue on nf_tables_offload at this stage. There are three actions only
and they add one single entry to the array.

As for the flowtable hardware offload (different infrastructure)
I proposed a different approach:

https://patchwork.ozlabs.org/project/netfilter-devel/patch/20260326200935.729750-1-pablo@netfilter.org/

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-03-26 21:49 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-25 16:41 [PATCH nf] netfilter: nf_tables: reject requests exceeding NF_FLOW_RULE_ACTION_MAX actions Florian Westphal
2026-03-26 12:03 ` Fernando Fernandez Mancera
2026-03-26 21:49 ` Pablo Neira Ayuso

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox