netfilter-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH nf] netfilter: nf_tables: validate objref and objrefmap expressions
@ 2025-10-04 23:04 Fernando Fernandez Mancera
  2025-10-05 11:43 ` Florian Westphal
  2025-10-07 20:54 ` Pablo Neira Ayuso
  0 siblings, 2 replies; 5+ messages in thread
From: Fernando Fernandez Mancera @ 2025-10-04 23:04 UTC (permalink / raw)
  To: netfilter-devel
  Cc: coreteam, pablo, fw, Fernando Fernandez Mancera,
	Georg Pfuetzenreuter

Referencing a synproxy stateful object can cause a kernel crash due to
its usage on the OUTPUT hook. See the following trace:

BUG: TASK stack guard page was hit at 000000008bda5b8c (stack is 000000003ab1c4a5..00000000494d8b12)
[...]
Call Trace:
 <TASK>
 __find_rr_leaf+0x99/0x230
 fib6_table_lookup+0x13b/0x2d0
 ip6_pol_route+0xa4/0x400
 fib6_rule_lookup+0x156/0x240
 ip6_route_output_flags+0xc6/0x150
 __nf_ip6_route+0x23/0x50
 synproxy_send_tcp_ipv6+0x106/0x200 [nf_synproxy_core 2531b21bd6e9569ffd76f28a949f7f2922fec65d]
 synproxy_send_client_synack_ipv6+0x1aa/0x1f0 [nf_synproxy_core 2531b21bd6e9569ffd76f28a949f7f2922fec65d]
 nft_synproxy_do_eval+0x263/0x310 [nft_synproxy 1dcf907d37286d566a63aee8e263fccf6aba22d5]
 nft_do_chain+0x5a8/0x5f0 [nf_tables 5137ebab9ca1fa5980eb27e6394e4f7bf139224e]
 nft_do_chain_inet+0x98/0x110 [nf_tables 5137ebab9ca1fa5980eb27e6394e4f7bf139224e]
 nf_hook_slow+0x43/0xc0
 __ip6_local_out+0xf0/0x170
 ip6_local_out+0x17/0x70
 synproxy_send_tcp_ipv6+0x1a2/0x200 [nf_synproxy_core 2531b21bd6e9569ffd76f28a949f7f2922fec65d]
 synproxy_send_client_synack_ipv6+0x1aa/0x1f0 [nf_synproxy_core 2531b21bd6e9569ffd76f28a949f7f2922fec65d]
[...]

To avoid such situations, implement objref and objrefmap expression
validate function. Currently, only NFT_OBJECT_SYNPROXY object type
requires validation. This will also handle a jump to a chain using a
synproxy object from the OUTPUT hook.

Now when trying to reference a synproxy object in the OUTPUT hook, nft
will produce the following error:

synproxy_crash.nft:11:3-26: Error: Could not process rule: Operation not supported
		synproxy name mysynproxy
		^^^^^^^^^^^^^^^^^^^^^^^^

Fixes: ee394f96ad75 ("netfilter: nft_synproxy: add synproxy stateful object support")
Reported-by: Georg Pfuetzenreuter <georg.pfuetzenreuter@suse.com>
Closes: https://bugzilla.suse.com/1250237
Signed-off-by: Fernando Fernandez Mancera <fmancera@suse.de>
---
 include/net/netfilter/nf_tables.h      |  5 +++
 include/net/netfilter/nf_tables_core.h |  2 +
 net/netfilter/nf_tables_api.c          | 43 +++++++++++++++++++++
 net/netfilter/nft_objref.c             | 52 ++++++++++++++++++++++++++
 4 files changed, 102 insertions(+)

diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
index fab7dc73f738..5994f465cbdc 100644
--- a/include/net/netfilter/nf_tables.h
+++ b/include/net/netfilter/nf_tables.h
@@ -1135,6 +1135,11 @@ int nft_setelem_validate(const struct nft_ctx *ctx, struct nft_set *set,
 			 const struct nft_set_iter *iter,
 			 struct nft_elem_priv *elem_priv);
 int nft_set_catchall_validate(const struct nft_ctx *ctx, struct nft_set *set);
+int nft_setelem_obj_validate(const struct nft_ctx *ctx, struct nft_set *set,
+			     const struct nft_set_iter *iter,
+			     struct nft_elem_priv *elem_priv);
+int nft_set_catchall_obj_validate(const struct nft_ctx *ctx,
+				  struct nft_set *set);
 int nf_tables_bind_chain(const struct nft_ctx *ctx, struct nft_chain *chain);
 void nf_tables_unbind_chain(const struct nft_ctx *ctx, struct nft_chain *chain);
 
diff --git a/include/net/netfilter/nf_tables_core.h b/include/net/netfilter/nf_tables_core.h
index b8df5acbb723..097564ec998d 100644
--- a/include/net/netfilter/nf_tables_core.h
+++ b/include/net/netfilter/nf_tables_core.h
@@ -180,6 +180,8 @@ void nft_payload_inner_eval(const struct nft_expr *expr, struct nft_regs *regs,
 
 void nft_objref_eval(const struct nft_expr *expr, struct nft_regs *regs,
 		     const struct nft_pktinfo *pkt);
+int nft_objref_validate_obj(const struct nft_ctx *ctx,
+			    const struct nft_object *obj);
 void nft_objref_map_eval(const struct nft_expr *expr, struct nft_regs *regs,
 			 const struct nft_pktinfo *pkt);
 struct nft_elem_priv *nft_dynset_new(struct nft_set *set,
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index eed434e0a970..470fe57ae6fb 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -4145,6 +4145,49 @@ static int nft_table_validate(struct net *net, const struct nft_table *table)
 	return 0;
 }
 
+int nft_setelem_obj_validate(const struct nft_ctx *ctx, struct nft_set *set,
+			     const struct nft_set_iter *iter,
+			     struct nft_elem_priv *elem_priv)
+{
+	const struct nft_set_ext *ext = nft_set_elem_ext(set, elem_priv);
+	struct nft_object *obj;
+
+	if (!nft_set_elem_active(ext, iter->genmask))
+		return 0;
+
+	if (nft_set_ext_exists(ext, NFT_SET_EXT_FLAGS) &&
+	    *nft_set_ext_flags(ext) & NFT_SET_ELEM_INTERVAL_END)
+		return 0;
+
+	obj = *nft_set_ext_obj(ext);
+
+	return nft_objref_validate_obj(ctx, obj);
+}
+
+int nft_set_catchall_obj_validate(const struct nft_ctx *ctx, struct nft_set *set)
+{
+	struct nft_set_iter dummy_iter = {
+		.genmask	= nft_genmask_next(ctx->net),
+	};
+	struct nft_set_elem_catchall *catchall;
+
+	struct nft_set_ext *ext;
+	int ret = 0;
+
+	list_for_each_entry_rcu(catchall, &set->catchall_list, list,
+				lockdep_commit_lock_is_held(ctx->net)) {
+		ext = nft_set_elem_ext(set, catchall->elem);
+		if (!nft_set_elem_active(ext, dummy_iter.genmask))
+			continue;
+
+		ret = nft_setelem_obj_validate(ctx, set, &dummy_iter, catchall->elem);
+		if (ret < 0)
+			return ret;
+	}
+
+	return ret;
+}
+
 int nft_setelem_validate(const struct nft_ctx *ctx, struct nft_set *set,
 			 const struct nft_set_iter *iter,
 			 struct nft_elem_priv *elem_priv)
diff --git a/net/netfilter/nft_objref.c b/net/netfilter/nft_objref.c
index 8ee66a86c3bc..2dd5dbd0354d 100644
--- a/net/netfilter/nft_objref.c
+++ b/net/netfilter/nft_objref.c
@@ -22,6 +22,36 @@ void nft_objref_eval(const struct nft_expr *expr,
 	obj->ops->eval(obj, regs, pkt);
 }
 
+int nft_objref_validate_obj(const struct nft_ctx *ctx,
+			    const struct nft_object *obj)
+{
+	unsigned int hooks;
+
+	switch (obj->ops->type->type) {
+	case NFT_OBJECT_SYNPROXY:
+		if (ctx->family != NFPROTO_IPV4 &&
+		    ctx->family != NFPROTO_IPV6 &&
+		    ctx->family != NFPROTO_INET)
+			return -EOPNOTSUPP;
+
+		hooks = (1 << NF_INET_LOCAL_IN) | (1 << NF_INET_FORWARD);
+
+		return nft_chain_validate_hooks(ctx->chain, hooks);
+	default:
+		break;
+	}
+
+	return 0;
+}
+
+static int nft_objref_validate(const struct nft_ctx *ctx,
+			       const struct nft_expr *expr)
+{
+	struct nft_object *obj = nft_objref_priv(expr);
+
+	return nft_objref_validate_obj(ctx, obj);
+}
+
 static int nft_objref_init(const struct nft_ctx *ctx,
 			   const struct nft_expr *expr,
 			   const struct nlattr * const tb[])
@@ -93,6 +123,7 @@ static const struct nft_expr_ops nft_objref_ops = {
 	.activate	= nft_objref_activate,
 	.deactivate	= nft_objref_deactivate,
 	.dump		= nft_objref_dump,
+	.validate	= nft_objref_validate,
 	.reduce		= NFT_REDUCE_READONLY,
 };
 
@@ -197,6 +228,26 @@ static void nft_objref_map_destroy(const struct nft_ctx *ctx,
 	nf_tables_destroy_set(ctx, priv->set);
 }
 
+static int nft_objref_map_validate(const struct nft_ctx *ctx,
+				   const struct nft_expr *expr)
+{
+	const struct nft_objref_map *priv = nft_expr_priv(expr);
+	struct nft_set_iter iter = {
+		.genmask	= nft_genmask_next(ctx->net),
+		.type		= NFT_ITER_UPDATE,
+		.fn		= nft_setelem_obj_validate,
+	};
+
+	priv->set->ops->walk(ctx, priv->set, &iter);
+	if (!iter.err)
+		iter.err = nft_set_catchall_obj_validate(ctx, priv->set);
+
+	if (iter.err < 0)
+		return iter.err;
+
+	return 0;
+}
+
 static const struct nft_expr_ops nft_objref_map_ops = {
 	.type		= &nft_objref_type,
 	.size		= NFT_EXPR_SIZE(sizeof(struct nft_objref_map)),
@@ -206,6 +257,7 @@ static const struct nft_expr_ops nft_objref_map_ops = {
 	.deactivate	= nft_objref_map_deactivate,
 	.destroy	= nft_objref_map_destroy,
 	.dump		= nft_objref_map_dump,
+	.validate	= nft_objref_map_validate,
 	.reduce		= NFT_REDUCE_READONLY,
 };
 
-- 
2.51.0


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

* Re: [PATCH nf] netfilter: nf_tables: validate objref and objrefmap expressions
  2025-10-04 23:04 [PATCH nf] netfilter: nf_tables: validate objref and objrefmap expressions Fernando Fernandez Mancera
@ 2025-10-05 11:43 ` Florian Westphal
  2025-10-05 15:02   ` Fernando Fernandez Mancera
  2025-10-07 20:54 ` Pablo Neira Ayuso
  1 sibling, 1 reply; 5+ messages in thread
From: Florian Westphal @ 2025-10-05 11:43 UTC (permalink / raw)
  To: Fernando Fernandez Mancera
  Cc: netfilter-devel, coreteam, pablo, Georg Pfuetzenreuter

Fernando Fernandez Mancera <fmancera@suse.de> wrote:
> Referencing a synproxy stateful object can cause a kernel crash due to
> its usage on the OUTPUT hook. See the following trace:

I edited this slightly to mention the recursion and applied
this patch to nf:testing.

Let me know if I messed anything up.

I did not alter the actual code changes; patch LGTM.

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

* Re: [PATCH nf] netfilter: nf_tables: validate objref and objrefmap expressions
  2025-10-05 11:43 ` Florian Westphal
@ 2025-10-05 15:02   ` Fernando Fernandez Mancera
  0 siblings, 0 replies; 5+ messages in thread
From: Fernando Fernandez Mancera @ 2025-10-05 15:02 UTC (permalink / raw)
  To: Florian Westphal; +Cc: netfilter-devel, coreteam, pablo, Georg Pfuetzenreuter



On 10/5/25 1:43 PM, Florian Westphal wrote:
> Fernando Fernandez Mancera <fmancera@suse.de> wrote:
>> Referencing a synproxy stateful object can cause a kernel crash due to
>> its usage on the OUTPUT hook. See the following trace:
> 
> I edited this slightly to mention the recursion and applied
> this patch to nf:testing.
> 
> Let me know if I messed anything up.
> 

All looks good, thank you Florian!


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

* Re: [PATCH nf] netfilter: nf_tables: validate objref and objrefmap expressions
  2025-10-04 23:04 [PATCH nf] netfilter: nf_tables: validate objref and objrefmap expressions Fernando Fernandez Mancera
  2025-10-05 11:43 ` Florian Westphal
@ 2025-10-07 20:54 ` Pablo Neira Ayuso
  2025-10-08  7:58   ` Fernando Fernandez Mancera
  1 sibling, 1 reply; 5+ messages in thread
From: Pablo Neira Ayuso @ 2025-10-07 20:54 UTC (permalink / raw)
  To: Fernando Fernandez Mancera
  Cc: netfilter-devel, coreteam, fw, Georg Pfuetzenreuter

Hi Fernando,

On Sun, Oct 05, 2025 at 01:04:24AM +0200, Fernando Fernandez Mancera wrote:
[...]
> To avoid such situations, implement objref and objrefmap expression
> validate function. Currently, only NFT_OBJECT_SYNPROXY object type
> requires validation. This will also handle a jump to a chain using a
> synproxy object from the OUTPUT hook.
> 
> Now when trying to reference a synproxy object in the OUTPUT hook, nft
> will produce the following error:
> 
> synproxy_crash.nft:11:3-26: Error: Could not process rule: Operation not supported
> 		synproxy name mysynproxy
> 		^^^^^^^^^^^^^^^^^^^^^^^^

Object maps only store one type of object, ie. all objects are of the
same type.

        if (nla[NFTA_SET_OBJ_TYPE] != NULL) {
                if (!(flags & NFT_SET_OBJECT))
                        return -EINVAL;

                desc.objtype = ntohl(nla_get_be32(nla[NFTA_SET_OBJ_TYPE]));
                if (desc.objtype == NFT_OBJECT_UNSPEC ||
                    desc.objtype > NFT_OBJECT_MAX)
                        return -EOPNOTSUPP;
        } else if (flags & NFT_SET_OBJECT)
                return -EINVAL;
        else
                desc.objtype = NFT_OBJECT_UNSPEC;

I think it should be possible to simplify this patch: From objref, you
could check if map is of NFT_OBJECT_SYNPROXY type, then check if the
right hook type is used.

Thanks.

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

* Re: [PATCH nf] netfilter: nf_tables: validate objref and objrefmap expressions
  2025-10-07 20:54 ` Pablo Neira Ayuso
@ 2025-10-08  7:58   ` Fernando Fernandez Mancera
  0 siblings, 0 replies; 5+ messages in thread
From: Fernando Fernandez Mancera @ 2025-10-08  7:58 UTC (permalink / raw)
  To: Pablo Neira Ayuso; +Cc: netfilter-devel, coreteam, fw, Georg Pfuetzenreuter



On 10/7/25 10:54 PM, Pablo Neira Ayuso wrote:
> Hi Fernando,
> 
> On Sun, Oct 05, 2025 at 01:04:24AM +0200, Fernando Fernandez Mancera wrote:
> [...]
>> To avoid such situations, implement objref and objrefmap expression
>> validate function. Currently, only NFT_OBJECT_SYNPROXY object type
>> requires validation. This will also handle a jump to a chain using a
>> synproxy object from the OUTPUT hook.
>>
>> Now when trying to reference a synproxy object in the OUTPUT hook, nft
>> will produce the following error:
>>
>> synproxy_crash.nft:11:3-26: Error: Could not process rule: Operation not supported
>> 		synproxy name mysynproxy
>> 		^^^^^^^^^^^^^^^^^^^^^^^^
> 
> Object maps only store one type of object, ie. all objects are of the
> same type.
> 
>          if (nla[NFTA_SET_OBJ_TYPE] != NULL) {
>                  if (!(flags & NFT_SET_OBJECT))
>                          return -EINVAL;
> 
>                  desc.objtype = ntohl(nla_get_be32(nla[NFTA_SET_OBJ_TYPE]));
>                  if (desc.objtype == NFT_OBJECT_UNSPEC ||
>                      desc.objtype > NFT_OBJECT_MAX)
>                          return -EOPNOTSUPP;
>          } else if (flags & NFT_SET_OBJECT)
>                  return -EINVAL;
>          else
>                  desc.objtype = NFT_OBJECT_UNSPEC;
> 
> I think it should be possible to simplify this patch: From objref, you
> could check if map is of NFT_OBJECT_SYNPROXY type, then check if the
> right hook type is used.
> 

Ah right, it makes sense. Thank you, let me send a V2.

> Thanks.
> 


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

end of thread, other threads:[~2025-10-08  7:58 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-04 23:04 [PATCH nf] netfilter: nf_tables: validate objref and objrefmap expressions Fernando Fernandez Mancera
2025-10-05 11:43 ` Florian Westphal
2025-10-05 15:02   ` Fernando Fernandez Mancera
2025-10-07 20:54 ` Pablo Neira Ayuso
2025-10-08  7:58   ` Fernando Fernandez Mancera

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).