* [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-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
* 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-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
* [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