| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20180528213804.GI3787@localhost.localdomain>
Date: Mon, 28 May 2018 18:38:04 -0300
From: Marcelo Ricardo Leitner <marcelo.leitner@...il.com>
To: Vlad Buslov <vladbu@...lanox.com>
Cc: jiri@...nulli.us, netdev@...r.kernel.org, jhs@...atatu.com,
xiyou.wangcong@...il.com, davem@...emloft.net, ast@...nel.org,
daniel@...earbox.net, kliteyn@...lanox.com
Subject: Re: [PATCH v3 03/11] net: sched: implement unlocked action init API
On Mon, May 28, 2018 at 12:17:21AM +0300, Vlad Buslov wrote:
> Add additional 'rtnl_held' argument to act API init functions. It is
> required to implement actions that need to release rtnl lock before loading
> kernel module and reacquire if afterwards.
>
> Signed-off-by: Vlad Buslov <vladbu@...lanox.com>
Reviewed-by: Marcelo Ricardo Leitner <marcelo.leitner@...il.com>
> ---
> Changes from V1 to V2:
> - Rename "unlocked" to "rtnl_held" for clarity.
>
> include/net/act_api.h | 6 ++++--
> net/sched/act_api.c | 18 +++++++++++-------
> net/sched/act_bpf.c | 3 ++-
> net/sched/act_connmark.c | 2 +-
> net/sched/act_csum.c | 3 ++-
> net/sched/act_gact.c | 3 ++-
> net/sched/act_ife.c | 3 ++-
> net/sched/act_ipt.c | 6 ++++--
> net/sched/act_mirred.c | 5 +++--
> net/sched/act_nat.c | 2 +-
> net/sched/act_pedit.c | 3 ++-
> net/sched/act_police.c | 2 +-
> net/sched/act_sample.c | 3 ++-
> net/sched/act_simple.c | 3 ++-
> net/sched/act_skbedit.c | 3 ++-
> net/sched/act_skbmod.c | 3 ++-
> net/sched/act_tunnel_key.c | 3 ++-
> net/sched/act_vlan.c | 3 ++-
> net/sched/cls_api.c | 5 +++--
> 19 files changed, 50 insertions(+), 29 deletions(-)
>
> diff --git a/include/net/act_api.h b/include/net/act_api.h
> index e634014605cb..888ff471bbf6 100644
> --- a/include/net/act_api.h
> +++ b/include/net/act_api.h
> @@ -92,7 +92,8 @@ struct tc_action_ops {
> struct netlink_ext_ack *extack);
> int (*init)(struct net *net, struct nlattr *nla,
> struct nlattr *est, struct tc_action **act, int ovr,
> - int bind, struct netlink_ext_ack *extack);
> + int bind, bool rtnl_held,
> + struct netlink_ext_ack *extack);
> int (*walk)(struct net *, struct sk_buff *,
> struct netlink_callback *, int,
> const struct tc_action_ops *,
> @@ -168,10 +169,11 @@ int tcf_action_exec(struct sk_buff *skb, struct tc_action **actions,
> int tcf_action_init(struct net *net, struct tcf_proto *tp, struct nlattr *nla,
> struct nlattr *est, char *name, int ovr, int bind,
> struct list_head *actions, size_t *attr_size,
> - struct netlink_ext_ack *extack);
> + bool rtnl_held, struct netlink_ext_ack *extack);
> struct tc_action *tcf_action_init_1(struct net *net, struct tcf_proto *tp,
> struct nlattr *nla, struct nlattr *est,
> char *name, int ovr, int bind,
> + bool rtnl_held,
> struct netlink_ext_ack *extack);
> int tcf_action_dump(struct sk_buff *skb, struct list_head *, int, int);
> int tcf_action_dump_old(struct sk_buff *skb, struct tc_action *a, int, int);
> diff --git a/net/sched/act_api.c b/net/sched/act_api.c
> index 4f064ecab882..256b0c93916c 100644
> --- a/net/sched/act_api.c
> +++ b/net/sched/act_api.c
> @@ -671,6 +671,7 @@ static struct tc_cookie *nla_memdup_cookie(struct nlattr **tb)
> struct tc_action *tcf_action_init_1(struct net *net, struct tcf_proto *tp,
> struct nlattr *nla, struct nlattr *est,
> char *name, int ovr, int bind,
> + bool rtnl_held,
> struct netlink_ext_ack *extack)
> {
> struct tc_action *a;
> @@ -721,9 +722,11 @@ struct tc_action *tcf_action_init_1(struct net *net, struct tcf_proto *tp,
> a_o = tc_lookup_action_n(act_name);
> if (a_o == NULL) {
> #ifdef CONFIG_MODULES
> - rtnl_unlock();
> + if (rtnl_held)
> + rtnl_unlock();
> request_module("act_%s", act_name);
> - rtnl_lock();
> + if (rtnl_held)
> + rtnl_lock();
>
> a_o = tc_lookup_action_n(act_name);
>
> @@ -746,9 +749,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,
> - extack);
> + rtnl_held, extack);
> else
> - err = a_o->init(net, nla, est, &a, ovr, bind, extack);
> + err = a_o->init(net, nla, est, &a, ovr, bind, rtnl_held,
> + extack);
> if (err < 0)
> goto err_mod;
>
> @@ -800,7 +804,7 @@ static void cleanup_a(struct list_head *actions, int ovr)
> int tcf_action_init(struct net *net, struct tcf_proto *tp, struct nlattr *nla,
> struct nlattr *est, char *name, int ovr, int bind,
> struct list_head *actions, size_t *attr_size,
> - struct netlink_ext_ack *extack)
> + bool rtnl_held, struct netlink_ext_ack *extack)
> {
> struct nlattr *tb[TCA_ACT_MAX_PRIO + 1];
> struct tc_action *act;
> @@ -814,7 +818,7 @@ int tcf_action_init(struct net *net, struct tcf_proto *tp, struct nlattr *nla,
>
> for (i = 1; i <= TCA_ACT_MAX_PRIO && tb[i]; i++) {
> act = tcf_action_init_1(net, tp, tb[i], est, name, ovr, bind,
> - extack);
> + rtnl_held, extack);
> if (IS_ERR(act)) {
> err = PTR_ERR(act);
> goto err;
> @@ -1173,7 +1177,7 @@ static int tcf_action_add(struct net *net, struct nlattr *nla,
> LIST_HEAD(actions);
>
> ret = tcf_action_init(net, NULL, nla, NULL, NULL, ovr, 0, &actions,
> - &attr_size, extack);
> + &attr_size, true, extack);
> if (ret)
> return ret;
>
> diff --git a/net/sched/act_bpf.c b/net/sched/act_bpf.c
> index 15a2a53cbde1..8ebf40a3506c 100644
> --- a/net/sched/act_bpf.c
> +++ b/net/sched/act_bpf.c
> @@ -276,7 +276,8 @@ 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, struct netlink_ext_ack *extack)
> + int replace, int bind, bool rtnl_held,
> + 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 188865034f9a..e3787aa0025a 100644
> --- a/net/sched/act_connmark.c
> +++ b/net/sched/act_connmark.c
> @@ -96,7 +96,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,
> + int ovr, int bind, bool rtnl_held,
> 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 da865f7b390a..334261943f9f 100644
> --- a/net/sched/act_csum.c
> +++ b/net/sched/act_csum.c
> @@ -46,7 +46,8 @@ 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, struct netlink_ext_ack *extack)
> + int bind, bool rtnl_held,
> + struct netlink_ext_ack *extack)
> {
> struct tc_action_net *tn = net_generic(net, csum_net_id);
> struct tcf_csum_params *params_old, *params_new;
> diff --git a/net/sched/act_gact.c b/net/sched/act_gact.c
> index ca83debd5a70..b4dfb2b4addc 100644
> --- a/net/sched/act_gact.c
> +++ b/net/sched/act_gact.c
> @@ -56,7 +56,8 @@ 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, struct netlink_ext_ack *extack)
> + int ovr, int bind, bool rtnl_held,
> + 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 706e84d6f912..3dccc4e1d378 100644
> --- a/net/sched/act_ife.c
> +++ b/net/sched/act_ife.c
> @@ -447,7 +447,8 @@ 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, struct netlink_ext_ack *extack)
> + int ovr, int bind, bool rtnl_held,
> + 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 7bce88dc11c9..9c21663a86a6 100644
> --- a/net/sched/act_ipt.c
> +++ b/net/sched/act_ipt.c
> @@ -196,7 +196,8 @@ 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, struct netlink_ext_ack *extack)
> + int bind, bool rtnl_held,
> + struct netlink_ext_ack *extack)
> {
> return __tcf_ipt_init(net, ipt_net_id, nla, est, a, &act_ipt_ops, ovr,
> bind);
> @@ -204,7 +205,8 @@ static int tcf_ipt_init(struct net *net, struct nlattr *nla,
>
> static int tcf_xt_init(struct net *net, struct nlattr *nla,
> struct nlattr *est, struct tc_action **a, int ovr,
> - int bind, struct netlink_ext_ack *extack)
> + int bind, bool unlocked,
> + struct netlink_ext_ack *extack)
> {
> return __tcf_ipt_init(net, xt_net_id, nla, est, a, &act_xt_ops, ovr,
> bind);
> diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
> index 82a8bdd67c47..5434f08f2eb7 100644
> --- a/net/sched/act_mirred.c
> +++ b/net/sched/act_mirred.c
> @@ -68,8 +68,9 @@ static unsigned int mirred_net_id;
> 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, struct netlink_ext_ack *extack)
> + struct nlattr *est, struct tc_action **a,
> + int ovr, int bind, bool rtnl_held,
> + struct netlink_ext_ack *extack)
> {
> struct tc_action_net *tn = net_generic(net, mirred_net_id);
> struct nlattr *tb[TCA_MIRRED_MAX + 1];
> diff --git a/net/sched/act_nat.c b/net/sched/act_nat.c
> index 457c2ae3de46..e6487ad1e4a8 100644
> --- a/net/sched/act_nat.c
> +++ b/net/sched/act_nat.c
> @@ -38,7 +38,7 @@ 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,
> - struct netlink_ext_ack *extack)
> + bool rtnl_held, 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 0102b2935fdb..7c9a3f24edba 100644
> --- a/net/sched/act_pedit.c
> +++ b/net/sched/act_pedit.c
> @@ -132,7 +132,8 @@ 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, struct netlink_ext_ack *extack)
> + int ovr, int bind, bool rtnl_held,
> + 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 a789b8060968..0e1c2fb0ebea 100644
> --- a/net/sched/act_police.c
> +++ b/net/sched/act_police.c
> @@ -75,7 +75,7 @@ static const struct nla_policy police_policy[TCA_POLICE_MAX + 1] = {
>
> static int tcf_act_police_init(struct net *net, struct nlattr *nla,
> struct nlattr *est, struct tc_action **a,
> - int ovr, int bind,
> + int ovr, int bind, bool rtnl_held,
> struct netlink_ext_ack *extack)
> {
> int ret = 0, err;
> diff --git a/net/sched/act_sample.c b/net/sched/act_sample.c
> index 4a46978db092..316fc645595d 100644
> --- a/net/sched/act_sample.c
> +++ b/net/sched/act_sample.c
> @@ -37,7 +37,8 @@ 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, struct netlink_ext_ack *extack)
> + int bind, bool rtnl_held,
> + struct netlink_ext_ack *extack)
> {
> struct tc_action_net *tn = net_generic(net, sample_net_id);
> struct nlattr *tb[TCA_SAMPLE_MAX + 1];
> diff --git a/net/sched/act_simple.c b/net/sched/act_simple.c
> index 95d5985b8d67..23fa893ea092 100644
> --- a/net/sched/act_simple.c
> +++ b/net/sched/act_simple.c
> @@ -79,7 +79,8 @@ 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, struct netlink_ext_ack *extack)
> + int ovr, int bind, bool rtnl_held,
> + 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 d418ec3b0ab9..85ed9d603dc1 100644
> --- a/net/sched/act_skbedit.c
> +++ b/net/sched/act_skbedit.c
> @@ -66,7 +66,8 @@ 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, struct netlink_ext_ack *extack)
> + int ovr, int bind, bool rtnl_held,
> + struct netlink_ext_ack *extack)
> {
> struct tc_action_net *tn = net_generic(net, skbedit_net_id);
> struct nlattr *tb[TCA_SKBEDIT_MAX + 1];
> diff --git a/net/sched/act_skbmod.c b/net/sched/act_skbmod.c
> index ff90d720eda3..026d6f58eda1 100644
> --- a/net/sched/act_skbmod.c
> +++ b/net/sched/act_skbmod.c
> @@ -84,7 +84,8 @@ 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, struct netlink_ext_ack *extack)
> + int ovr, int bind, bool rtnl_held,
> + struct netlink_ext_ack *extack)
> {
> struct tc_action_net *tn = net_generic(net, skbmod_net_id);
> struct nlattr *tb[TCA_SKBMOD_MAX + 1];
> diff --git a/net/sched/act_tunnel_key.c b/net/sched/act_tunnel_key.c
> index c6e50695414b..ed698fcb0e5a 100644
> --- a/net/sched/act_tunnel_key.c
> +++ b/net/sched/act_tunnel_key.c
> @@ -70,7 +70,8 @@ static const struct nla_policy tunnel_key_policy[TCA_TUNNEL_KEY_MAX + 1] = {
>
> static int tunnel_key_init(struct net *net, struct nlattr *nla,
> struct nlattr *est, struct tc_action **a,
> - int ovr, int bind, struct netlink_ext_ack *extack)
> + int ovr, int bind, bool rtnl_held,
> + struct netlink_ext_ack *extack)
> {
> struct tc_action_net *tn = net_generic(net, tunnel_key_net_id);
> struct nlattr *tb[TCA_TUNNEL_KEY_MAX + 1];
> diff --git a/net/sched/act_vlan.c b/net/sched/act_vlan.c
> index 8dda78473004..72d2d78fb942 100644
> --- a/net/sched/act_vlan.c
> +++ b/net/sched/act_vlan.c
> @@ -109,7 +109,8 @@ 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, struct netlink_ext_ack *extack)
> + int ovr, int bind, bool rtnl_held,
> + struct netlink_ext_ack *extack)
> {
> struct tc_action_net *tn = net_generic(net, vlan_net_id);
> struct nlattr *tb[TCA_VLAN_MAX + 1];
> diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
> index 963e4bf0aab8..efbf01ce14c2 100644
> --- a/net/sched/cls_api.c
> +++ b/net/sched/cls_api.c
> @@ -1438,7 +1438,7 @@ int tcf_exts_validate(struct net *net, struct tcf_proto *tp, struct nlattr **tb,
> if (exts->police && tb[exts->police]) {
> act = tcf_action_init_1(net, tp, tb[exts->police],
> rate_tlv, "police", ovr,
> - TCA_ACT_BIND, extack);
> + TCA_ACT_BIND, true, extack);
> if (IS_ERR(act))
> return PTR_ERR(act);
>
> @@ -1451,7 +1451,8 @@ int tcf_exts_validate(struct net *net, struct tcf_proto *tp, struct nlattr **tb,
>
> err = tcf_action_init(net, tp, tb[exts->action],
> rate_tlv, NULL, ovr, TCA_ACT_BIND,
> - &actions, &attr_size, extack);
> + &actions, &attr_size, true,
> + extack);
> if (err)
> return err;
> list_for_each_entry(act, &actions, list)
> --
> 2.7.5
>
Powered by blists - more mailing lists