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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Mon, 28 May 2018 18:38:57 -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 10/11] net: sched: atomically check-allocate action

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 <vladbu@...lanox.com>

Reviewed-by: Marcelo Ricardo Leitner <marcelo.leitner@...il.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
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ