Merge branch 'net-sched-validate-the-control-action-with-all-the-other-parameters'

Davide Caratti says:

====================
net/sched: validate the control action with all the other parameters

currently, the kernel checks for bad values of the control action in
tcf_action_init_1(), after a successful call to the action's init()
function. When the control action is 'goto chain', this causes two
undesired behaviors:

1. "misconfigured action after replace that causes kernel crash":
   if users replace a valid TC action with another one having invalid
   control action, all the new configuration data (including the bad
   control action) are applied successfully, even if the kernel returned
   an error. As a consequence, it's possible to trigger a NULL pointer
   dereference in the traffic path of every TC action (1), replacing the
   control action with 'goto chain x', when chain <x> doesn't exist.

2. "refcount leak that makes kmemleak complain"
   when a valid 'goto chain' action is overwritten with another action,
   the kernel forgets to decrease refcounts in the chain.

The above problems can be fixed if we validate the control action in each
action's init() function, the same way as we are already doing for all the
other configuration parameters.
Now that chains can be released after an action is replaced, we need to
care about concurrent access of 'goto_chain' pointer: ensure we access it
through RCU, like we did with most action-specific configuration parameters.

- Patch 1 removes the wrong checks and provides functions that can be
  used to properly validate control actions in  individual actions
- Patch 2 to 16 fix individual actions, and add TDC selftest code to
  verify the correct behavior (2)
- Patch 17 and 18 fix concurrent access issues on 'goto_chain', that can be
  observed after the chain refcount leak is fixed.

Changes since v1:
- reword the cover letter
- condense the extack message in case tc_action_check_ctrlact() is called
  with invalid parameters.
- add tcf_action_set_ctrlact() to avoid code duplication an make the
  RCU-ification of 'goto_chain' easier.
- fix errors in act_ife, act_simple, act_skbedit, and avoid useless 'goto
  end' in act_connmark, thanks a lot to Vlad Buslov.
- avoid dereferencing 'goto_chain' in tcf_gact_goto_chain_index(), so
  we don't have to care about the grace period there.
- let actions respect the grace period when they release chains, thanks
  to Cong Wang and Vlad Buslov.

Changes since RFC:
- include a fix for all TC actions
- add a selftest for each TC action
- squash fix for refcount leaks into a single patch, the first in the
  series, thanks to Cong Wang
- ensure that chain refcount is released without tcfa_lock held, thanks
  to Vlad Buslov

Notes:
(1) act_ipt didn't need any fix, as the control action is constantly equal
    to TC_ACT_OK.
(2) the selftest for act_simple fails because userspace tc backend for
    'simple' does not parse the control action correctly (and hardcodes it
    to TC_ACT_PIPE).
====================

Signed-off-by: David S. Miller <davem@davemloft.net>

1 files changed, 7 insertions, 2 deletions

diff --git a/include/net/act_api.h b/include/net/act_api.h
index c745e9ccfab2..c61a1bf4e3de 100644
--- a/include/net/act_api.h
+++ b/include/net/act_api.h
@@ -39,7 +39,7 @@ struct tc_action {
 	struct gnet_stats_basic_cpu __percpu *cpu_bstats_hw;
 	struct gnet_stats_queue __percpu *cpu_qstats;
 	struct tc_cookie	__rcu *act_cookie;
-	struct tcf_chain	*goto_chain;
+	struct tcf_chain	__rcu *goto_chain;
 };
 #define tcf_index	common.tcfa_index
 #define tcf_refcnt	common.tcfa_refcnt
@@ -90,7 +90,7 @@ struct tc_action_ops {
 	int     (*lookup)(struct net *net, struct tc_action **a, u32 index);
 	int     (*init)(struct net *net, struct nlattr *nla,
 			struct nlattr *est, struct tc_action **act, int ovr,
-			int bind, bool rtnl_held,
+			int bind, bool rtnl_held, struct tcf_proto *tp,
 			struct netlink_ext_ack *extack);
 	int     (*walk)(struct net *, struct sk_buff *,
 			struct netlink_callback *, int,
@@ -181,6 +181,11 @@ int tcf_action_dump_old(struct sk_buff *skb, struct tc_action *a, int, int);
 int tcf_action_dump_1(struct sk_buff *skb, struct tc_action *a, int, int);
 int tcf_action_copy_stats(struct sk_buff *, struct tc_action *, int);
 
+int tcf_action_check_ctrlact(int action, struct tcf_proto *tp,
+			     struct tcf_chain **handle,
+			     struct netlink_ext_ack *newchain);
+struct tcf_chain *tcf_action_set_ctrlact(struct tc_action *a, int action,
+					 struct tcf_chain *newchain);
 #endif /* CONFIG_NET_CLS_ACT */
 
 static inline void tcf_action_stats_update(struct tc_action *a, u64 bytes,