netdev - [PATCH net-next 11/14] net: sched: act_vlan: remove dependency on rtnl lock

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: <1533538465-23199-12-git-send-email-vladbu@mellanox.com>
Date:   Mon,  6 Aug 2018 09:54:22 +0300
From:   Vlad Buslov <vladbu@...lanox.com>
To:     netdev@...r.kernel.org
Cc:     davem@...emloft.net, jhs@...atatu.com, xiyou.wangcong@...il.com,
        jiri@...nulli.us, pablo@...filter.org, kadlec@...ckhole.kfki.hu,
        fw@...len.de, ast@...nel.org, daniel@...earbox.net,
        edumazet@...gle.com, keescook@...omium.org,
        marcelo.leitner@...il.com, Vlad Buslov <vladbu@...lanox.com>
Subject: [PATCH net-next 11/14] net: sched: act_vlan: remove dependency on rtnl lock

Use tcf spinlock to protect vlan action private data from concurrent
modification during dump and init. Use rcu swap operation to reassign
params pointer under protection of tcf lock. (old params value is not used
by init, so there is no need of standalone rcu dereference step)

Remove rtnl assertion that is no longer necessary.

Signed-off-by: Vlad Buslov <vladbu@...lanox.com>
---
 net/sched/act_vlan.c | 27 +++++++++++++++------------
 1 file changed, 15 insertions(+), 12 deletions(-)

diff --git a/net/sched/act_vlan.c b/net/sched/act_vlan.c
index 15a0ee214c9c..5bde17fe3608 100644
--- a/net/sched/act_vlan.c
+++ b/net/sched/act_vlan.c
@@ -109,7 +109,7 @@ static int tcf_vlan_init(struct net *net, struct nlattr *nla,
 {
 	struct tc_action_net *tn = net_generic(net, vlan_net_id);
 	struct nlattr *tb[TCA_VLAN_MAX + 1];
-	struct tcf_vlan_params *p, *p_old;
+	struct tcf_vlan_params *p;
 	struct tc_vlan *parm;
 	struct tcf_vlan *v;
 	int action;
@@ -202,26 +202,24 @@ static int tcf_vlan_init(struct net *net, struct nlattr *nla,
 
 	v = to_vlan(*a);
 
-	ASSERT_RTNL();
 	p = kzalloc(sizeof(*p), GFP_KERNEL);
 	if (!p) {
 		tcf_idr_release(*a, bind);
 		return -ENOMEM;
 	}
 
-	v->tcf_action = parm->action;
-
-	p_old = rtnl_dereference(v->vlan_p);
-
 	p->tcfv_action = action;
 	p->tcfv_push_vid = push_vid;
 	p->tcfv_push_prio = push_prio;
 	p->tcfv_push_proto = push_proto;
 
-	rcu_assign_pointer(v->vlan_p, p);
+	spin_lock(&v->tcf_lock);
+	v->tcf_action = parm->action;
+	rcu_swap_protected(v->vlan_p, p, lockdep_is_held(&v->tcf_lock));
+	spin_unlock(&v->tcf_lock);
 
-	if (p_old)
-		kfree_rcu(p_old, rcu);
+	if (p)
+		kfree_rcu(p, rcu);
 
 	if (ret == ACT_P_CREATED)
 		tcf_idr_insert(tn, *a);
@@ -243,16 +241,18 @@ static int tcf_vlan_dump(struct sk_buff *skb, struct tc_action *a,
 {
 	unsigned char *b = skb_tail_pointer(skb);
 	struct tcf_vlan *v = to_vlan(a);
-	struct tcf_vlan_params *p = rtnl_dereference(v->vlan_p);
+	struct tcf_vlan_params *p;
 	struct tc_vlan opt = {
 		.index    = v->tcf_index,
 		.refcnt   = refcount_read(&v->tcf_refcnt) - ref,
 		.bindcnt  = atomic_read(&v->tcf_bindcnt) - bind,
-		.action   = v->tcf_action,
-		.v_action = p->tcfv_action,
 	};
 	struct tcf_t t;
 
+	spin_lock(&v->tcf_lock);
+	opt.action = v->tcf_action;
+	p = rcu_dereference_protected(v->vlan_p, lockdep_is_held(&v->tcf_lock));
+	opt.v_action = p->tcfv_action;
 	if (nla_put(skb, TCA_VLAN_PARMS, sizeof(opt), &opt))
 		goto nla_put_failure;
 
@@ -268,9 +268,12 @@ static int tcf_vlan_dump(struct sk_buff *skb, struct tc_action *a,
 	tcf_tm_dump(&t, &v->tcf_tm);
 	if (nla_put_64bit(skb, TCA_VLAN_TM, sizeof(t), &t, TCA_VLAN_PAD))
 		goto nla_put_failure;
+	spin_unlock(&v->tcf_lock);
+
 	return skb->len;
 
 nla_put_failure:
+	spin_unlock(&v->tcf_lock);
 	nlmsg_trim(skb, b);
 	return -1;
 }
-- 
2.7.5