Netdev List
 help / color / mirror / Atom feed
* [PATCH net] net/sched: act_ct: fix nf_connlabels leak on two error paths
@ 2026-06-17 21:57 Michael Bommarito
  2026-06-18 10:41 ` Jamal Hadi Salim
  2026-06-21 22:10 ` patchwork-bot+netdevbpf
  0 siblings, 2 replies; 3+ messages in thread
From: Michael Bommarito @ 2026-06-17 21:57 UTC (permalink / raw)
  To: Jamal Hadi Salim, Jiri Pirko
  Cc: Pablo Neira Ayuso, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	netdev, linux-kernel

tcf_ct_fill_params() calls nf_connlabels_get() (setting put_labels) when
TCA_CT_LABELS is present, but two later error sites use a bare return
instead of "goto err", skipping the err: nf_connlabels_put() cleanup.
They also precede the "p->put_labels = put_labels" assignment, so the
tcf_ct_params_free() fallback does not release the count either. Each
failed RTM_NEWACTION on these paths leaks one nf_connlabels reference:
net->ct.labels_used is incremented and never released. The action is
reachable with CAP_NET_ADMIN over the netns, i.e. from an unprivileged
user namespace on default-userns kernels.

Impact: an unprivileged user with CAP_NET_ADMIN over a network namespace
(e.g. via user namespaces) leaks one nf_connlabels reference per failed
RTM_NEWACTION on the two error paths; net->ct.labels_used is never
released.

The err: label is safe to reach from both sites: p->tmpl is still NULL
there (kzalloc'd, not yet assigned) and nf_ct_put(NULL) is a no-op, so
no inline release is needed.

Fixes: 70f06c115bcc ("sched: act_ct: switch to per-action label counting")
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
---
Testing: refcount/counter leak (CWE-772); no sanitizer for this class, so
the oracle is the nf_connlabels accounting counter net->ct.labels_used.

Reproduction (UML, before/after, same trigger): CONFIG_NET_ACT_CT=y,
NF_CONNTRACK_LABELS=y, NF_CONNTRACK_ZONES=n (forces the zone-disabled
path). A raw RTM_NEWACTION trigger adds "action ct label 0x1/0x1 zone 1"
20 times; each returns -EOPNOTSUPP.
  stock:   net->ct.labels_used climbs 1,2,...,20 (get, then bare return,
           no put) -- 20 leaked counts, never recovered.
  patched: counter stays balanced (get then goto err -> put); baseline.
Control: the same loop without "label" (no nf_connlabels_get) leaves the
counter unchanged on both trees -- the trigger reached the labels path and
the synthesis is not itself the cause.

Conditions: reachable via RTM_NEWACTION with CAP_NET_ADMIN over the netns,
i.e. an unprivileged user in a fresh user+net namespace on default-userns
distros. The easy path needs CONFIG_NF_CONNTRACK_ZONES=n; the
nf_ct_tmpl_alloc ENOMEM path leaks on any config under memory pressure.

Mitigations: restrict unprivileged user namespaces; otherwise none short of
the fix. Harness (trigger.c, init) available on request.

 net/sched/act_ct.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/net/sched/act_ct.c b/net/sched/act_ct.c
index 6158e13c98d35..f5866a364a74a 100644
--- a/net/sched/act_ct.c
+++ b/net/sched/act_ct.c
@@ -1295,7 +1295,8 @@ static int tcf_ct_fill_params(struct net *net,
 	if (tb[TCA_CT_ZONE]) {
 		if (!IS_ENABLED(CONFIG_NF_CONNTRACK_ZONES)) {
 			NL_SET_ERR_MSG_MOD(extack, "Conntrack zones isn't enabled.");
-			return -EOPNOTSUPP;
+			err = -EOPNOTSUPP;
+			goto err;
 		}
 
 		tcf_ct_set_key_val(tb,
@@ -1308,7 +1309,8 @@ static int tcf_ct_fill_params(struct net *net,
 	tmpl = nf_ct_tmpl_alloc(net, &zone, GFP_KERNEL);
 	if (!tmpl) {
 		NL_SET_ERR_MSG_MOD(extack, "Failed to allocate conntrack template");
-		return -ENOMEM;
+		err = -ENOMEM;
+		goto err;
 	}
 	p->tmpl = tmpl;
 	if (tb[TCA_CT_HELPER_NAME]) {
-- 
2.53.0


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

* Re: [PATCH net] net/sched: act_ct: fix nf_connlabels leak on two error paths
  2026-06-17 21:57 [PATCH net] net/sched: act_ct: fix nf_connlabels leak on two error paths Michael Bommarito
@ 2026-06-18 10:41 ` Jamal Hadi Salim
  2026-06-21 22:10 ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 3+ messages in thread
From: Jamal Hadi Salim @ 2026-06-18 10:41 UTC (permalink / raw)
  To: Michael Bommarito
  Cc: Jiri Pirko, Pablo Neira Ayuso, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, netdev, linux-kernel

On Wed, Jun 17, 2026 at 5:57 PM Michael Bommarito
<michael.bommarito@gmail.com> wrote:
>
> tcf_ct_fill_params() calls nf_connlabels_get() (setting put_labels) when
> TCA_CT_LABELS is present, but two later error sites use a bare return
> instead of "goto err", skipping the err: nf_connlabels_put() cleanup.
> They also precede the "p->put_labels = put_labels" assignment, so the
> tcf_ct_params_free() fallback does not release the count either. Each
> failed RTM_NEWACTION on these paths leaks one nf_connlabels reference:
> net->ct.labels_used is incremented and never released. The action is
> reachable with CAP_NET_ADMIN over the netns, i.e. from an unprivileged
> user namespace on default-userns kernels.
>
> Impact: an unprivileged user with CAP_NET_ADMIN over a network namespace
> (e.g. via user namespaces) leaks one nf_connlabels reference per failed
> RTM_NEWACTION on the two error paths; net->ct.labels_used is never
> released.
>
> The err: label is safe to reach from both sites: p->tmpl is still NULL
> there (kzalloc'd, not yet assigned) and nf_ct_put(NULL) is a no-op, so
> no inline release is needed.
>
> Fixes: 70f06c115bcc ("sched: act_ct: switch to per-action label counting")
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
> ---
> Testing: refcount/counter leak (CWE-772); no sanitizer for this class, so
> the oracle is the nf_connlabels accounting counter net->ct.labels_used.
>
> Reproduction (UML, before/after, same trigger): CONFIG_NET_ACT_CT=y,
> NF_CONNTRACK_LABELS=y, NF_CONNTRACK_ZONES=n (forces the zone-disabled
> path). A raw RTM_NEWACTION trigger adds "action ct label 0x1/0x1 zone 1"
> 20 times; each returns -EOPNOTSUPP.
>   stock:   net->ct.labels_used climbs 1,2,...,20 (get, then bare return,
>            no put) -- 20 leaked counts, never recovered.
>   patched: counter stays balanced (get then goto err -> put); baseline.
> Control: the same loop without "label" (no nf_connlabels_get) leaves the
> counter unchanged on both trees -- the trigger reached the labels path and
> the synthesis is not itself the cause.
>
> Conditions: reachable via RTM_NEWACTION with CAP_NET_ADMIN over the netns,
> i.e. an unprivileged user in a fresh user+net namespace on default-userns
> distros. The easy path needs CONFIG_NF_CONNTRACK_ZONES=n; the
> nf_ct_tmpl_alloc ENOMEM path leaks on any config under memory pressure.
>
> Mitigations: restrict unprivileged user namespaces; otherwise none short of
> the fix. Harness (trigger.c, init) available on request.
>
>  net/sched/act_ct.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/net/sched/act_ct.c b/net/sched/act_ct.c
> index 6158e13c98d35..f5866a364a74a 100644
> --- a/net/sched/act_ct.c
> +++ b/net/sched/act_ct.c
> @@ -1295,7 +1295,8 @@ static int tcf_ct_fill_params(struct net *net,
>         if (tb[TCA_CT_ZONE]) {
>                 if (!IS_ENABLED(CONFIG_NF_CONNTRACK_ZONES)) {
>                         NL_SET_ERR_MSG_MOD(extack, "Conntrack zones isn't enabled.");
> -                       return -EOPNOTSUPP;
> +                       err = -EOPNOTSUPP;
> +                       goto err;
>                 }
>
>                 tcf_ct_set_key_val(tb,
> @@ -1308,7 +1309,8 @@ static int tcf_ct_fill_params(struct net *net,
>         tmpl = nf_ct_tmpl_alloc(net, &zone, GFP_KERNEL);
>         if (!tmpl) {
>                 NL_SET_ERR_MSG_MOD(extack, "Failed to allocate conntrack template");
> -               return -ENOMEM;
> +               err = -ENOMEM;
> +               goto err;
>         }
>         p->tmpl = tmpl;
>         if (tb[TCA_CT_HELPER_NAME]) {

Looks sane to me.
Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>

cheers,
jamal
> --
> 2.53.0
>

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

* Re: [PATCH net] net/sched: act_ct: fix nf_connlabels leak on two error paths
  2026-06-17 21:57 [PATCH net] net/sched: act_ct: fix nf_connlabels leak on two error paths Michael Bommarito
  2026-06-18 10:41 ` Jamal Hadi Salim
@ 2026-06-21 22:10 ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 3+ messages in thread
From: patchwork-bot+netdevbpf @ 2026-06-21 22:10 UTC (permalink / raw)
  To: Michael Bommarito
  Cc: jhs, jiri, pablo, edumazet, kuba, pabeni, netdev, linux-kernel

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Wed, 17 Jun 2026 17:57:08 -0400 you wrote:
> tcf_ct_fill_params() calls nf_connlabels_get() (setting put_labels) when
> TCA_CT_LABELS is present, but two later error sites use a bare return
> instead of "goto err", skipping the err: nf_connlabels_put() cleanup.
> They also precede the "p->put_labels = put_labels" assignment, so the
> tcf_ct_params_free() fallback does not release the count either. Each
> failed RTM_NEWACTION on these paths leaks one nf_connlabels reference:
> net->ct.labels_used is incremented and never released. The action is
> reachable with CAP_NET_ADMIN over the netns, i.e. from an unprivileged
> user namespace on default-userns kernels.
> 
> [...]

Here is the summary with links:
  - [net] net/sched: act_ct: fix nf_connlabels leak on two error paths
    https://git.kernel.org/netdev/net/c/16e088016f38

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



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

end of thread, other threads:[~2026-06-21 22:10 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-17 21:57 [PATCH net] net/sched: act_ct: fix nf_connlabels leak on two error paths Michael Bommarito
2026-06-18 10:41 ` Jamal Hadi Salim
2026-06-21 22:10 ` patchwork-bot+netdevbpf

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