netdev - [PATCH net 1/2] net/sched: act_mirred: Fix leak when redirecting to self on egress

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <20251230191814.213789-1-jhs@mojatatu.com>
Date: Tue, 30 Dec 2025 14:18:13 -0500
From: Jamal Hadi Salim <jhs@...atatu.com>
To: davem@...emloft.net,
	edumazet@...gle.com,
	kuba@...nel.org,
	pabeni@...hat.com,
	horms@...nel.org,
	andrew+netdev@...n.ch
Cc: netdev@...r.kernel.org,
	xiyou.wangcong@...il.com,
	jiri@...nulli.us,
	victor@...atatu.com,
	Jamal Hadi Salim <jhs@...atatu.com>
Subject: [PATCH net 1/2] net/sched: act_mirred: Fix leak when redirecting to self on egress

Whenever a mirred redirect to self on egress happens, mirred allocates a
new skb (skb_to_send). The loop to self check was done after that
allocation, but was not freeing the newly allocated skb, causing a leak.

Fix this by moving the if-statement to before the allocation of the new
skb.

The issue was found by running the accompanying tdc test in 2/2
with config kmemleak enabled.
After a few minutes the kmemleak thread ran and reported the leak coming from
mirred.

Fixes: 1d856251a009 ("net/sched: act_mirred: fix loop detection")
Signed-off-by: Jamal Hadi Salim <jhs@...atatu.com>
---
 net/sched/act_mirred.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
index 91c96cc625bd..c9653b76a4cf 100644
--- a/net/sched/act_mirred.c
+++ b/net/sched/act_mirred.c
@@ -266,6 +266,17 @@ static int tcf_mirred_to_dev(struct sk_buff *skb, struct tcf_mirred *m,
 		goto err_cant_do;
 	}
 
+	want_ingress = tcf_mirred_act_wants_ingress(m_eaction);
+
+	if (dev == skb->dev && want_ingress == at_ingress) {
+		pr_notice_once("tc mirred: Loop (%s:%s --> %s:%s)\n",
+			       netdev_name(skb->dev),
+			       at_ingress ? "ingress" : "egress",
+			       netdev_name(dev),
+			       want_ingress ? "ingress" : "egress");
+		goto err_cant_do;
+	}
+
 	/* we could easily avoid the clone only if called by ingress and clsact;
 	 * since we can't easily detect the clsact caller, skip clone only for
 	 * ingress - that covers the TC S/W datapath.
@@ -279,17 +290,6 @@ static int tcf_mirred_to_dev(struct sk_buff *skb, struct tcf_mirred *m,
 			goto err_cant_do;
 	}
 
-	want_ingress = tcf_mirred_act_wants_ingress(m_eaction);
-
-	if (dev == skb->dev && want_ingress == at_ingress) {
-		pr_notice_once("tc mirred: Loop (%s:%s --> %s:%s)\n",
-			       netdev_name(skb->dev),
-			       at_ingress ? "ingress" : "egress",
-			       netdev_name(dev),
-			       want_ingress ? "ingress" : "egress");
-		goto err_cant_do;
-	}
-
 	/* All mirred/redirected skbs should clear previous ct info */
 	nf_reset_ct(skb_to_send);
 	if (want_ingress && !at_ingress) /* drop dst for egress -> ingress */
-- 
2.52.0