Commit 98923537 authored by Jamal Hadi Salim's avatar Jamal Hadi Salim Committed by Jakub Kicinski
Browse files

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: 1d856251 ("net/sched: act_mirred: fix loop detection")
Signed-off-by: default avatarJamal Hadi Salim <jhs@mojatatu.com>
Link: https://patch.msgid.link/20260101135608.253079-2-jhs@mojatatu.com


Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parent d96bfb68

net/sched/act_mirred.c

+12 −12
Original line number Diff line number Diff line
@@ -266,11 +266,22 @@ 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);



	at_ingress = skb_at_tc_ingress(skb);

	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.

	 */

	at_ingress = skb_at_tc_ingress(skb);

	dont_clone = skb_at_tc_ingress(skb) && is_redirect &&

		tcf_mirred_can_reinsert(retval);

	if (!dont_clone) {
@@ -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 */