Skip to content

Commit 91ba554

Browse files
tohojoNipaLocal
authored and
NipaLocal
committed
tc: Return an error if filters try to attach too many actions
While developing the fix for the buffer sizing issue in [0], I noticed that the kernel will happily accept a long list of actions for a filter, and then just silently truncate that list down to a maximum of 32 actions. That seems less than ideal, so this patch changes the action parsing to return an error message and refuse to create the filter in this case. This results in an error like: # ip link add type veth # tc qdisc replace dev veth0 root handle 1: fq_codel # tc -echo filter add dev veth0 parent 1: u32 match u32 0 0 $(for i in $(seq 33); do echo action pedit munge ip dport set 22; done) Error: Only 32 actions supported per filter. We have an error talking to the kernel Instead of just creating a filter with 32 actions and dropping the last one. This is obviously a change in UAPI. But seeing as creating more than 32 filters has never actually *worked*, it seems that returning an explicit error is better, and any use cases that get broken by this were already broken just in more subtle ways. [0] https://lore.kernel.org/r/[email protected] Signed-off-by: Toke Høiland-Jørgensen <[email protected]> Signed-off-by: NipaLocal <nipa@local>
1 parent fa5a39a commit 91ba554

File tree

1 file changed

+14
-2
lines changed

1 file changed

+14
-2
lines changed

net/sched/act_api.c

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1461,17 +1461,29 @@ int tcf_action_init(struct net *net, struct tcf_proto *tp, struct nlattr *nla,
14611461
struct netlink_ext_ack *extack)
14621462
{
14631463
struct tc_action_ops *ops[TCA_ACT_MAX_PRIO] = {};
1464-
struct nlattr *tb[TCA_ACT_MAX_PRIO + 1];
1464+
struct nlattr *tb[TCA_ACT_MAX_PRIO + 2];
14651465
struct tc_action *act;
14661466
size_t sz = 0;
14671467
int err;
14681468
int i;
14691469

1470-
err = nla_parse_nested_deprecated(tb, TCA_ACT_MAX_PRIO, nla, NULL,
1470+
err = nla_parse_nested_deprecated(tb, TCA_ACT_MAX_PRIO + 1, nla, NULL,
14711471
extack);
14721472
if (err < 0)
14731473
return err;
14741474

1475+
/* The nested attributes are parsed as types, but they are really an
1476+
* array of actions. So we parse one more than we can handle, and return
1477+
* an error if the last one is set (as that indicates that the request
1478+
* contained more than the maximum number of actions).
1479+
*/
1480+
if (tb[TCA_ACT_MAX_PRIO + 1]) {
1481+
NL_SET_ERR_MSG_FMT(extack,
1482+
"Only %d actions supported per filter",
1483+
TCA_ACT_MAX_PRIO);
1484+
return -EINVAL;
1485+
}
1486+
14751487
for (i = 1; i <= TCA_ACT_MAX_PRIO && tb[i]; i++) {
14761488
struct tc_action_ops *a_o;
14771489

0 commit comments

Comments
 (0)