| Message ID | 20260325164130.29060-1-fw@strlen.de |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | [nf] netfilter: nf_tables: reject requests exceeding NF_FLOW_RULE_ACTION_MAX actions | expand |
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);
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/
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);
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(-)