Netdev List
 help / color / mirror / Atom feed
* [PATCH net] netfilter: conntrack: avoid recursive master destruction
@ 2026-10-01 18:02 Daehyeon Ko
  2026-10-01 19:19 ` Florian Westphal
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Daehyeon Ko @ 2026-10-01 18:02 UTC (permalink / raw)
  To: pablo, fw; +Cc: phil, netfilter-devel, coreteam, netdev, Daehyeon Ko, stable

Conntrack entries created through ctnetlink can reference another
confirmed entry as their master.  There is no limit on the resulting
chain depth.

When the last external reference to such a chain is dropped,
nf_ct_destroy() puts the master reference.  If that is the master's last
reference, nf_ct_put() invokes nf_ct_destroy() recursively.  A sufficiently
long chain therefore exhausts the task stack.

Release the master reference directly.  When it was the final reference,
continue destroying it in the current invocation.  This keeps the existing
refcount and lifetime rules while bounding stack use.

Fixes: 5faa1f4cb5a1 ("[NETFILTER]: nf_conntrack_netlink: add support to related connections")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Daehyeon Ko <4ncienth@gmail.com>
---
Tested on net e23a64eb244356ee47c0620f0722d51bd88db522 and exact
v6.12.105. A source reproducer and userns launcher are available privately
on request and are intentionally omitted from this public posting.

The trigger needs CONFIG_USER_NS, CONFIG_NET_NS, CONFIG_NF_CONNTRACK and
CONFIG_NF_CT_NETLINK. Host UID 65534 used only namespace-local
CAP_NET_ADMIN. The essential vulnerable trace is:

  BUG: TASK stack guard page was hit at ffffc90001197ff8
  CPU: 1 UID: 65534 PID: 178 Comm: conntrack-maste
  nf_ct_destroy+0x1ac/0x5f0 (repeated)

Fixed current and LTS 6,000-entry runs ended with nf_conntrack_count=0 and
no crash marker. The netdev allyesconfig and allmodconfig W=1 full builds
were not run.

 net/netfilter/nf_conntrack_core.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
index d0d9e5ea84a09..0ce6141b3dfd7 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -592,6 +592,10 @@ static void warn_on_keymap_list_leak(const struct net *net)
 void nf_ct_destroy(struct nf_conntrack *nfct)
 {
 	struct nf_conn *ct = (struct nf_conn *)nfct;
+	struct nf_conn *master;
+	bool destroy_master;
+
+again:
 
 	WARN_ON(refcount_read(&nfct->use) != 0);
 
@@ -610,10 +614,17 @@ void nf_ct_destroy(struct nf_conntrack *nfct)
 	 */
 	nf_ct_remove_expectations(ct);
 
-	if (ct->master)
-		nf_ct_put(ct->master);
+	master = ct->master;
+	destroy_master = master &&
+		refcount_dec_and_test(&master->ct_general.use);
 
 	nf_conntrack_free(ct);
+
+	if (destroy_master) {
+		ct = master;
+		nfct = &ct->ct_general;
+		goto again;
+	}
 }
 EXPORT_SYMBOL(nf_ct_destroy);
 
-- 
2.55.0

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

* Re: [PATCH net] netfilter: conntrack: avoid recursive master destruction
  2026-10-01 18:02 [PATCH net] netfilter: conntrack: avoid recursive master destruction Daehyeon Ko
@ 2026-10-01 19:19 ` Florian Westphal
  2026-10-02  5:39   ` Daehyeon Ko
  2026-10-02  9:55   ` Pablo Neira Ayuso
  2026-10-02  9:55 ` Pablo Neira Ayuso
  2026-10-02 16:56 ` [PATCH v2 net] netfilter: conntrack: reject nested ctnetlink master chains Daehyeon Ko
  2 siblings, 2 replies; 11+ messages in thread
From: Florian Westphal @ 2026-10-01 19:19 UTC (permalink / raw)
  To: Daehyeon Ko; +Cc: pablo, phil, netfilter-devel, coreteam, netdev, stable

Daehyeon Ko <4ncienth@gmail.com> wrote:
> Conntrack entries created through ctnetlink can reference another
> confirmed entry as their master.  There is no limit on the resulting
> chain depth.

I don't think this should be allowed.  I.e. (totally untested):

diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
--- a/net/netfilter/nf_conntrack_netlink.c
+++ b/net/netfilter/nf_conntrack_netlink.c
@@ -2359,6 +2359,12 @@ ctnetlink_create_conntrack(struct net *net,
                        goto err2;
                }
                master_ct = nf_ct_tuplehash_to_ctrack(master_h);
+               if (master_ct->master) {
+                       nf_ct_put(master_ct);
+                       err = -EOPNOTSUPP;
+                       goto err2;
+               }
+
                __set_bit(IPS_EXPECTED_BIT, &ct->status);
                ct->master = master_ct;

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

* Re: [PATCH net] netfilter: conntrack: avoid recursive master destruction
  2026-10-01 19:19 ` Florian Westphal
@ 2026-10-02  5:39   ` Daehyeon Ko
  2026-10-02  9:56     ` Pablo Neira Ayuso
  2026-10-02  9:55   ` Pablo Neira Ayuso
  1 sibling, 1 reply; 11+ messages in thread
From: Daehyeon Ko @ 2026-10-02  5:39 UTC (permalink / raw)
  To: fw; +Cc: pablo, phil, netfilter-devel, coreteam, netdev, stable

Thanks for taking a look.

I think this check is reasonable for CTA_TUPLE_MASTER, but it does not seem
to make master chains impossible by itself.  There is another master writer
in init_conntrack(): expectation fulfillment assigns exp->master to the new
conntrack.

In particular, ctnetlink_glue_attach_expect() can attach an expectation
from an NFQUEUE verdict and set assign_helper.  The expected child then has
both a master and a helper, so it can serve as the master of another
userspace-helper expectation.  That path does not pass through the proposed
check and has no cumulative depth bound.

This helper propagation was intentionally restored by dcb0f9aefdd6
("netfilter: nf_conntrack_expect: restore helper propagation via
expectation") for SIP, H.323 and userspace helpers.  Restricting all nested
masters would therefore need a wider helper/expectation compatibility
change.

For that reason I think nf_ct_destroy() still needs to be stack-safe.  The
creation-time check could be added separately if nested conntrackd restore
should also be rejected.  Does that match your intended invariant?

Thanks,
Daehyeon

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

* Re: [PATCH net] netfilter: conntrack: avoid recursive master destruction
  2026-10-01 18:02 [PATCH net] netfilter: conntrack: avoid recursive master destruction Daehyeon Ko
  2026-10-01 19:19 ` Florian Westphal
@ 2026-10-02  9:55 ` Pablo Neira Ayuso
  2026-10-02 16:56 ` [PATCH v2 net] netfilter: conntrack: reject nested ctnetlink master chains Daehyeon Ko
  2 siblings, 0 replies; 11+ messages in thread
From: Pablo Neira Ayuso @ 2026-10-02  9:55 UTC (permalink / raw)
  To: Daehyeon Ko; +Cc: fw, phil, netfilter-devel, coreteam, netdev, stable

On Fri, Oct 02, 2026 at 03:02:24AM +0900, Daehyeon Ko wrote:
> Conntrack entries created through ctnetlink can reference another
> confirmed entry as their master.  There is no limit on the resulting
> chain depth.

Then, just limit ctnetlink because this makes no sense.

> When the last external reference to such a chain is dropped,
> nf_ct_destroy() puts the master reference.  If that is the master's last
> reference, nf_ct_put() invokes nf_ct_destroy() recursively.  A sufficiently
> long chain therefore exhausts the task stack.
> 
> Release the master reference directly.  When it was the final reference,
> continue destroying it in the current invocation.  This keeps the existing
> refcount and lifetime rules while bounding stack use.
> 
> Fixes: 5faa1f4cb5a1 ("[NETFILTER]: nf_conntrack_netlink: add support to related connections")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Daehyeon Ko <4ncienth@gmail.com>
> ---
> Tested on net e23a64eb244356ee47c0620f0722d51bd88db522 and exact
> v6.12.105. A source reproducer and userns launcher are available privately
> on request and are intentionally omitted from this public posting.
> 
> The trigger needs CONFIG_USER_NS, CONFIG_NET_NS, CONFIG_NF_CONNTRACK and
> CONFIG_NF_CT_NETLINK. Host UID 65534 used only namespace-local
> CAP_NET_ADMIN. The essential vulnerable trace is:
> 
>   BUG: TASK stack guard page was hit at ffffc90001197ff8
>   CPU: 1 UID: 65534 PID: 178 Comm: conntrack-maste
>   nf_ct_destroy+0x1ac/0x5f0 (repeated)
> 
> Fixed current and LTS 6,000-entry runs ended with nf_conntrack_count=0 and
> no crash marker. The netdev allyesconfig and allmodconfig W=1 full builds
> were not run.
> 
>  net/netfilter/nf_conntrack_core.c | 15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
> index d0d9e5ea84a09..0ce6141b3dfd7 100644
> --- a/net/netfilter/nf_conntrack_core.c
> +++ b/net/netfilter/nf_conntrack_core.c
> @@ -592,6 +592,10 @@ static void warn_on_keymap_list_leak(const struct net *net)
>  void nf_ct_destroy(struct nf_conntrack *nfct)
>  {
>  	struct nf_conn *ct = (struct nf_conn *)nfct;
> +	struct nf_conn *master;
> +	bool destroy_master;
> +
> +again:
>  
>  	WARN_ON(refcount_read(&nfct->use) != 0);
>  
> @@ -610,10 +614,17 @@ void nf_ct_destroy(struct nf_conntrack *nfct)
>  	 */
>  	nf_ct_remove_expectations(ct);
>  
> -	if (ct->master)
> -		nf_ct_put(ct->master);
> +	master = ct->master;
> +	destroy_master = master &&
> +		refcount_dec_and_test(&master->ct_general.use);
>  
>  	nf_conntrack_free(ct);
> +
> +	if (destroy_master) {
> +		ct = master;
> +		nfct = &ct->ct_general;
> +		goto again;
> +	}
>  }
>  EXPORT_SYMBOL(nf_ct_destroy);
>  
> -- 
> 2.55.0

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

* Re: [PATCH net] netfilter: conntrack: avoid recursive master destruction
  2026-10-01 19:19 ` Florian Westphal
  2026-10-02  5:39   ` Daehyeon Ko
@ 2026-10-02  9:55   ` Pablo Neira Ayuso
  1 sibling, 0 replies; 11+ messages in thread
From: Pablo Neira Ayuso @ 2026-10-02  9:55 UTC (permalink / raw)
  To: Florian Westphal
  Cc: Daehyeon Ko, phil, netfilter-devel, coreteam, netdev, stable

On Thu, Oct 01, 2026 at 09:19:15PM +0200, Florian Westphal wrote:
> Daehyeon Ko <4ncienth@gmail.com> wrote:
> > Conntrack entries created through ctnetlink can reference another
> > confirmed entry as their master.  There is no limit on the resulting
> > chain depth.
> 
> I don't think this should be allowed.  I.e. (totally untested):
> 
> diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
> --- a/net/netfilter/nf_conntrack_netlink.c
> +++ b/net/netfilter/nf_conntrack_netlink.c
> @@ -2359,6 +2359,12 @@ ctnetlink_create_conntrack(struct net *net,
>                         goto err2;
>                 }
>                 master_ct = nf_ct_tuplehash_to_ctrack(master_h);
> +               if (master_ct->master) {
> +                       nf_ct_put(master_ct);
> +                       err = -EOPNOTSUPP;
> +                       goto err2;
> +               }

Agreed. Thanks.

> +
>                 __set_bit(IPS_EXPECTED_BIT, &ct->status);
>                 ct->master = master_ct;

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

* Re: [PATCH net] netfilter: conntrack: avoid recursive master destruction
  2026-10-02  5:39   ` Daehyeon Ko
@ 2026-10-02  9:56     ` Pablo Neira Ayuso
  0 siblings, 0 replies; 11+ messages in thread
From: Pablo Neira Ayuso @ 2026-10-02  9:56 UTC (permalink / raw)
  To: Daehyeon Ko; +Cc: fw, phil, netfilter-devel, coreteam, netdev, stable

On Fri, Oct 02, 2026 at 02:39:41PM +0900, Daehyeon Ko wrote:
> Thanks for taking a look.
> 
> I think this check is reasonable for CTA_TUPLE_MASTER, but it does not seem
> to make master chains impossible by itself.  There is another master writer
> in init_conntrack(): expectation fulfillment assigns exp->master to the new
> conntrack.
> 
> In particular, ctnetlink_glue_attach_expect() can attach an expectation
> from an NFQUEUE verdict and set assign_helper.  The expected child then has
> both a master and a helper, so it can serve as the master of another
> userspace-helper expectation.  That path does not pass through the proposed
> check and has no cumulative depth bound.

Then, just restrict glue_attach_expect() to reject this too.

> This helper propagation was intentionally restored by dcb0f9aefdd6
> ("netfilter: nf_conntrack_expect: restore helper propagation via
> expectation") for SIP, H.323 and userspace helpers.  Restricting all nested
> masters would therefore need a wider helper/expectation compatibility
> change.
> 
> For that reason I think nf_ct_destroy() still needs to be stack-safe.  The
> creation-time check could be added separately if nested conntrackd restore
> should also be rejected.  Does that match your intended invariant?
> 
> Thanks,
> Daehyeon

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

* [PATCH v2 net] netfilter: conntrack: reject nested ctnetlink master chains
  2026-10-01 18:02 [PATCH net] netfilter: conntrack: avoid recursive master destruction Daehyeon Ko
  2026-10-01 19:19 ` Florian Westphal
  2026-10-02  9:55 ` Pablo Neira Ayuso
@ 2026-10-02 16:56 ` Daehyeon Ko
  2026-10-02 16:58   ` netdev-bot+sinfo
                     ` (2 more replies)
  2 siblings, 3 replies; 11+ messages in thread
From: Daehyeon Ko @ 2026-10-02 16:56 UTC (permalink / raw)
  To: pablo, fw; +Cc: phil, netfilter-devel, coreteam, netdev, stable, 4ncienth

Conntracks created through ctnetlink can reference another conntrack as
their master.  Userspace can repeat this to build an unbounded chain whose
recursive destruction exhausts the kernel stack.

Stop the repeatable userspace paths.  Reject a direct master that already
has a master, and reject NFQUEUE-attached expectations for such conntracks.
Keep regular ctnetlink and kernel helper expectations unchanged.

Fixes: 5faa1f4cb5a1 ("[NETFILTER]: nf_conntrack_netlink: add support to related connections")
Cc: stable@vger.kernel.org
Suggested-by: Florian Westphal <fw@strlen.de>
Suggested-by: Pablo Neira Ayuso <pablo@netfilter.org>
Assisted-by: LLM
Signed-off-by: Daehyeon Ko <4ncienth@gmail.com>
---
Changes in v2:
- Replace iterative destruction with the maintainer-requested creation-time
  restrictions.
- Reject nested NFQUEUE-attached expectations while preserving regular
  ctnetlink and kernel helper expectations.

Tested on net 71a77ab76e74 and exact v6.12.105.  In both userns runs a
one-level master was accepted, 5,998 nested direct attempts returned
EOPNOTSUPP, and cleanup ended with nf_conntrack_count=0 without a crash.
A policy control also confirmed that terminal IPCTNL_MSG_EXP_NEW remains
accepted on a related master.  A real iptables NFQUEUE/NFQA_EXP control
created one expectation before the patch and none after it.  The related
object is W=1 warning-free.  The netdev allyesconfig and allmodconfig W=1
full builds were not run.

 net/netfilter/nf_conntrack_netlink.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
index 4e5d7c70143683..c68e79d1ac87b9 100644
--- a/net/netfilter/nf_conntrack_netlink.c
+++ b/net/netfilter/nf_conntrack_netlink.c
@@ -2359,6 +2359,11 @@ ctnetlink_create_conntrack(struct net *net,
 			goto err2;
 		}
 		master_ct = nf_ct_tuplehash_to_ctrack(master_h);
+		if (master_ct->master) {
+			nf_ct_put(master_ct);
+			err = -EOPNOTSUPP;
+			goto err2;
+		}
 		__set_bit(IPS_EXPECTED_BIT, &ct->status);
 		ct->master = master_ct;
 	}
@@ -2864,6 +2869,9 @@ ctnetlink_glue_attach_expect(const struct nlattr *attr, struct nf_conn *ct,
 	struct nf_conntrack_expect *exp;
 	int err;
 
+	if (ct->master)
+		return -EOPNOTSUPP;
+
 	err = nla_parse_nested_deprecated(cda, CTA_EXPECT_MAX, attr,
 					  exp_nla_policy, NULL);
 	if (err < 0)
-- 
2.55.0

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

* Re: [PATCH v2 net] netfilter: conntrack: reject nested ctnetlink master chains
  2026-10-02 16:56 ` [PATCH v2 net] netfilter: conntrack: reject nested ctnetlink master chains Daehyeon Ko
@ 2026-10-02 16:58   ` netdev-bot+sinfo
  2026-10-02 17:35   ` Florian Westphal
  2026-10-06  7:57   ` netdev-bot+sashiko
  2 siblings, 0 replies; 11+ messages in thread
From: netdev-bot+sinfo @ 2026-10-02 16:58 UTC (permalink / raw)
  To: Daehyeon Ko; +Cc: pablo, fw, phil, netfilter-devel, coreteam, netdev, stable

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

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

* Re: [PATCH v2 net] netfilter: conntrack: reject nested ctnetlink master chains
  2026-10-02 16:56 ` [PATCH v2 net] netfilter: conntrack: reject nested ctnetlink master chains Daehyeon Ko
  2026-10-02 16:58   ` netdev-bot+sinfo
@ 2026-10-02 17:35   ` Florian Westphal
  2026-10-06  7:57   ` netdev-bot+sashiko
  2 siblings, 0 replies; 11+ messages in thread
From: Florian Westphal @ 2026-10-02 17:35 UTC (permalink / raw)
  To: Daehyeon Ko; +Cc: pablo, phil, netfilter-devel, coreteam, netdev, stable

Daehyeon Ko <4ncienth@gmail.com> wrote:
> Conntracks created through ctnetlink can reference another conntrack as
> their master.  Userspace can repeat this to build an unbounded chain whose
> recursive destruction exhausts the kernel stack.
> 
> Stop the repeatable userspace paths.  Reject a direct master that already
> has a master, and reject NFQUEUE-attached expectations for such conntracks.
> Keep regular ctnetlink and kernel helper expectations unchanged.
> 
> Fixes: 5faa1f4cb5a1 ("[NETFILTER]: nf_conntrack_netlink: add support to related connections")
> Cc: stable@vger.kernel.org
> Suggested-by: Florian Westphal <fw@strlen.de>
> Suggested-by: Pablo Neira Ayuso <pablo@netfilter.org>
> Assisted-by: LLM
> Signed-off-by: Daehyeon Ko <4ncienth@gmail.com>

LGTM.  FWIW I think we should also apply the other patch to
destroy iteratively instead of recursing.

Reviewed-by: Florian Westphal <fw@strlen.de>

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

* Re: [PATCH v2 net] netfilter: conntrack: reject nested ctnetlink master chains
  2026-10-02 16:56 ` [PATCH v2 net] netfilter: conntrack: reject nested ctnetlink master chains Daehyeon Ko
  2026-10-02 16:58   ` netdev-bot+sinfo
  2026-10-02 17:35   ` Florian Westphal
@ 2026-10-06  7:57   ` netdev-bot+sashiko
  2026-10-07  1:33     ` Daehyeon Ko
  2 siblings, 1 reply; 11+ messages in thread
From: netdev-bot+sashiko @ 2026-10-06  7:57 UTC (permalink / raw)
  To: 4ncienth; +Cc: pablo, fw, phil, netfilter-devel, coreteam, netdev, stable, kuba

Thank you for your contribution! Sashiko AI review found 3 potential
issue(s) to consider.

Critical: 0 · High: 2 · Medium: 1 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] The fix does not stop userspace from building an unbounded
  ct->master chain.
- [High] The commit keeps "kernel helper expectations unchanged", but the
  H.323 Q.931 helper nests itself without limit.
- [Medium] The new check in ctnetlink_create_conntrack()
  (nf_conntrack_netlink.c:2362) makes IPCTNL_MSG_CT_NEW fail with…

--

netfilter: conntrack: reject nested ctnetlink master chains

The patch rejects ctnetlink conntrack creation when the CTA_TUPLE_MASTER
conntrack already has a master. It also rejects NFQUEUE-attached
expectations (NFQA_EXP) for conntracks that have a master. The aim is to
stop userspace from building unbounded master chains, whose recursive
release in nf_ct_destroy() exhausts the kernel stack.

> diff --git a/net/netfilter/nf_conntrack_netlink.c b/net/netfilter/nf_conntrack_netlink.c
> index 4e5d7c7014368..c68e79d1ac87b 100644
> --- a/net/netfilter/nf_conntrack_netlink.c
> +++ b/net/netfilter/nf_conntrack_netlink.c
> @@ -2359,6 +2359,11 @@ ctnetlink_create_conntrack(struct net *net,
>  			goto err2;
>  		}
>  		master_ct = nf_ct_tuplehash_to_ctrack(master_h);
> +		if (master_ct->master) {
> +			nf_ct_put(master_ct);
> +			err = -EOPNOTSUPP;
> +			goto err2;
> +		}

[Severity: Medium]
Does this check reject state that the kernel itself creates? In H.323, a
RAS expectation creates the Q.931 conntrack. That Q.931 conntrack then
sets up the H.245 expectation:

net/netfilter/nf_conntrack_h323_main.c:expect_h245() {
    ...
	rcu_assign_pointer(exp->assign_helper, nf_conntrack_helper_h245_ptr);
    ...
}

So the resulting H.245 and RTP conntracks have a master that itself has
a master.

A state-sync daemon such as conntrackd may recreate those flows with
IPCTNL_MSG_CT_NEW and a CTA_TUPLE_MASTER that points at the related Q.931
conntrack. That request used to be accepted and now gets -EOPNOTSUPP, so
these flows would not be replicated after a failover.

Since this is tagged for stable, could the commit message mention this
change in what CT_NEW accepts?

>  		__set_bit(IPS_EXPECTED_BIT, &ct->status);
>  		ct->master = master_ct;
>  	}
> @@ -2864,6 +2869,9 @@ ctnetlink_glue_attach_expect(const struct nlattr *attr, struct nf_conn *ct,
>  	struct nf_conntrack_expect *exp;
>  	int err;
>  
> +	if (ct->master)
> +		return -EOPNOTSUPP;
> +

[Severity: High]
Can nf_tables still reach the same stack exhaustion? The commit message
says "Stop the repeatable userspace paths", but the patch only covers
CTA_TUPLE_MASTER and NFQA_EXP.

The first path uses an nft ct expectation object alone.
nft_ct_expect_obj_eval() only skips conntracks that are confirmed,
templates, or that already have a help extension:

net/netfilter/nft_ct.c:nft_ct_expect_obj_eval() {
    ...
	ct = nf_ct_get(pkt->skb, &ctinfo);
	if (!ct || nf_ct_is_confirmed(ct) || nf_ct_is_template(ct)) {
		regs->verdict.code = NFT_BREAK;
		return;
	}

	help = nfct_help(ct);
	if (help) {
    ...
}

A RELATED child created from an expectation with a NULL assign_helper has
no help extension, so it also gets the ct_expect_help helper.

ct_expect_help() then calls nf_ct_expect_related() with the child as
exp->master, and it never checks ct->master. Over loopback, each new flow
from a fresh source port consumes the previous expectation and arms the
next one, so every flow adds a level.

The second path combines an nft ct helper with ctnetlink.
IPCTNL_MSG_EXP_NEW only stops the chain because ctnetlink_alloc_expect()
requires help->helper.

nft_ct_helper_obj_eval() assigns a helper to any unconfirmed conntrack,
related ones included. After that, ctnetlink_create_expect() succeeds
with the child as master.

A kernel helper attached this way, such as FTP, could also extend the
chain from traffic alone.

In every case init_conntrack() sets ct->master = exp->master with no
depth limit. Freeing the leaf then recurses once per level:

nf_ct_put()
  nf_ct_destroy()
    nf_ct_put(ct->master)
      nf_ct_destroy()
        ...

Both nft paths need only CAP_NET_ADMIN in a user/net namespace, the same
privilege ctnetlink needs.

Would a central bound in nf_ct_expect_related() or init_conntrack() be
needed to close this, or an iterative release of the master chain in
nf_ct_destroy()? Gating each entry point separately leaves these paths
open.

[Severity: High]
The commit message says "Keep regular ctnetlink and kernel helper
expectations unchanged". Can the H.323 Q.931 helper build an unbounded
chain on its own?

For a Facility message with reason callForwarded, process_facility()
calls expect_callforwarding(). That function gives the expected child the
Q.931 helper again:

net/netfilter/nf_conntrack_h323_main.c:expect_callforwarding() {
    ...
	rcu_assign_pointer(exp->assign_helper, nf_conntrack_helper_q931_ptr[0]);
    ...
}

Neither function checks IPS_EXPECTED or ct->master. Each new expected
Q.931 connection can therefore send another Facility(callForwarded) and
create the next level.

The default callforward_filter does not seem to prevent this.
callforward_do_filter() only filters when the nexthop and device are the
same, and two distinct local addresses have different nexthops.

This works over loopback from an unprivileged user namespace once the
helper is attached. It also works for remote peers wherever the Q.931
helper is enabled. Either way, the same recursion in nf_ct_destroy()
exhausts the stack.

SIP REGISTER does not seem to have this problem, because
process_register_request() returns early when IPS_EXPECTED is set.

>  	err = nla_parse_nested_deprecated(cda, CTA_EXPECT_MAX, attr,
>  					  exp_nla_policy, NULL);
>  	if (err < 0)

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261002165601.1754467-1-4ncienth%40gmail.com

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

* Re: [PATCH v2 net] netfilter: conntrack: reject nested ctnetlink master chains
  2026-10-06  7:57   ` netdev-bot+sashiko
@ 2026-10-07  1:33     ` Daehyeon Ko
  0 siblings, 0 replies; 11+ messages in thread
From: Daehyeon Ko @ 2026-10-07  1:33 UTC (permalink / raw)
  To: netdev-bot+sashiko
  Cc: pablo, fw, phil, netfilter-devel, coreteam, netdev, stable, kuba

Thanks for the review.  All three points are valid.

The nft ct expectation object can attach its internal helper to an
unconfirmed expected child, then arm the next expectation with that child
as master.  I confirmed a bounded four-entry chain through this path, and
current net retains the same unchecked transition.  An nft helper followed
by IPCTNL_MSG_EXP_NEW can repeat in the same way.  The H.323 Q.931
call-forwarding path also propagates its helper without a depth bound.

The CT_NEW compatibility concern is valid as well.  CTA_TUPLE_MASTER was
introduced for conntrackd replication, so rejecting nested helper-created
state changes that interface.

V2 is therefore incomplete and should not be applied.  I have reworked the
replacement to make master destruction stack-safe while preserving nested
expectation semantics.

pw-bot: cr

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

end of thread, other threads:[~2026-10-07  1:33 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-01 18:02 [PATCH net] netfilter: conntrack: avoid recursive master destruction Daehyeon Ko
2026-10-01 19:19 ` Florian Westphal
2026-10-02  5:39   ` Daehyeon Ko
2026-10-02  9:56     ` Pablo Neira Ayuso
2026-10-02  9:55   ` Pablo Neira Ayuso
2026-10-02  9:55 ` Pablo Neira Ayuso
2026-10-02 16:56 ` [PATCH v2 net] netfilter: conntrack: reject nested ctnetlink master chains Daehyeon Ko
2026-10-02 16:58   ` netdev-bot+sinfo
2026-10-02 17:35   ` Florian Westphal
2026-10-06  7:57   ` netdev-bot+sashiko
2026-10-07  1:33     ` Daehyeon Ko

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