Currently, when initializing an action, the user-space can specify
and use arbitrary values for the tcfa_action field. If the value
is unknown by the kernel, is implicitly threaded as TC_ACT_UNSPEC.
This change explicitly checks for unknown values at action creation
time, and explicitly convert them to TC_ACT_UNSPEC. No functional
changes are introduced, but this will allow introducing tcfa_action
values not exposed to user-space in a later patch.
Note: we can't use the above to hide TC_ACT_REDIRECT from user-space,
as the latter is already part of uAPI.
Signed-off-by: Paolo Abeni <pab...@redhat.com>
---
include/uapi/linux/pkt_cls.h | 6 ++++--
net/sched/act_api.c | 10 +++++++++-
2 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/include/uapi/linux/pkt_cls.h b/include/uapi/linux/pkt_cls.h
index c4262d911596..c8a24861d4c8 100644
--- a/include/uapi/linux/pkt_cls.h
+++ b/include/uapi/linux/pkt_cls.h
@@ -45,6 +45,7 @@ enum {
* the skb and act like everything
* is alright.
*/
+#define TC_ACT_VALUE_MAX TC_ACT_TRAP
/* There is a special kind of actions called "extended actions",
* which need a value parameter. These have a local opcode located in
@@ -55,11 +56,12 @@ enum {
#define __TC_ACT_EXT_SHIFT 28
#define __TC_ACT_EXT(local) ((local) << __TC_ACT_EXT_SHIFT)
#define TC_ACT_EXT_VAL_MASK ((1 << __TC_ACT_EXT_SHIFT) - 1)
-#define TC_ACT_EXT_CMP(combined, opcode) \
- (((combined) & (~TC_ACT_EXT_VAL_MASK)) == opcode)
+#define TC_ACT_EXT_OPCODE(combined) ((combined) & (~TC_ACT_EXT_VAL_MASK))
+#define TC_ACT_EXT_CMP(combined, opcode) (TC_ACT_EXT_OPCODE(combined) ==
opcode)
#define TC_ACT_JUMP __TC_ACT_EXT(1)
#define TC_ACT_GOTO_CHAIN __TC_ACT_EXT(2)
+#define TC_ACT_EXT_OPCODE_MAX TC_ACT_GOTO_CHAIN
/* Action type identifiers*/
enum {
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index 24b5534967fe..5044f4809b37 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -798,6 +798,7 @@ struct tc_action *tcf_action_init_1(struct net *net, struct
tcf_proto *tp,
char act_name[IFNAMSIZ];
struct nlattr *tb[TCA_ACT_MAX + 1];
struct nlattr *kind;
+ int opcode;
int err;
if (name == NULL) {
@@ -884,7 +885,8 @@ struct tc_action *tcf_action_init_1(struct net *net, struct
tcf_proto *tp,
if (err != ACT_P_CREATED)
module_put(a_o->owner);
- if (TC_ACT_EXT_CMP(a->tcfa_action, TC_ACT_GOTO_CHAIN)) {
+ opcode = TC_ACT_EXT_OPCODE(a->tcfa_action);
+ if (opcode == TC_ACT_GOTO_CHAIN) {
err = tcf_action_goto_chain_init(a, tp);
if (err) {
struct tc_action *actions[] = { a, NULL };
@@ -898,6 +900,12 @@ struct tc_action *tcf_action_init_1(struct net *net,
struct tcf_proto *tp,
if (a->tcfa_action == TC_ACT_REDIRECT) {
net_warn_ratelimited("TC_ACT_REDIRECT can't be used directly");
a->tcfa_action = TC_ACT_UNSPEC;
+ } else if ((!opcode && a->tcfa_action > TC_ACT_VALUE_MAX) ||
+ (opcode && opcode > TC_ACT_EXT_OPCODE_MAX &&
+ a->tcfa_action != TC_ACT_UNSPEC)) {
+ net_warn_ratelimited("invalid %d action value",
+ a->tcfa_action);
+ a->tcfa_action = TC_ACT_UNSPEC;
}
return a;
--
2.17.1
