On Mon, May 28, 2018 at 12:17:28AM +0300, Vlad Buslov wrote:
> Implement function that atomically checks if action exists and either takes
> reference to it, or allocates idr slot for action index to prevent
> concurrent allocations of actions with same index. Use EBUSY error pointer
> to indicate that idr slot is reserved.
>
> Implement cleanup helper function that removes temporary error pointer from
> idr. (in case of error between idr allocation and insertion of newly
> created action to specified index)
>
> Refactor all action init functions to insert new action to idr using this
> API.
>
> Signed-off-by: Vlad Buslov <vla...@mellanox.com>
Reviewed-by: Marcelo Ricardo Leitner <marcelo.leit...@gmail.com>
> ---
> Changes from V1 to V2:
> - Remove unique idr insertion function. Change original idr insert to do
> the same thing.
> - Refactor action check-alloc code into standalone function.
>
> include/net/act_api.h | 3 ++
> net/sched/act_api.c | 92
> ++++++++++++++++++++++++++++++++++++----------
> net/sched/act_bpf.c | 11 ++++--
> net/sched/act_connmark.c | 10 +++--
> net/sched/act_csum.c | 11 ++++--
> net/sched/act_gact.c | 11 ++++--
> net/sched/act_ife.c | 6 ++-
> net/sched/act_ipt.c | 13 ++++++-
> net/sched/act_mirred.c | 16 ++++++--
> net/sched/act_nat.c | 11 ++++--
> net/sched/act_pedit.c | 15 ++++++--
> net/sched/act_police.c | 9 ++++-
> net/sched/act_sample.c | 11 ++++--
> net/sched/act_simple.c | 11 +++++-
> net/sched/act_skbedit.c | 11 +++++-
> net/sched/act_skbmod.c | 11 +++++-
> net/sched/act_tunnel_key.c | 9 ++++-
> net/sched/act_vlan.c | 17 ++++++++-
> 18 files changed, 218 insertions(+), 60 deletions(-)
>
> diff --git a/include/net/act_api.h b/include/net/act_api.h
> index d256e20507b9..cd4547476074 100644
> --- a/include/net/act_api.h
> +++ b/include/net/act_api.h
> @@ -154,6 +154,9 @@ int tcf_idr_create(struct tc_action_net *tn, u32 index,
> struct nlattr *est,
> int bind, bool cpustats);
> void tcf_idr_insert(struct tc_action_net *tn, struct tc_action *a);
>
> +void tcf_idr_cleanup(struct tc_action_net *tn, u32 index);
> +int tcf_idr_check_alloc(struct tc_action_net *tn, u32 *index,
> + struct tc_action **a, int bind);
> int tcf_idr_delete_index(struct tc_action_net *tn, u32 index);
> int __tcf_idr_release(struct tc_action *a, bool bind, bool strict);
>
> diff --git a/net/sched/act_api.c b/net/sched/act_api.c
> index eefe8c2fe667..9511502e1cbb 100644
> --- a/net/sched/act_api.c
> +++ b/net/sched/act_api.c
> @@ -303,7 +303,9 @@ static bool __tcf_idr_check(struct tc_action_net *tn, u32
> index,
>
> spin_lock(&idrinfo->lock);
> p = idr_find(&idrinfo->action_idr, index);
> - if (p) {
> + if (IS_ERR(p)) {
> + p = NULL;
> + } else if (p) {
> refcount_inc(&p->tcfa_refcnt);
> if (bind)
> atomic_inc(&p->tcfa_bindcnt);
> @@ -371,7 +373,6 @@ int tcf_idr_create(struct tc_action_net *tn, u32 index,
> struct nlattr *est,
> {
> struct tc_action *p = kzalloc(ops->size, GFP_KERNEL);
> struct tcf_idrinfo *idrinfo = tn->idrinfo;
> - struct idr *idr = &idrinfo->action_idr;
> int err = -ENOMEM;
>
> if (unlikely(!p))
> @@ -389,20 +390,6 @@ int tcf_idr_create(struct tc_action_net *tn, u32 index,
> struct nlattr *est,
> goto err2;
> }
> spin_lock_init(&p->tcfa_lock);
> - idr_preload(GFP_KERNEL);
> - spin_lock(&idrinfo->lock);
> - /* user doesn't specify an index */
> - if (!index) {
> - index = 1;
> - err = idr_alloc_u32(idr, NULL, &index, UINT_MAX, GFP_ATOMIC);
> - } else {
> - err = idr_alloc_u32(idr, NULL, &index, index, GFP_ATOMIC);
> - }
> - spin_unlock(&idrinfo->lock);
> - idr_preload_end();
> - if (err)
> - goto err3;
> -
> p->tcfa_index = index;
> p->tcfa_tm.install = jiffies;
> p->tcfa_tm.lastuse = jiffies;
> @@ -412,7 +399,7 @@ int tcf_idr_create(struct tc_action_net *tn, u32 index,
> struct nlattr *est,
> &p->tcfa_rate_est,
> &p->tcfa_lock, NULL, est);
> if (err)
> - goto err4;
> + goto err3;
> }
>
> p->idrinfo = idrinfo;
> @@ -420,8 +407,6 @@ int tcf_idr_create(struct tc_action_net *tn, u32 index,
> struct nlattr *est,
> INIT_LIST_HEAD(&p->list);
> *a = p;
> return 0;
> -err4:
> - idr_remove(idr, index);
> err3:
> free_percpu(p->cpu_qstats);
> err2:
> @@ -437,11 +422,78 @@ void tcf_idr_insert(struct tc_action_net *tn, struct
> tc_action *a)
> struct tcf_idrinfo *idrinfo = tn->idrinfo;
>
> spin_lock(&idrinfo->lock);
> - idr_replace(&idrinfo->action_idr, a, a->tcfa_index);
> + /* Replace ERR_PTR(-EBUSY) allocated by tcf_idr_check_alloc */
> + WARN_ON(!IS_ERR(idr_replace(&idrinfo->action_idr, a, a->tcfa_index)));
> spin_unlock(&idrinfo->lock);
> }
> EXPORT_SYMBOL(tcf_idr_insert);
>
> +/* Cleanup idr index that was allocated but not initialized. */
> +
> +void tcf_idr_cleanup(struct tc_action_net *tn, u32 index)
> +{
> + struct tcf_idrinfo *idrinfo = tn->idrinfo;
> +
> + spin_lock(&idrinfo->lock);
> + /* Remove ERR_PTR(-EBUSY) allocated by tcf_idr_check_alloc */
> + WARN_ON(!IS_ERR(idr_remove(&idrinfo->action_idr, index)));
> + spin_unlock(&idrinfo->lock);
> +}
> +EXPORT_SYMBOL(tcf_idr_cleanup);
> +
> +/* Check if action with specified index exists. If actions is found,
> increments
> + * its reference and bind counters, and return 1. Otherwise insert temporary
> + * error pointer (to prevent concurrent users from inserting actions with
> same
> + * index) and return 0.
> + */
> +
> +int tcf_idr_check_alloc(struct tc_action_net *tn, u32 *index,
> + struct tc_action **a, int bind)
> +{
> + struct tcf_idrinfo *idrinfo = tn->idrinfo;
> + struct tc_action *p;
> + int ret;
> +
> +again:
> + spin_lock(&idrinfo->lock);
> + if (*index) {
> + p = idr_find(&idrinfo->action_idr, *index);
> + if (IS_ERR(p)) {
> + /* This means that another process allocated
> + * index but did not assign the pointer yet.
> + */
> + spin_unlock(&idrinfo->lock);
> + goto again;
> + }
> +
> + if (p) {
> + refcount_inc(&p->tcfa_refcnt);
> + if (bind)
> + atomic_inc(&p->tcfa_bindcnt);
> + *a = p;
> + ret = 1;
> + } else {
> + *a = NULL;
> + ret = idr_alloc_u32(&idrinfo->action_idr, NULL, index,
> + *index, GFP_ATOMIC);
> + if (!ret)
> + idr_replace(&idrinfo->action_idr,
> + ERR_PTR(-EBUSY), *index);
> + }
> + } else {
> + *index = 1;
> + *a = NULL;
> + ret = idr_alloc_u32(&idrinfo->action_idr, NULL, index,
> + UINT_MAX, GFP_ATOMIC);
> + if (!ret)
> + idr_replace(&idrinfo->action_idr, ERR_PTR(-EBUSY),
> + *index);
> + }
> + spin_unlock(&idrinfo->lock);
> + return ret;
> +}
> +EXPORT_SYMBOL(tcf_idr_check_alloc);
> +
> void tcf_idrinfo_destroy(const struct tc_action_ops *ops,
> struct tcf_idrinfo *idrinfo)
> {
> diff --git a/net/sched/act_bpf.c b/net/sched/act_bpf.c
> index d3f4ac6f2c4b..06f743d8ed41 100644
> --- a/net/sched/act_bpf.c
> +++ b/net/sched/act_bpf.c
> @@ -299,14 +299,17 @@ static int tcf_bpf_init(struct net *net, struct nlattr
> *nla,
>
> parm = nla_data(tb[TCA_ACT_BPF_PARMS]);
>
> - if (!tcf_idr_check(tn, parm->index, act, bind)) {
> + ret = tcf_idr_check_alloc(tn, &parm->index, act, bind);
> + if (!ret) {
> ret = tcf_idr_create(tn, parm->index, est, act,
> &act_bpf_ops, bind, true);
> - if (ret < 0)
> + if (ret < 0) {
> + tcf_idr_cleanup(tn, parm->index);
> return ret;
> + }
>
> res = ACT_P_CREATED;
> - } else {
> + } else if (ret > 0) {
> /* Don't override defaults. */
> if (bind)
> return 0;
> @@ -315,6 +318,8 @@ static int tcf_bpf_init(struct net *net, struct nlattr
> *nla,
> tcf_idr_release(*act, bind);
> return -EEXIST;
> }
> + } else {
> + return ret;
> }
>
> is_bpf = tb[TCA_ACT_BPF_OPS_LEN] && tb[TCA_ACT_BPF_OPS];
> diff --git a/net/sched/act_connmark.c b/net/sched/act_connmark.c
> index 701e90244eff..1e31f0e448e2 100644
> --- a/net/sched/act_connmark.c
> +++ b/net/sched/act_connmark.c
> @@ -118,11 +118,14 @@ static int tcf_connmark_init(struct net *net, struct
> nlattr *nla,
>
> parm = nla_data(tb[TCA_CONNMARK_PARMS]);
>
> - if (!tcf_idr_check(tn, parm->index, a, bind)) {
> + ret = tcf_idr_check_alloc(tn, &parm->index, a, bind);
> + if (!ret) {
> ret = tcf_idr_create(tn, parm->index, est, a,
> &act_connmark_ops, bind, false);
> - if (ret)
> + if (ret) {
> + tcf_idr_cleanup(tn, parm->index);
> return ret;
> + }
>
> ci = to_connmark(*a);
> ci->tcf_action = parm->action;
> @@ -131,7 +134,7 @@ static int tcf_connmark_init(struct net *net, struct
> nlattr *nla,
>
> tcf_idr_insert(tn, *a);
> ret = ACT_P_CREATED;
> - } else {
> + } else if (ret > 0) {
> ci = to_connmark(*a);
> if (bind)
> return 0;
> @@ -142,6 +145,7 @@ static int tcf_connmark_init(struct net *net, struct
> nlattr *nla,
> /* replacing action and zone */
> ci->tcf_action = parm->action;
> ci->zone = parm->zone;
> + ret = 0;
> }
>
> return ret;
> diff --git a/net/sched/act_csum.c b/net/sched/act_csum.c
> index 5dbee136b0a1..bd232d3bd022 100644
> --- a/net/sched/act_csum.c
> +++ b/net/sched/act_csum.c
> @@ -67,19 +67,24 @@ static int tcf_csum_init(struct net *net, struct nlattr
> *nla,
> return -EINVAL;
> parm = nla_data(tb[TCA_CSUM_PARMS]);
>
> - if (!tcf_idr_check(tn, parm->index, a, bind)) {
> + err = tcf_idr_check_alloc(tn, &parm->index, a, bind);
> + if (!err) {
> ret = tcf_idr_create(tn, parm->index, est, a,
> &act_csum_ops, bind, true);
> - if (ret)
> + if (ret) {
> + tcf_idr_cleanup(tn, parm->index);
> return ret;
> + }
> ret = ACT_P_CREATED;
> - } else {
> + } else if (err > 0) {
> if (bind)/* dont override defaults */
> return 0;
> if (!ovr) {
> tcf_idr_release(*a, bind);
> return -EEXIST;
> }
> + } else {
> + return err;
> }
>
> p = to_tcf_csum(*a);
> diff --git a/net/sched/act_gact.c b/net/sched/act_gact.c
> index 11c4de3f344e..661b72b9147d 100644
> --- a/net/sched/act_gact.c
> +++ b/net/sched/act_gact.c
> @@ -91,19 +91,24 @@ static int tcf_gact_init(struct net *net, struct nlattr
> *nla,
> }
> #endif
>
> - if (!tcf_idr_check(tn, parm->index, a, bind)) {
> + err = tcf_idr_check_alloc(tn, &parm->index, a, bind);
> + if (!err) {
> ret = tcf_idr_create(tn, parm->index, est, a,
> &act_gact_ops, bind, true);
> - if (ret)
> + if (ret) {
> + tcf_idr_cleanup(tn, parm->index);
> return ret;
> + }
> ret = ACT_P_CREATED;
> - } else {
> + } else if (err > 0) {
> if (bind)/* dont override defaults */
> return 0;
> if (!ovr) {
> tcf_idr_release(*a, bind);
> return -EEXIST;
> }
> + } else {
> + return err;
> }
>
> gact = to_gact(*a);
> diff --git a/net/sched/act_ife.c b/net/sched/act_ife.c
> index 3dd3d79c5a4b..5bf0e79796c0 100644
> --- a/net/sched/act_ife.c
> +++ b/net/sched/act_ife.c
> @@ -483,7 +483,10 @@ static int tcf_ife_init(struct net *net, struct nlattr
> *nla,
> if (!p)
> return -ENOMEM;
>
> - exists = tcf_idr_check(tn, parm->index, a, bind);
> + err = tcf_idr_check_alloc(tn, &parm->index, a, bind);
> + if (err < 0)
> + return err;
> + exists = err;
> if (exists && bind) {
> kfree(p);
> return 0;
> @@ -493,6 +496,7 @@ static int tcf_ife_init(struct net *net, struct nlattr
> *nla,
> ret = tcf_idr_create(tn, parm->index, est, a, &act_ife_ops,
> bind, true);
> if (ret) {
> + tcf_idr_cleanup(tn, parm->index);
> kfree(p);
> return ret;
> }
> diff --git a/net/sched/act_ipt.c b/net/sched/act_ipt.c
> index 85e85dfba401..0dc787a57798 100644
> --- a/net/sched/act_ipt.c
> +++ b/net/sched/act_ipt.c
> @@ -119,13 +119,18 @@ static int __tcf_ipt_init(struct net *net, unsigned int
> id, struct nlattr *nla,
> if (tb[TCA_IPT_INDEX] != NULL)
> index = nla_get_u32(tb[TCA_IPT_INDEX]);
>
> - exists = tcf_idr_check(tn, index, a, bind);
> + err = tcf_idr_check_alloc(tn, &index, a, bind);
> + if (err < 0)
> + return err;
> + exists = err;
> if (exists && bind)
> return 0;
>
> if (tb[TCA_IPT_HOOK] == NULL || tb[TCA_IPT_TARG] == NULL) {
> if (exists)
> tcf_idr_release(*a, bind);
> + else
> + tcf_idr_cleanup(tn, index);
> return -EINVAL;
> }
>
> @@ -133,14 +138,18 @@ static int __tcf_ipt_init(struct net *net, unsigned int
> id, struct nlattr *nla,
> if (nla_len(tb[TCA_IPT_TARG]) < td->u.target_size) {
> if (exists)
> tcf_idr_release(*a, bind);
> + else
> + tcf_idr_cleanup(tn, index);
> return -EINVAL;
> }
>
> if (!exists) {
> ret = tcf_idr_create(tn, index, est, a, ops, bind,
> false);
> - if (ret)
> + if (ret) {
> + tcf_idr_cleanup(tn, index);
> return ret;
> + }
> ret = ACT_P_CREATED;
> } else {
> if (bind)/* dont override defaults */
> diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
> index e08aed06d7f8..6afd89a36c69 100644
> --- a/net/sched/act_mirred.c
> +++ b/net/sched/act_mirred.c
> @@ -79,7 +79,7 @@ static int tcf_mirred_init(struct net *net, struct nlattr
> *nla,
> struct tcf_mirred *m;
> struct net_device *dev;
> bool exists = false;
> - int ret;
> + int ret, err;
>
> if (!nla) {
> NL_SET_ERR_MSG_MOD(extack, "Mirred requires attributes to be
> passed");
> @@ -94,7 +94,10 @@ static int tcf_mirred_init(struct net *net, struct nlattr
> *nla,
> }
> parm = nla_data(tb[TCA_MIRRED_PARMS]);
>
> - exists = tcf_idr_check(tn, parm->index, a, bind);
> + err = tcf_idr_check_alloc(tn, &parm->index, a, bind);
> + if (err < 0)
> + return err;
> + exists = err;
> if (exists && bind)
> return 0;
>
> @@ -107,6 +110,8 @@ static int tcf_mirred_init(struct net *net, struct nlattr
> *nla,
> default:
> if (exists)
> tcf_idr_release(*a, bind);
> + else
> + tcf_idr_cleanup(tn, parm->index);
> NL_SET_ERR_MSG_MOD(extack, "Unknown mirred option");
> return -EINVAL;
> }
> @@ -115,6 +120,8 @@ static int tcf_mirred_init(struct net *net, struct nlattr
> *nla,
> if (dev == NULL) {
> if (exists)
> tcf_idr_release(*a, bind);
> + else
> + tcf_idr_cleanup(tn, parm->index);
> return -ENODEV;
> }
> mac_header_xmit = dev_is_mac_header_xmit(dev);
> @@ -124,13 +131,16 @@ static int tcf_mirred_init(struct net *net, struct
> nlattr *nla,
>
> if (!exists) {
> if (!dev) {
> + tcf_idr_cleanup(tn, parm->index);
> NL_SET_ERR_MSG_MOD(extack, "Specified device does not
> exist");
> return -EINVAL;
> }
> ret = tcf_idr_create(tn, parm->index, est, a,
> &act_mirred_ops, bind, true);
> - if (ret)
> + if (ret) {
> + tcf_idr_cleanup(tn, parm->index);
> return ret;
> + }
> ret = ACT_P_CREATED;
> } else if (!ovr) {
> tcf_idr_release(*a, bind);
> diff --git a/net/sched/act_nat.c b/net/sched/act_nat.c
> index 1f91e8e66c0f..4dd9188a72fd 100644
> --- a/net/sched/act_nat.c
> +++ b/net/sched/act_nat.c
> @@ -57,19 +57,24 @@ static int tcf_nat_init(struct net *net, struct nlattr
> *nla, struct nlattr *est,
> return -EINVAL;
> parm = nla_data(tb[TCA_NAT_PARMS]);
>
> - if (!tcf_idr_check(tn, parm->index, a, bind)) {
> + err = tcf_idr_check_alloc(tn, &parm->index, a, bind);
> + if (!err) {
> ret = tcf_idr_create(tn, parm->index, est, a,
> &act_nat_ops, bind, false);
> - if (ret)
> + if (ret) {
> + tcf_idr_cleanup(tn, parm->index);
> return ret;
> + }
> ret = ACT_P_CREATED;
> - } else {
> + } else if (err > 0) {
> if (bind)
> return 0;
> if (!ovr) {
> tcf_idr_release(*a, bind);
> return -EEXIST;
> }
> + } else {
> + return err;
> }
> p = to_tcf_nat(*a);
>
> diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c
> index fbf283f2ac34..2bd1d3f61488 100644
> --- a/net/sched/act_pedit.c
> +++ b/net/sched/act_pedit.c
> @@ -167,13 +167,18 @@ static int tcf_pedit_init(struct net *net, struct
> nlattr *nla,
> if (IS_ERR(keys_ex))
> return PTR_ERR(keys_ex);
>
> - if (!tcf_idr_check(tn, parm->index, a, bind)) {
> - if (!parm->nkeys)
> + err = tcf_idr_check_alloc(tn, &parm->index, a, bind);
> + if (!err) {
> + if (!parm->nkeys) {
> + tcf_idr_cleanup(tn, parm->index);
> return -EINVAL;
> + }
> ret = tcf_idr_create(tn, parm->index, est, a,
> &act_pedit_ops, bind, false);
> - if (ret)
> + if (ret) {
> + tcf_idr_cleanup(tn, parm->index);
> return ret;
> + }
> p = to_pedit(*a);
> keys = kmalloc(ksize, GFP_KERNEL);
> if (keys == NULL) {
> @@ -182,7 +187,7 @@ static int tcf_pedit_init(struct net *net, struct nlattr
> *nla,
> return -ENOMEM;
> }
> ret = ACT_P_CREATED;
> - } else {
> + } else if (err > 0) {
> if (bind)
> return 0;
> if (!ovr) {
> @@ -197,6 +202,8 @@ static int tcf_pedit_init(struct net *net, struct nlattr
> *nla,
> return -ENOMEM;
> }
> }
> + } else {
> + return err;
> }
>
> spin_lock_bh(&p->tcf_lock);
> diff --git a/net/sched/act_police.c b/net/sched/act_police.c
> index 99335cca739e..1f3192ea8df7 100644
> --- a/net/sched/act_police.c
> +++ b/net/sched/act_police.c
> @@ -101,15 +101,20 @@ static int tcf_act_police_init(struct net *net, struct
> nlattr *nla,
> return -EINVAL;
>
> parm = nla_data(tb[TCA_POLICE_TBF]);
> - exists = tcf_idr_check(tn, parm->index, a, bind);
> + err = tcf_idr_check_alloc(tn, &parm->index, a, bind);
> + if (err < 0)
> + return err;
> + exists = err;
> if (exists && bind)
> return 0;
>
> if (!exists) {
> ret = tcf_idr_create(tn, parm->index, NULL, a,
> &act_police_ops, bind, false);
> - if (ret)
> + if (ret) {
> + tcf_idr_cleanup(tn, parm->index);
> return ret;
> + }
> ret = ACT_P_CREATED;
> } else if (!ovr) {
> tcf_idr_release(*a, bind);
> diff --git a/net/sched/act_sample.c b/net/sched/act_sample.c
> index a8582e1347db..3079e7be5bde 100644
> --- a/net/sched/act_sample.c
> +++ b/net/sched/act_sample.c
> @@ -46,7 +46,7 @@ static int tcf_sample_init(struct net *net, struct nlattr
> *nla,
> struct tc_sample *parm;
> struct tcf_sample *s;
> bool exists = false;
> - int ret;
> + int ret, err;
>
> if (!nla)
> return -EINVAL;
> @@ -59,15 +59,20 @@ static int tcf_sample_init(struct net *net, struct nlattr
> *nla,
>
> parm = nla_data(tb[TCA_SAMPLE_PARMS]);
>
> - exists = tcf_idr_check(tn, parm->index, a, bind);
> + err = tcf_idr_check_alloc(tn, &parm->index, a, bind);
> + if (err < 0)
> + return err;
> + exists = err;
> if (exists && bind)
> return 0;
>
> if (!exists) {
> ret = tcf_idr_create(tn, parm->index, est, a,
> &act_sample_ops, bind, false);
> - if (ret)
> + if (ret) {
> + tcf_idr_cleanup(tn, parm->index);
> return ret;
> + }
> ret = ACT_P_CREATED;
> } else if (!ovr) {
> tcf_idr_release(*a, bind);
> diff --git a/net/sched/act_simple.c b/net/sched/act_simple.c
> index 78fffd329ed9..2cc874f791df 100644
> --- a/net/sched/act_simple.c
> +++ b/net/sched/act_simple.c
> @@ -101,13 +101,18 @@ static int tcf_simp_init(struct net *net, struct nlattr
> *nla,
> return -EINVAL;
>
> parm = nla_data(tb[TCA_DEF_PARMS]);
> - exists = tcf_idr_check(tn, parm->index, a, bind);
> + err = tcf_idr_check_alloc(tn, &parm->index, a, bind);
> + if (err < 0)
> + return err;
> + exists = err;
> if (exists && bind)
> return 0;
>
> if (tb[TCA_DEF_DATA] == NULL) {
> if (exists)
> tcf_idr_release(*a, bind);
> + else
> + tcf_idr_cleanup(tn, parm->index);
> return -EINVAL;
> }
>
> @@ -116,8 +121,10 @@ static int tcf_simp_init(struct net *net, struct nlattr
> *nla,
> if (!exists) {
> ret = tcf_idr_create(tn, parm->index, est, a,
> &act_simp_ops, bind, false);
> - if (ret)
> + if (ret) {
> + tcf_idr_cleanup(tn, parm->index);
> return ret;
> + }
>
> d = to_defact(*a);
> ret = alloc_defdata(d, defdata);
> diff --git a/net/sched/act_skbedit.c b/net/sched/act_skbedit.c
> index c0607d1319eb..29a15172a99d 100644
> --- a/net/sched/act_skbedit.c
> +++ b/net/sched/act_skbedit.c
> @@ -117,21 +117,28 @@ static int tcf_skbedit_init(struct net *net, struct
> nlattr *nla,
>
> parm = nla_data(tb[TCA_SKBEDIT_PARMS]);
>
> - exists = tcf_idr_check(tn, parm->index, a, bind);
> + err = tcf_idr_check_alloc(tn, &parm->index, a, bind);
> + if (err < 0)
> + return err;
> + exists = err;
> if (exists && bind)
> return 0;
>
> if (!flags) {
> if (exists)
> tcf_idr_release(*a, bind);
> + else
> + tcf_idr_cleanup(tn, parm->index);
> return -EINVAL;
> }
>
> if (!exists) {
> ret = tcf_idr_create(tn, parm->index, est, a,
> &act_skbedit_ops, bind, false);
> - if (ret)
> + if (ret) {
> + tcf_idr_cleanup(tn, parm->index);
> return ret;
> + }
>
> d = to_skbedit(*a);
> ret = ACT_P_CREATED;
> diff --git a/net/sched/act_skbmod.c b/net/sched/act_skbmod.c
> index e844381af066..cdc6bacfb190 100644
> --- a/net/sched/act_skbmod.c
> +++ b/net/sched/act_skbmod.c
> @@ -128,21 +128,28 @@ static int tcf_skbmod_init(struct net *net, struct
> nlattr *nla,
> if (parm->flags & SKBMOD_F_SWAPMAC)
> lflags = SKBMOD_F_SWAPMAC;
>
> - exists = tcf_idr_check(tn, parm->index, a, bind);
> + err = tcf_idr_check_alloc(tn, &parm->index, a, bind);
> + if (err < 0)
> + return err;
> + exists = err;
> if (exists && bind)
> return 0;
>
> if (!lflags) {
> if (exists)
> tcf_idr_release(*a, bind);
> + else
> + tcf_idr_cleanup(tn, parm->index);
> return -EINVAL;
> }
>
> if (!exists) {
> ret = tcf_idr_create(tn, parm->index, est, a,
> &act_skbmod_ops, bind, true);
> - if (ret)
> + if (ret) {
> + tcf_idr_cleanup(tn, parm->index);
> return ret;
> + }
>
> ret = ACT_P_CREATED;
> } else if (!ovr) {
> diff --git a/net/sched/act_tunnel_key.c b/net/sched/act_tunnel_key.c
> index bd53f39a345b..4679b620af12 100644
> --- a/net/sched/act_tunnel_key.c
> +++ b/net/sched/act_tunnel_key.c
> @@ -99,7 +99,10 @@ static int tunnel_key_init(struct net *net, struct nlattr
> *nla,
> return -EINVAL;
>
> parm = nla_data(tb[TCA_TUNNEL_KEY_PARMS]);
> - exists = tcf_idr_check(tn, parm->index, a, bind);
> + err = tcf_idr_check_alloc(tn, &parm->index, a, bind);
> + if (err < 0)
> + return err;
> + exists = err;
> if (exists && bind)
> return 0;
>
> @@ -162,7 +165,7 @@ static int tunnel_key_init(struct net *net, struct nlattr
> *nla,
> ret = tcf_idr_create(tn, parm->index, est, a,
> &act_tunnel_key_ops, bind, true);
> if (ret)
> - return ret;
> + goto err_out;
>
> ret = ACT_P_CREATED;
> } else if (!ovr) {
> @@ -198,6 +201,8 @@ static int tunnel_key_init(struct net *net, struct nlattr
> *nla,
> err_out:
> if (exists)
> tcf_idr_release(*a, bind);
> + else
> + tcf_idr_cleanup(tn, parm->index);
> return ret;
> }
>
> diff --git a/net/sched/act_vlan.c b/net/sched/act_vlan.c
> index 4ac0d565e437..bae9822837d7 100644
> --- a/net/sched/act_vlan.c
> +++ b/net/sched/act_vlan.c
> @@ -134,7 +134,10 @@ static int tcf_vlan_init(struct net *net, struct nlattr
> *nla,
> if (!tb[TCA_VLAN_PARMS])
> return -EINVAL;
> parm = nla_data(tb[TCA_VLAN_PARMS]);
> - exists = tcf_idr_check(tn, parm->index, a, bind);
> + err = tcf_idr_check_alloc(tn, &parm->index, a, bind);
> + if (err < 0)
> + return err;
> + exists = err;
> if (exists && bind)
> return 0;
>
> @@ -146,12 +149,16 @@ static int tcf_vlan_init(struct net *net, struct nlattr
> *nla,
> if (!tb[TCA_VLAN_PUSH_VLAN_ID]) {
> if (exists)
> tcf_idr_release(*a, bind);
> + else
> + tcf_idr_cleanup(tn, parm->index);
> return -EINVAL;
> }
> push_vid = nla_get_u16(tb[TCA_VLAN_PUSH_VLAN_ID]);
> if (push_vid >= VLAN_VID_MASK) {
> if (exists)
> tcf_idr_release(*a, bind);
> + else
> + tcf_idr_cleanup(tn, parm->index);
> return -ERANGE;
> }
>
> @@ -162,6 +169,8 @@ static int tcf_vlan_init(struct net *net, struct nlattr
> *nla,
> case htons(ETH_P_8021AD):
> break;
> default:
> + if (!exists)
> + tcf_idr_cleanup(tn, parm->index);
> return -EPROTONOSUPPORT;
> }
> } else {
> @@ -174,6 +183,8 @@ static int tcf_vlan_init(struct net *net, struct nlattr
> *nla,
> default:
> if (exists)
> tcf_idr_release(*a, bind);
> + else
> + tcf_idr_cleanup(tn, parm->index);
> return -EINVAL;
> }
> action = parm->v_action;
> @@ -181,8 +192,10 @@ static int tcf_vlan_init(struct net *net, struct nlattr
> *nla,
> if (!exists) {
> ret = tcf_idr_create(tn, parm->index, est, a,
> &act_vlan_ops, bind, true);
> - if (ret)
> + if (ret) {
> + tcf_idr_cleanup(tn, parm->index);
> return ret;
> + }
>
> ret = ACT_P_CREATED;
> } else if (!ovr) {
> --
> 2.7.5
>
