netdev - Re: [Patch net-nnext] net_sched: add missing tcf_lock for act

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]

Message-ID: <vbf5zzsunky.fsf@reg-r-vrt-018-180.mtr.labs.mlnx>
Date:   Thu, 30 Aug 2018 13:59:41 +0300
From:   Vlad Buslov <vladbu@...lanox.com>
To:     Cong Wang <xiyou.wangcong@...il.com>
Cc:     netdev@...r.kernel.org
Subject: Re: [Patch net-nnext] net_sched: add missing tcf_lock for act_connmark

On Wed 29 Aug 2018 at 17:15, Cong Wang <xiyou.wangcong@...il.com> wrote:
> According to the new locking rule, we have to take tcf_lock
> for both ->init() and ->dump(), as RTNL will be removed.
> However, it is missing for act_connmark.

Thank you for finding and fixing this!

>
> Cc: Vlad Buslov <vladbu@...lanox.com>
> Signed-off-by: Cong Wang <xiyou.wangcong@...il.com>
> ---
>  net/sched/act_connmark.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/net/sched/act_connmark.c b/net/sched/act_connmark.c
> index e869c0ee63c8..8475913f2070 100644
> --- a/net/sched/act_connmark.c
> +++ b/net/sched/act_connmark.c
> @@ -143,8 +143,10 @@ static int tcf_connmark_init(struct net *net, struct nlattr *nla,
>  			return -EEXIST;
>  		}
>  		/* replacing action and zone */
> +		spin_lock_bh(&ci->tcf_lock);
>  		ci->tcf_action = parm->action;
>  		ci->zone = parm->zone;
> +		spin_unlock_bh(&ci->tcf_lock);
>  		ret = 0;
>  	}
>  
> @@ -156,16 +158,16 @@ static inline int tcf_connmark_dump(struct sk_buff *skb, struct tc_action *a,
>  {
>  	unsigned char *b = skb_tail_pointer(skb);
>  	struct tcf_connmark_info *ci = to_connmark(a);
> -
>  	struct tc_connmark opt = {
>  		.index   = ci->tcf_index,
>  		.refcnt  = refcount_read(&ci->tcf_refcnt) - ref,
>  		.bindcnt = atomic_read(&ci->tcf_bindcnt) - bind,
> -		.action  = ci->tcf_action,
> -		.zone   = ci->zone,
>  	};
>  	struct tcf_t t;
>  
> +	spin_lock_bh(&ci->tcf_lock);
> +	opt.action = ci->tcf_action;
> +	opt.zone = ci->zone;
>  	if (nla_put(skb, TCA_CONNMARK_PARMS, sizeof(opt), &opt))
>  		goto nla_put_failure;
>  
> @@ -173,9 +175,12 @@ static inline int tcf_connmark_dump(struct sk_buff *skb, struct tc_action *a,
>  	if (nla_put_64bit(skb, TCA_CONNMARK_TM, sizeof(t), &t,
>  			  TCA_CONNMARK_PAD))
>  		goto nla_put_failure;
> +	spin_unlock_bh(&ci->tcf_lock);
>  
>  	return skb->len;
> +
>  nla_put_failure:
> +	spin_unlock_bh(&ci->tcf_lock);
>  	nlmsg_trim(skb, b);
>  	return -1;
>  }