[PATCH net-next 2/2] net/sched: act_gate: cap the number of scheduling entries

[Jamal Hadi Salim <jhs@mojatatu.com>]

parse_gate_list() accepts _any number_ of TCA_GATE_ONE_ENTRY elements.
The only bound/cap is the enclosing nlattr u16 nla_len (~5460
minimal 12-byte entries). Each entry is a separate GFP_ATOMIC
allocation, and the dump of a large list grows one action's reply well
past NLMSG_GOODSIZE.

The bugs:
1. A single gate can make tcf_dump_walker() silently truncate
"tc actions ls"
2. Lack of capping has been demonstrated to be exploitable to
create an OOM through amplification of RTM_GETACTION replies.

So we need to provide an upper bound cap of TCA_GATE_ONE_ENTRY elements.

Some context:
The gate action offloads through FLOW_ACTION_GATE. The offload
interface passes the priority, base time, cycle time, cycle-time
extension, the complete entry count and an array holding each entry's
gate state, interval, IPV and maximum-octet value. The in-tree SJA1105
DSA driver forwards that complete list to sja1105_vl_gate() without any
entry cap check, and composes every supplied entry into the hardware
schedule table.

Act gate challenges:
SJA1105 and SJA1110 are two device families in the one sja1105 driver.
They expose different schedule-table capacities: SJA1105 provides up to
1024 schedule entries and SJA1110 up to 4096. These are total table
capacities shared with other schedules, so they are upper bounds, not a
guaranteed per-action budget. Software gate actions with _a lot more_
entries install and serialize today, so a low cap would reject working
configurations and would look like a UAPI breakage; but an infinite size
of entries is a bogus choice; 65 entries is the boundary configuration
verified during this work.

Netlink Challenges:
Netlink attributes record their total size, including the four-byte
header, in a 16-bit nla_len, so a nested attribute can describe at most
65532 bytes. A minimal accepted input entry is 12 bytes, which puts the
max we can fit at roughly 5460 entries; a dumped entry occupies 36 bytes,
or 40 bytes when it carries the gate-open flag.
1024 entries therefore serialize to 36868-40964 bytes, which fits
inside one nested attribute, while a 4096-entry schedule would need
roughly 144-160 KiB which _cannot fit_ at the moment due to the 16bit
length.

Note, Note: This represents challenges primarily with netlink.

The Cap:
Cap the list at 1024 entries. This value is based on the existing
SJA1105 schedule-table capacity and preserves the verified 65-entry
configuration while providing a substantial headroom, and keeps the
complete gate list inside the current 16-bit TLV format. It
deliberately declines to expose SJA1110's 1025-4096 range through
an interface that cannot serialize it, and deliberately rejects
software-only schedules above 1024.

This is an operational cap on a configuration the uAPI accepted before,
so it is net-next hardening; if we made this a stable backport then
it would start rejecting gate configurations that install today (even
though those settings would be totally bogus).

Caveat Emptor:
While this patch fixes the binding of per-action storage and the dump
amplification; it does not by itself fix tcf_dump_walker() truncation
whose skb is smaller than the reply for such an action (for example the
kernel clamps a dump skb via netlink_recvmsg()/netlink_dump() to
SKB_WITH_OVERHEAD(32768), so "tc actions ls" is already truncated below
the 1024 this cap admits, and you cannot dump the top of the range
without changing iproute2 code)
It also does not remove generic netlink receive-queue amplification.
A companion iproute2 change can enumerate actions with a terse dump and
then fetch each one with an indexed RTM_GETACTION.

Conditions to recreate the bug:
Cap net admin with CONFIG_NET_ACT_GATE=y. Install a gate
action with 1025 or more minimal TCA_GATE_ONE_ENTRY elements over a raw
netlink RTM_NEWACTION request. The cap rejects the list with -E2BIG and
an extack naming the limit; 1024 entries install and an indexed
RTM_GETACTION returns a well-formed reply.

Reviewed-by: Victor Nogueira <victor@mojatatu.com>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
 net/sched/act_gate.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/net/sched/act_gate.c b/net/sched/act_gate.c
index 14801c604bd9..d8e8c2356bd5 100644
--- a/net/sched/act_gate.c
+++ b/net/sched/act_gate.c
@@ -18,6 +18,17 @@
 
 static struct tc_action_ops act_gate_ops;
 
+/* A netlink attribute records its total length, including the 4-byte
+ * header, in a u16 nla_len, so one nested attribute can describe at most
+ * 65532 bytes. tcf_gate_dump() emits each schedule entry as 36 bytes, or
+ * 40 with the gate-open flag, so 1024 entries serialize to 36868-40964
+ * bytes and still fit a single TCA_GATE_ENTRY_LIST nest, while 4096 would
+ * need ~144-160 KiB and cannot be represented. 1024 is also the schedule
+ * table capacity of SJA1105 (SJA1110 provides 4096). Cap the entry list
+ * at 1024.
+ */
+#define GATE_ENTRIES_MAX	1024
+
 static ktime_t gate_get_time(struct tcf_gate *gact)
 {
 	ktime_t mono = ktime_get();
@@ -277,6 +288,14 @@ static int parse_gate_list(struct nlattr *list_attr,
 			continue;
 		}
 
+		if (i >= GATE_ENTRIES_MAX) {
+			NL_SET_ERR_MSG_FMT(extack,
+					   "Too many schedule entries, at most %u are supported",
+					   GATE_ENTRIES_MAX);
+			err = -E2BIG;
+			goto release_list;
+		}
+
 		entry = kzalloc_obj(*entry, GFP_ATOMIC);
 		if (!entry) {
 			NL_SET_ERR_MSG(extack, "Not enough memory for entry");
-- 
2.43.0