From patchwork Fri Feb 15 23:06:27 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Davide Caratti X-Patchwork-Id: 1043274 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming-netdev@ozlabs.org Delivered-To: patchwork-incoming-netdev@ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 441TRJ1gsbz9sDr for ; Sat, 16 Feb 2019 10:06:52 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2394091AbfBOXGv (ORCPT ); Fri, 15 Feb 2019 18:06:51 -0500 Received: from mx1.redhat.com ([209.132.183.28]:34410 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2393996AbfBOXGv (ORCPT ); Fri, 15 Feb 2019 18:06:51 -0500 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id D4AF2300ADB; Fri, 15 Feb 2019 23:06:50 +0000 (UTC) Received: from new-host.redhat.com (ovpn-204-38.brq.redhat.com [10.40.204.38]) by smtp.corp.redhat.com (Postfix) with ESMTP id EFBC460C69; Fri, 15 Feb 2019 23:06:48 +0000 (UTC) From: Davide Caratti To: Jamal Hadi Salim , Cong Wang , Jiri Pirko Cc: "David S. Miller" , Vlad Buslov , Paolo Abeni , netdev@vger.kernel.org Subject: [PATCH RFC 1/5] net/sched: fix refcount leak when 'goto_chain' is used Date: Sat, 16 Feb 2019 00:06:27 +0100 Message-Id: <4bb07d66c377864996d5e53356026e339b0b28d4.1550271080.git.dcaratti@redhat.com> In-Reply-To: References: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.29]); Fri, 15 Feb 2019 23:06:51 +0000 (UTC) Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org when replacing valid 'goto chain' actions with another valid 'goto chain' action, the kernel leaks chain->action_refcnt and chain->refcnt. Since we unconditionally take the refcount again, if the control action is a 'goto chain', we can just drop them after ->init() has ended successfully. Fixes: db50514f9a9c ("net: sched: add termination action to allow goto chain") Signed-off-by: Davide Caratti --- net/sched/act_api.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/net/sched/act_api.c b/net/sched/act_api.c index d4b8355737d8..91d79fac8cb2 100644 --- a/net/sched/act_api.c +++ b/net/sched/act_api.c @@ -907,6 +907,11 @@ 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 (a->goto_chain) { + tcf_action_goto_chain_fini(a); + a->goto_chain = NULL; + } + if (TC_ACT_EXT_CMP(a->tcfa_action, TC_ACT_GOTO_CHAIN)) { err = tcf_action_goto_chain_init(a, tp); if (err) { From patchwork Fri Feb 15 23:06:28 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Davide Caratti X-Patchwork-Id: 1043275 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming-netdev@ozlabs.org Delivered-To: patchwork-incoming-netdev@ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 441TRQ21Mdz9sDr for ; Sat, 16 Feb 2019 10:06:58 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2394163AbfBOXG5 (ORCPT ); Fri, 15 Feb 2019 18:06:57 -0500 Received: from mx1.redhat.com ([209.132.183.28]:48826 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2393969AbfBOXG4 (ORCPT ); Fri, 15 Feb 2019 18:06:56 -0500 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 01EC0A42BA; Fri, 15 Feb 2019 23:06:56 +0000 (UTC) Received: from new-host.redhat.com (ovpn-204-38.brq.redhat.com [10.40.204.38]) by smtp.corp.redhat.com (Postfix) with ESMTP id 8F7FF60C69; Fri, 15 Feb 2019 23:06:53 +0000 (UTC) From: Davide Caratti To: Jamal Hadi Salim , Cong Wang , Jiri Pirko Cc: "David S. Miller" , Vlad Buslov , Paolo Abeni , netdev@vger.kernel.org Subject: [PATCH RFC 2/5] net/sched: prepare TC actions to properly validate the control action Date: Sat, 16 Feb 2019 00:06:28 +0100 Message-Id: In-Reply-To: References: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.28]); Fri, 15 Feb 2019 23:06:56 +0000 (UTC) Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org - add tcf_action_check_ctrlact(), and pass a pointer to struct tcf_proto in each actions's init() function, to allow validation of 'goto chain' control action. - add tcf_action_set_ctrlact(), to set the control action, release the previous 'goto_chain' handle and replace it with the new one. Signed-off-by: Davide Caratti --- include/net/act_api.h | 9 +++++- net/sched/act_api.c | 58 ++++++++++++++++++++++++++++++++++++-- net/sched/act_bpf.c | 2 +- net/sched/act_connmark.c | 1 + net/sched/act_csum.c | 2 +- net/sched/act_gact.c | 2 +- net/sched/act_ife.c | 2 +- net/sched/act_ipt.c | 11 ++++---- net/sched/act_mirred.c | 1 + net/sched/act_nat.c | 3 +- net/sched/act_pedit.c | 2 +- net/sched/act_police.c | 1 + net/sched/act_sample.c | 2 +- net/sched/act_simple.c | 2 +- net/sched/act_skbedit.c | 1 + net/sched/act_skbmod.c | 1 + net/sched/act_tunnel_key.c | 1 + net/sched/act_vlan.c | 2 +- 18 files changed, 86 insertions(+), 17 deletions(-) diff --git a/include/net/act_api.h b/include/net/act_api.h index dbc795ec659e..55a8d44ac0c7 100644 --- a/include/net/act_api.h +++ b/include/net/act_api.h @@ -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,13 @@ 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 *extack); + +void tcf_action_set_ctrlact(struct tc_action *p, int action, + struct tcf_chain *goto_chain); + #endif /* CONFIG_NET_CLS_ACT */ static inline void tcf_action_stats_update(struct tc_action *a, u64 bytes, diff --git a/net/sched/act_api.c b/net/sched/act_api.c index 91d79fac8cb2..088b0d846bde 100644 --- a/net/sched/act_api.c +++ b/net/sched/act_api.c @@ -71,6 +71,60 @@ static void tcf_set_action_cookie(struct tc_cookie __rcu **old_cookie, call_rcu(&old->rcu, tcf_free_cookie_rcu); } +int tcf_action_check_ctrlact(int action, struct tcf_proto *tp, + struct tcf_chain **handle, + struct netlink_ext_ack *extack) +{ + int opcode = TC_ACT_EXT_OPCODE(action), ret = -EINVAL; + u32 chain_index; + + if (!opcode) + ret = action > TC_ACT_VALUE_MAX ? -EINVAL : 0; + else if (opcode <= TC_ACT_EXT_OPCODE_MAX || action == TC_ACT_UNSPEC) + ret = 0; + if (ret) { + NL_SET_ERR_MSG(extack, "invalid control action"); + goto end; + } + + if (TC_ACT_EXT_CMP(action, TC_ACT_GOTO_CHAIN)) { + chain_index = action & TC_ACT_EXT_VAL_MASK; + if (!tp) { + ret = -EINVAL; + NL_SET_ERR_MSG(extack, + "can't use goto_chain with NULL proto"); + goto end; + } + if (!handle) { + ret = -EINVAL; + NL_SET_ERR_MSG(extack, + "can't put goto_chain on NULL handle"); + goto end; + } + *handle = tcf_chain_get_by_act(tp->chain->block, chain_index); + if (!*handle) { + ret = -ENOMEM; + NL_SET_ERR_MSG(extack, + "can't allocate goto_chain handle"); + } + } +end: + return ret; +} +EXPORT_SYMBOL(tcf_action_check_ctrlact); + +void tcf_action_set_ctrlact(struct tc_action *p, int action, + struct tcf_chain *goto_chain) +{ + struct tcf_chain *old; + + old = xchg(&p->goto_chain, goto_chain); + if (old) + tcf_chain_put_by_act(old); + p->tcfa_action = action; +} +EXPORT_SYMBOL(tcf_action_set_ctrlact); + /* XXX: For standalone actions, we don't need a RCU grace period either, because * actions are always connected to filters and filters are already destroyed in * RCU callbacks, so after a RCU grace period actions are already disconnected @@ -890,10 +944,10 @@ struct tc_action *tcf_action_init_1(struct net *net, struct tcf_proto *tp, /* backward compatibility for policer */ if (name == NULL) err = a_o->init(net, tb[TCA_ACT_OPTIONS], est, &a, ovr, bind, - rtnl_held, extack); + rtnl_held, tp, extack); else err = a_o->init(net, nla, est, &a, ovr, bind, rtnl_held, - extack); + tp, extack); if (err < 0) goto err_mod; diff --git a/net/sched/act_bpf.c b/net/sched/act_bpf.c index c7633843e223..88a729bdab25 100644 --- a/net/sched/act_bpf.c +++ b/net/sched/act_bpf.c @@ -278,7 +278,7 @@ static void tcf_bpf_prog_fill_cfg(const struct tcf_bpf *prog, static int tcf_bpf_init(struct net *net, struct nlattr *nla, struct nlattr *est, struct tc_action **act, int replace, int bind, bool rtnl_held, - struct netlink_ext_ack *extack) + struct tcf_proto *tp, struct netlink_ext_ack *extack) { struct tc_action_net *tn = net_generic(net, bpf_net_id); struct nlattr *tb[TCA_ACT_BPF_MAX + 1]; diff --git a/net/sched/act_connmark.c b/net/sched/act_connmark.c index 8475913f2070..30c4c109c80c 100644 --- a/net/sched/act_connmark.c +++ b/net/sched/act_connmark.c @@ -97,6 +97,7 @@ static const struct nla_policy connmark_policy[TCA_CONNMARK_MAX + 1] = { static int tcf_connmark_init(struct net *net, struct nlattr *nla, struct nlattr *est, struct tc_action **a, int ovr, int bind, bool rtnl_held, + struct tcf_proto *tp, struct netlink_ext_ack *extack) { struct tc_action_net *tn = net_generic(net, connmark_net_id); diff --git a/net/sched/act_csum.c b/net/sched/act_csum.c index 3dc25b7806d7..1ae120c9ab02 100644 --- a/net/sched/act_csum.c +++ b/net/sched/act_csum.c @@ -46,7 +46,7 @@ static struct tc_action_ops act_csum_ops; static int tcf_csum_init(struct net *net, struct nlattr *nla, struct nlattr *est, struct tc_action **a, int ovr, - int bind, bool rtnl_held, + int bind, bool rtnl_held, struct tcf_proto *tp, struct netlink_ext_ack *extack) { struct tc_action_net *tn = net_generic(net, csum_net_id); diff --git a/net/sched/act_gact.c b/net/sched/act_gact.c index b61c20ebb314..727bbca9534b 100644 --- a/net/sched/act_gact.c +++ b/net/sched/act_gact.c @@ -57,7 +57,7 @@ static const struct nla_policy gact_policy[TCA_GACT_MAX + 1] = { static int tcf_gact_init(struct net *net, struct nlattr *nla, struct nlattr *est, struct tc_action **a, int ovr, int bind, bool rtnl_held, - struct netlink_ext_ack *extack) + struct tcf_proto *tp, struct netlink_ext_ack *extack) { struct tc_action_net *tn = net_generic(net, gact_net_id); struct nlattr *tb[TCA_GACT_MAX + 1]; diff --git a/net/sched/act_ife.c b/net/sched/act_ife.c index 30b63fa23ee2..9b2eb941e093 100644 --- a/net/sched/act_ife.c +++ b/net/sched/act_ife.c @@ -469,7 +469,7 @@ static int populate_metalist(struct tcf_ife_info *ife, struct nlattr **tb, static int tcf_ife_init(struct net *net, struct nlattr *nla, struct nlattr *est, struct tc_action **a, int ovr, int bind, bool rtnl_held, - struct netlink_ext_ack *extack) + struct tcf_proto *tp, struct netlink_ext_ack *extack) { struct tc_action_net *tn = net_generic(net, ife_net_id); struct nlattr *tb[TCA_IFE_MAX + 1]; diff --git a/net/sched/act_ipt.c b/net/sched/act_ipt.c index 8af6c11d2482..4b3c844ca988 100644 --- a/net/sched/act_ipt.c +++ b/net/sched/act_ipt.c @@ -97,7 +97,8 @@ static const struct nla_policy ipt_policy[TCA_IPT_MAX + 1] = { static int __tcf_ipt_init(struct net *net, unsigned int id, struct nlattr *nla, struct nlattr *est, struct tc_action **a, - const struct tc_action_ops *ops, int ovr, int bind) + const struct tc_action_ops *ops, int ovr, int bind, + struct tcf_proto *tp) { struct tc_action_net *tn = net_generic(net, id); struct nlattr *tb[TCA_IPT_MAX + 1]; @@ -206,20 +207,20 @@ static int __tcf_ipt_init(struct net *net, unsigned int id, struct nlattr *nla, static int tcf_ipt_init(struct net *net, struct nlattr *nla, struct nlattr *est, struct tc_action **a, int ovr, - int bind, bool rtnl_held, + int bind, bool rtnl_held, struct tcf_proto *tp, struct netlink_ext_ack *extack) { return __tcf_ipt_init(net, ipt_net_id, nla, est, a, &act_ipt_ops, ovr, - bind); + bind, tp); } static int tcf_xt_init(struct net *net, struct nlattr *nla, struct nlattr *est, struct tc_action **a, int ovr, - int bind, bool unlocked, + int bind, bool unlocked, struct tcf_proto *tp, struct netlink_ext_ack *extack) { return __tcf_ipt_init(net, xt_net_id, nla, est, a, &act_xt_ops, ovr, - bind); + bind, tp); } static int tcf_ipt_act(struct sk_buff *skb, const struct tc_action *a, diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c index c8cf4d10c435..69dda57f1097 100644 --- a/net/sched/act_mirred.c +++ b/net/sched/act_mirred.c @@ -94,6 +94,7 @@ static struct tc_action_ops act_mirred_ops; static int tcf_mirred_init(struct net *net, struct nlattr *nla, struct nlattr *est, struct tc_action **a, int ovr, int bind, bool rtnl_held, + struct tcf_proto *tp, struct netlink_ext_ack *extack) { struct tc_action_net *tn = net_generic(net, mirred_net_id); diff --git a/net/sched/act_nat.c b/net/sched/act_nat.c index c5c1e23add77..526c4c99bcce 100644 --- a/net/sched/act_nat.c +++ b/net/sched/act_nat.c @@ -38,7 +38,8 @@ static const struct nla_policy nat_policy[TCA_NAT_MAX + 1] = { static int tcf_nat_init(struct net *net, struct nlattr *nla, struct nlattr *est, struct tc_action **a, int ovr, int bind, - bool rtnl_held, struct netlink_ext_ack *extack) + bool rtnl_held, struct tcf_proto *tp, + struct netlink_ext_ack *extack) { struct tc_action_net *tn = net_generic(net, nat_net_id); struct nlattr *tb[TCA_NAT_MAX + 1]; diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c index 2b372a06b432..1c7a0db7b466 100644 --- a/net/sched/act_pedit.c +++ b/net/sched/act_pedit.c @@ -138,7 +138,7 @@ static int tcf_pedit_key_ex_dump(struct sk_buff *skb, static int tcf_pedit_init(struct net *net, struct nlattr *nla, struct nlattr *est, struct tc_action **a, int ovr, int bind, bool rtnl_held, - struct netlink_ext_ack *extack) + struct tcf_proto *tp, struct netlink_ext_ack *extack) { struct tc_action_net *tn = net_generic(net, pedit_net_id); struct nlattr *tb[TCA_PEDIT_MAX + 1]; diff --git a/net/sched/act_police.c b/net/sched/act_police.c index ec8ec55e0fe8..a444dd78a244 100644 --- a/net/sched/act_police.c +++ b/net/sched/act_police.c @@ -83,6 +83,7 @@ static const struct nla_policy police_policy[TCA_POLICE_MAX + 1] = { static int tcf_police_init(struct net *net, struct nlattr *nla, struct nlattr *est, struct tc_action **a, int ovr, int bind, bool rtnl_held, + struct tcf_proto *tp, struct netlink_ext_ack *extack) { int ret = 0, tcfp_result = TC_ACT_OK, err, size; diff --git a/net/sched/act_sample.c b/net/sched/act_sample.c index 1a0c682fd734..b2154edcb535 100644 --- a/net/sched/act_sample.c +++ b/net/sched/act_sample.c @@ -37,7 +37,7 @@ static const struct nla_policy sample_policy[TCA_SAMPLE_MAX + 1] = { static int tcf_sample_init(struct net *net, struct nlattr *nla, struct nlattr *est, struct tc_action **a, int ovr, - int bind, bool rtnl_held, + int bind, bool rtnl_held, struct tcf_proto *tp, struct netlink_ext_ack *extack) { struct tc_action_net *tn = net_generic(net, sample_net_id); diff --git a/net/sched/act_simple.c b/net/sched/act_simple.c index 902957beceb3..640ee5b785dc 100644 --- a/net/sched/act_simple.c +++ b/net/sched/act_simple.c @@ -80,7 +80,7 @@ static const struct nla_policy simple_policy[TCA_DEF_MAX + 1] = { static int tcf_simp_init(struct net *net, struct nlattr *nla, struct nlattr *est, struct tc_action **a, int ovr, int bind, bool rtnl_held, - struct netlink_ext_ack *extack) + struct tcf_proto *tp, struct netlink_ext_ack *extack) { struct tc_action_net *tn = net_generic(net, simp_net_id); struct nlattr *tb[TCA_DEF_MAX + 1]; diff --git a/net/sched/act_skbedit.c b/net/sched/act_skbedit.c index 64dba3708fce..9fc8cfdd35b1 100644 --- a/net/sched/act_skbedit.c +++ b/net/sched/act_skbedit.c @@ -96,6 +96,7 @@ static const struct nla_policy skbedit_policy[TCA_SKBEDIT_MAX + 1] = { static int tcf_skbedit_init(struct net *net, struct nlattr *nla, struct nlattr *est, struct tc_action **a, int ovr, int bind, bool rtnl_held, + struct tcf_proto *tp, struct netlink_ext_ack *extack) { struct tc_action_net *tn = net_generic(net, skbedit_net_id); diff --git a/net/sched/act_skbmod.c b/net/sched/act_skbmod.c index 59710a183bd3..35572d0e4576 100644 --- a/net/sched/act_skbmod.c +++ b/net/sched/act_skbmod.c @@ -82,6 +82,7 @@ static const struct nla_policy skbmod_policy[TCA_SKBMOD_MAX + 1] = { static int tcf_skbmod_init(struct net *net, struct nlattr *nla, struct nlattr *est, struct tc_action **a, int ovr, int bind, bool rtnl_held, + struct tcf_proto *tp, struct netlink_ext_ack *extack) { struct tc_action_net *tn = net_generic(net, skbmod_net_id); diff --git a/net/sched/act_tunnel_key.c b/net/sched/act_tunnel_key.c index 8b43fe0130f7..c4adc53e0fb4 100644 --- a/net/sched/act_tunnel_key.c +++ b/net/sched/act_tunnel_key.c @@ -209,6 +209,7 @@ static void tunnel_key_release_params(struct tcf_tunnel_key_params *p) static int tunnel_key_init(struct net *net, struct nlattr *nla, struct nlattr *est, struct tc_action **a, int ovr, int bind, bool rtnl_held, + struct tcf_proto *tp, struct netlink_ext_ack *extack) { struct tc_action_net *tn = net_generic(net, tunnel_key_net_id); diff --git a/net/sched/act_vlan.c b/net/sched/act_vlan.c index 93fdaf707313..80fd0e238a10 100644 --- a/net/sched/act_vlan.c +++ b/net/sched/act_vlan.c @@ -105,7 +105,7 @@ static const struct nla_policy vlan_policy[TCA_VLAN_MAX + 1] = { static int tcf_vlan_init(struct net *net, struct nlattr *nla, struct nlattr *est, struct tc_action **a, int ovr, int bind, bool rtnl_held, - struct netlink_ext_ack *extack) + struct tcf_proto *tp, struct netlink_ext_ack *extack) { struct tc_action_net *tn = net_generic(net, vlan_net_id); struct nlattr *tb[TCA_VLAN_MAX + 1]; From patchwork Fri Feb 15 23:06:29 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Davide Caratti X-Patchwork-Id: 1043276 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming-netdev@ozlabs.org Delivered-To: patchwork-incoming-netdev@ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 441TRT1vQWz9sML for ; Sat, 16 Feb 2019 10:07:01 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2394279AbfBOXHA (ORCPT ); Fri, 15 Feb 2019 18:07:00 -0500 Received: from mx1.redhat.com ([209.132.183.28]:54968 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2394215AbfBOXG7 (ORCPT ); Fri, 15 Feb 2019 18:06:59 -0500 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 8CE49C059B87; Fri, 15 Feb 2019 23:06:59 +0000 (UTC) Received: from new-host.redhat.com (ovpn-204-38.brq.redhat.com [10.40.204.38]) by smtp.corp.redhat.com (Postfix) with ESMTP id A7C6360C69; Fri, 15 Feb 2019 23:06:57 +0000 (UTC) From: Davide Caratti To: Jamal Hadi Salim , Cong Wang , Jiri Pirko Cc: "David S. Miller" , Vlad Buslov , Paolo Abeni , netdev@vger.kernel.org Subject: [PATCH RFC 3/5] net/sched: act_bpf: validate the control action inside init() Date: Sat, 16 Feb 2019 00:06:29 +0100 Message-Id: <7673fdb060c9b44656d7ef356c318db4ad197080.1550271080.git.dcaratti@redhat.com> In-Reply-To: References: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.32]); Fri, 15 Feb 2019 23:06:59 +0000 (UTC) Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Don't overwrite act_bpf data if the control control action is not valid, to prevent loosing the previous configuration in case validation failed. Not doing that caused NULL dereference in the data path if 'goto chain' is used. Tested with: # ./tdc.py -c bpf Fixes: db50514f9a9c ("net: sched: add termination action to allow goto chain") Fixes: 97763dc0f401 ("net_sched: reject unknown tcfa_action values") Signed-off-by: Davide Caratti --- net/sched/act_bpf.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/net/sched/act_bpf.c b/net/sched/act_bpf.c index 88a729bdab25..e2c2ba5faeb3 100644 --- a/net/sched/act_bpf.c +++ b/net/sched/act_bpf.c @@ -17,6 +17,7 @@ #include #include +#include #include #include @@ -282,6 +283,7 @@ static int tcf_bpf_init(struct net *net, struct nlattr *nla, { struct tc_action_net *tn = net_generic(net, bpf_net_id); struct nlattr *tb[TCA_ACT_BPF_MAX + 1]; + struct tcf_chain *newchain = NULL; struct tcf_bpf_cfg cfg, old; struct tc_act_bpf *parm; struct tcf_bpf *prog; @@ -323,6 +325,10 @@ static int tcf_bpf_init(struct net *net, struct nlattr *nla, return ret; } + ret = tcf_action_check_ctrlact(parm->action, tp, &newchain, extack); + if (ret < 0) + goto out; + is_bpf = tb[TCA_ACT_BPF_OPS_LEN] && tb[TCA_ACT_BPF_OPS]; is_ebpf = tb[TCA_ACT_BPF_FD]; @@ -350,7 +356,7 @@ static int tcf_bpf_init(struct net *net, struct nlattr *nla, if (cfg.bpf_num_ops) prog->bpf_num_ops = cfg.bpf_num_ops; - prog->tcf_action = parm->action; + tcf_action_set_ctrlact(*act, parm->action, newchain); rcu_assign_pointer(prog->filter, cfg.filter); spin_unlock_bh(&prog->tcf_lock); @@ -364,6 +370,8 @@ static int tcf_bpf_init(struct net *net, struct nlattr *nla, return res; out: + if (newchain) + tcf_chain_put_by_act(newchain); tcf_idr_release(*act, bind); return ret; From patchwork Fri Feb 15 23:06:30 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Davide Caratti X-Patchwork-Id: 1043277 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming-netdev@ozlabs.org Delivered-To: patchwork-incoming-netdev@ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 441TRX618Bz9sML for ; Sat, 16 Feb 2019 10:07:04 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2394306AbfBOXHD (ORCPT ); Fri, 15 Feb 2019 18:07:03 -0500 Received: from mx1.redhat.com ([209.132.183.28]:55714 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2393969AbfBOXHD (ORCPT ); Fri, 15 Feb 2019 18:07:03 -0500 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id CDF98C04AC51; Fri, 15 Feb 2019 23:07:02 +0000 (UTC) Received: from new-host.redhat.com (ovpn-204-38.brq.redhat.com [10.40.204.38]) by smtp.corp.redhat.com (Postfix) with ESMTP id EB01060C69; Fri, 15 Feb 2019 23:07:00 +0000 (UTC) From: Davide Caratti To: Jamal Hadi Salim , Cong Wang , Jiri Pirko Cc: "David S. Miller" , Vlad Buslov , Paolo Abeni , netdev@vger.kernel.org Subject: [PATCH RFC 4/5] net/sched: act_csum: validate the control action inside init() Date: Sat, 16 Feb 2019 00:06:30 +0100 Message-Id: <6a7083d426719911ffb2e7b13c573496d9a90f67.1550271080.git.dcaratti@redhat.com> In-Reply-To: References: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Fri, 15 Feb 2019 23:07:02 +0000 (UTC) Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Don't overwrite act_csum data if the control control action is not valid, to prevent loosing the previous configuration in case validation failed. Not doing that caused NULL dereference in the data path if 'goto chain' is used. Tested with: # ./tdc.py -c csum Fixes: db50514f9a9c ("net: sched: add termination action to allow goto chain") Fixes: 97763dc0f401 ("net_sched: reject unknown tcfa_action values") Signed-off-by: Davide Caratti --- net/sched/act_csum.c | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/net/sched/act_csum.c b/net/sched/act_csum.c index 1ae120c9ab02..bf0940156886 100644 --- a/net/sched/act_csum.c +++ b/net/sched/act_csum.c @@ -33,6 +33,7 @@ #include #include +#include #include #include @@ -52,6 +53,7 @@ static int tcf_csum_init(struct net *net, struct nlattr *nla, struct tc_action_net *tn = net_generic(net, csum_net_id); struct tcf_csum_params *params_new; struct nlattr *tb[TCA_CSUM_MAX + 1]; + struct tcf_chain *newchain = NULL; struct tc_csum *parm; struct tcf_csum *p; int ret = 0, err; @@ -87,17 +89,23 @@ static int tcf_csum_init(struct net *net, struct nlattr *nla, return err; } + err = tcf_action_check_ctrlact(parm->action, tp, &newchain, extack); + if (unlikely(err)) { + ret = err; + goto error; + } + p = to_tcf_csum(*a); params_new = kzalloc(sizeof(*params_new), GFP_KERNEL); if (unlikely(!params_new)) { - tcf_idr_release(*a, bind); - return -ENOMEM; + ret = -ENOMEM; + goto error; } params_new->update_flags = parm->update_flags; spin_lock_bh(&p->tcf_lock); - p->tcf_action = parm->action; + tcf_action_set_ctrlact(*a, parm->action, newchain); rcu_swap_protected(p->params, params_new, lockdep_is_held(&p->tcf_lock)); spin_unlock_bh(&p->tcf_lock); @@ -108,7 +116,13 @@ static int tcf_csum_init(struct net *net, struct nlattr *nla, if (ret == ACT_P_CREATED) tcf_idr_insert(tn, *a); +end: return ret; +error: + if (newchain) + tcf_chain_put_by_act(newchain); + tcf_idr_release(*a, bind); + goto end; } /** From patchwork Fri Feb 15 23:06:31 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Davide Caratti X-Patchwork-Id: 1043279 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming-netdev@ozlabs.org Delivered-To: patchwork-incoming-netdev@ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 441TRc6FVqz9sDr for ; Sat, 16 Feb 2019 10:07:08 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2394314AbfBOXHH (ORCPT ); Fri, 15 Feb 2019 18:07:07 -0500 Received: from mx1.redhat.com ([209.132.183.28]:40346 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2393969AbfBOXHH (ORCPT ); Fri, 15 Feb 2019 18:07:07 -0500 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 6FC75BE329; Fri, 15 Feb 2019 23:07:07 +0000 (UTC) Received: from new-host.redhat.com (ovpn-204-38.brq.redhat.com [10.40.204.38]) by smtp.corp.redhat.com (Postfix) with ESMTP id 9ACD960C6A; Fri, 15 Feb 2019 23:07:04 +0000 (UTC) From: Davide Caratti To: Jamal Hadi Salim , Cong Wang , Jiri Pirko Cc: "David S. Miller" , Vlad Buslov , Paolo Abeni , netdev@vger.kernel.org Subject: [PATCH RFC 5/5] net/sched: act_gact: validate the control action inside init() Date: Sat, 16 Feb 2019 00:06:31 +0100 Message-Id: <469e1ba2395d64b15e8d1b8bf08fcd281a3572e7.1550271080.git.dcaratti@redhat.com> In-Reply-To: References: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.30]); Fri, 15 Feb 2019 23:07:07 +0000 (UTC) Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org Don't overwrite act_gact data if the control control action is not valid, to prevent loosing the previous configuration in case validation failed. Not doing that caused NULL dereference in the data path if 'goto chain' is used. Tested with: # ./tdc.py -c gact Fixes: db50514f9a9c ("net: sched: add termination action to allow goto chain") Fixes: 97763dc0f401 ("net_sched: reject unknown tcfa_action values") Signed-off-by: Davide Caratti --- net/sched/act_gact.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/net/sched/act_gact.c b/net/sched/act_gact.c index 727bbca9534b..530e8bb8f94d 100644 --- a/net/sched/act_gact.c +++ b/net/sched/act_gact.c @@ -20,6 +20,7 @@ #include #include #include +#include #include #include @@ -61,6 +62,7 @@ static int tcf_gact_init(struct net *net, struct nlattr *nla, { struct tc_action_net *tn = net_generic(net, gact_net_id); struct nlattr *tb[TCA_GACT_MAX + 1]; + struct tcf_chain *newchain = NULL; struct tc_gact *parm; struct tcf_gact *gact; int ret = 0; @@ -116,10 +118,15 @@ static int tcf_gact_init(struct net *net, struct nlattr *nla, return err; } + err = tcf_action_check_ctrlact(parm->action, tp, &newchain, extack); + if (unlikely(err)) { + ret = err; + goto error; + } gact = to_gact(*a); spin_lock_bh(&gact->tcf_lock); - gact->tcf_action = parm->action; + tcf_action_set_ctrlact(*a, parm->action, newchain); #ifdef CONFIG_GACT_PROB if (p_parm) { gact->tcfg_paction = p_parm->paction; @@ -135,7 +142,13 @@ static int tcf_gact_init(struct net *net, struct nlattr *nla, if (ret == ACT_P_CREATED) tcf_idr_insert(tn, *a); +end: return ret; +error: + if (newchain) + tcf_chain_put_by_act(newchain); + tcf_idr_release(*a, bind); + goto end; } static int tcf_gact_act(struct sk_buff *skb, const struct tc_action *a,