On Wed 29 Aug 2018 at 17:15, Cong Wang <xiyou.wangc...@gmail.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 <vla...@mellanox.com>
> Signed-off-by: Cong Wang <xiyou.wangc...@gmail.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;
> }
