Skip to content

Commit ed76f5e

Browse files
w1ldptrdavem330
w1ldptr
authored and
davem330
committed
net: sched: protect filter_chain list with filter_chain_lock mutex
Extend tcf_chain with new filter_chain_lock mutex. Always lock the chain when accessing filter_chain list, instead of relying on rtnl lock. Dereference filter_chain with tcf_chain_dereference() lockdep macro to verify that all users of chain_list have the lock taken. Rearrange tp insert/remove code in tc_new_tfilter/tc_del_tfilter to execute all necessary code while holding chain lock in order to prevent invalidation of chain_info structure by potential concurrent change. This also serializes calls to tcf_chain0_head_change(), which allows head change callbacks to rely on filter_chain_lock for synchronization instead of rtnl mutex. Signed-off-by: Vlad Buslov <vladbu@mellanox.com> Acked-by: Jiri Pirko <jiri@mellanox.com> Signed-off-by: David S. Miller <davem@davemloft.net>
1 parent a565482 commit ed76f5e

File tree

3 files changed

+101
-33
lines changed

3 files changed

+101
-33
lines changed

‎include/net/sch_generic.h‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -341,6 +341,8 @@ struct qdisc_skb_cb {
341341
typedef void tcf_chain_head_change_t(struct tcf_proto *tp_head, void *priv);
342342

343343
struct tcf_chain {
344+
/* Protects filter_chain. */
345+
struct mutex filter_chain_lock;
344346
struct tcf_proto __rcu *filter_chain;
345347
struct list_head list;
346348
struct tcf_block *block;
@@ -374,6 +376,21 @@ struct tcf_block {
374376
struct rcu_head rcu;
375377
};
376378

379+
#ifdef CONFIG_PROVE_LOCKING
380+
static inline bool lockdep_tcf_chain_is_locked(struct tcf_chain *chain)
381+
{
382+
return lockdep_is_held(&chain->filter_chain_lock);
383+
}
384+
#else
385+
static inline bool lockdep_tcf_chain_is_locked(struct tcf_block *chain)
386+
{
387+
return true;
388+
}
389+
#endif /* #ifdef CONFIG_PROVE_LOCKING */
390+
391+
#define tcf_chain_dereference(p, chain) \
392+
rcu_dereference_protected(p, lockdep_tcf_chain_is_locked(chain))
393+
377394
static inline void tcf_block_offload_inc(struct tcf_block *block, u32 *flags)
378395
{
379396
if (*flags & TCA_CLS_FLAGS_IN_HW)

‎net/sched/cls_api.c‎

Lines changed: 79 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,7 @@ static struct tcf_chain *tcf_chain_create(struct tcf_block *block,
221221
if (!chain)
222222
return NULL;
223223
list_add_tail(&chain->list, &block->chain_list);
224+
mutex_init(&chain->filter_chain_lock);
224225
chain->block = block;
225226
chain->index = chain_index;
226227
chain->refcnt = 1;
@@ -280,6 +281,7 @@ static void tcf_chain_destroy(struct tcf_chain *chain, bool free_block)
280281
{
281282
struct tcf_block *block = chain->block;
282283

284+
mutex_destroy(&chain->filter_chain_lock);
283285
kfree(chain);
284286
if (free_block)
285287
tcf_block_destroy(block);
@@ -443,9 +445,13 @@ static void tcf_chain_put_explicitly_created(struct tcf_chain *chain)
443445

444446
static void tcf_chain_flush(struct tcf_chain *chain)
445447
{
446-
struct tcf_proto *tp = rtnl_dereference(chain->filter_chain);
448+
struct tcf_proto *tp;
447449

450+
mutex_lock(&chain->filter_chain_lock);
451+
tp = tcf_chain_dereference(chain->filter_chain, chain);
448452
tcf_chain0_head_change(chain, NULL);
453+
mutex_unlock(&chain->filter_chain_lock);
454+
449455
while (tp) {
450456
RCU_INIT_POINTER(chain->filter_chain, tp->next);
451457
tcf_proto_destroy(tp, NULL);
@@ -785,11 +791,29 @@ tcf_chain0_head_change_cb_add(struct tcf_block *block,
785791

786792
mutex_lock(&block->lock);
787793
chain0 = block->chain0.chain;
788-
if (chain0 && chain0->filter_chain)
789-
tcf_chain_head_change_item(item, chain0->filter_chain);
790-
list_add(&item->list, &block->chain0.filter_chain_list);
794+
if (chain0)
795+
tcf_chain_hold(chain0);
796+
else
797+
list_add(&item->list, &block->chain0.filter_chain_list);
791798
mutex_unlock(&block->lock);
792799

800+
if (chain0) {
801+
struct tcf_proto *tp_head;
802+
803+
mutex_lock(&chain0->filter_chain_lock);
804+
805+
tp_head = tcf_chain_dereference(chain0->filter_chain, chain0);
806+
if (tp_head)
807+
tcf_chain_head_change_item(item, tp_head);
808+
809+
mutex_lock(&block->lock);
810+
list_add(&item->list, &block->chain0.filter_chain_list);
811+
mutex_unlock(&block->lock);
812+
813+
mutex_unlock(&chain0->filter_chain_lock);
814+
tcf_chain_put(chain0);
815+
}
816+
793817
return 0;
794818
}
795819

@@ -1464,9 +1488,10 @@ struct tcf_chain_info {
14641488
struct tcf_proto __rcu *next;
14651489
};
14661490

1467-
static struct tcf_proto *tcf_chain_tp_prev(struct tcf_chain_info *chain_info)
1491+
static struct tcf_proto *tcf_chain_tp_prev(struct tcf_chain *chain,
1492+
struct tcf_chain_info *chain_info)
14681493
{
1469-
return rtnl_dereference(*chain_info->pprev);
1494+
return tcf_chain_dereference(*chain_info->pprev, chain);
14701495
}
14711496

14721497
static void tcf_chain_tp_insert(struct tcf_chain *chain,
@@ -1475,7 +1500,7 @@ static void tcf_chain_tp_insert(struct tcf_chain *chain,
14751500
{
14761501
if (*chain_info->pprev == chain->filter_chain)
14771502
tcf_chain0_head_change(chain, tp);
1478-
RCU_INIT_POINTER(tp->next, tcf_chain_tp_prev(chain_info));
1503+
RCU_INIT_POINTER(tp->next, tcf_chain_tp_prev(chain, chain_info));
14791504
rcu_assign_pointer(*chain_info->pprev, tp);
14801505
tcf_chain_hold(chain);
14811506
}
@@ -1484,7 +1509,7 @@ static void tcf_chain_tp_remove(struct tcf_chain *chain,
14841509
struct tcf_chain_info *chain_info,
14851510
struct tcf_proto *tp)
14861511
{
1487-
struct tcf_proto *next = rtnl_dereference(chain_info->next);
1512+
struct tcf_proto *next = tcf_chain_dereference(chain_info->next, chain);
14881513

14891514
if (tp == chain->filter_chain)
14901515
tcf_chain0_head_change(chain, next);
@@ -1502,7 +1527,8 @@ static struct tcf_proto *tcf_chain_tp_find(struct tcf_chain *chain,
15021527

15031528
/* Check the chain for existence of proto-tcf with this priority */
15041529
for (pprev = &chain->filter_chain;
1505-
(tp = rtnl_dereference(*pprev)); pprev = &tp->next) {
1530+
(tp = tcf_chain_dereference(*pprev, chain));
1531+
pprev = &tp->next) {
15061532
if (tp->prio >= prio) {
15071533
if (tp->prio == prio) {
15081534
if (prio_allocate ||
@@ -1710,12 +1736,13 @@ static int tc_new_tfilter(struct sk_buff *skb, struct nlmsghdr *n,
17101736
goto errout;
17111737
}
17121738

1739+
mutex_lock(&chain->filter_chain_lock);
17131740
tp = tcf_chain_tp_find(chain, &chain_info, protocol,
17141741
prio, prio_allocate);
17151742
if (IS_ERR(tp)) {
17161743
NL_SET_ERR_MSG(extack, "Filter with specified priority/protocol not found");
17171744
err = PTR_ERR(tp);
1718-
goto errout;
1745+
goto errout_locked;
17191746
}
17201747

17211748
if (tp == NULL) {
@@ -1724,29 +1751,37 @@ static int tc_new_tfilter(struct sk_buff *skb, struct nlmsghdr *n,
17241751
if (tca[TCA_KIND] == NULL || !protocol) {
17251752
NL_SET_ERR_MSG(extack, "Filter kind and protocol must be specified");
17261753
err = -EINVAL;
1727-
goto errout;
1754+
goto errout_locked;
17281755
}
17291756

17301757
if (!(n->nlmsg_flags & NLM_F_CREATE)) {
17311758
NL_SET_ERR_MSG(extack, "Need both RTM_NEWTFILTER and NLM_F_CREATE to create a new filter");
17321759
err = -ENOENT;
1733-
goto errout;
1760+
goto errout_locked;
17341761
}
17351762

17361763
if (prio_allocate)
1737-
prio = tcf_auto_prio(tcf_chain_tp_prev(&chain_info));
1764+
prio = tcf_auto_prio(tcf_chain_tp_prev(chain,
1765+
&chain_info));
17381766

1767+
mutex_unlock(&chain->filter_chain_lock);
17391768
tp = tcf_proto_create(nla_data(tca[TCA_KIND]),
17401769
protocol, prio, chain, extack);
17411770
if (IS_ERR(tp)) {
17421771
err = PTR_ERR(tp);
17431772
goto errout;
17441773
}
1774+
1775+
mutex_lock(&chain->filter_chain_lock);
1776+
tcf_chain_tp_insert(chain, &chain_info, tp);
1777+
mutex_unlock(&chain->filter_chain_lock);
17451778
tp_created = 1;
17461779
} else if (tca[TCA_KIND] && nla_strcmp(tca[TCA_KIND], tp->ops->kind)) {
17471780
NL_SET_ERR_MSG(extack, "Specified filter kind does not match existing one");
17481781
err = -EINVAL;
1749-
goto errout;
1782+
goto errout_locked;
1783+
} else {
1784+
mutex_unlock(&chain->filter_chain_lock);
17501785
}
17511786

17521787
fh = tp->ops->get(tp, t->tcm_handle);
@@ -1772,15 +1807,11 @@ static int tc_new_tfilter(struct sk_buff *skb, struct nlmsghdr *n,
17721807
err = tp->ops->change(net, skb, tp, cl, t->tcm_handle, tca, &fh,
17731808
n->nlmsg_flags & NLM_F_CREATE ? TCA_ACT_NOREPLACE : TCA_ACT_REPLACE,
17741809
extack);
1775-
if (err == 0) {
1776-
if (tp_created)
1777-
tcf_chain_tp_insert(chain, &chain_info, tp);
1810+
if (err == 0)
17781811
tfilter_notify(net, skb, n, tp, block, q, parent, fh,
17791812
RTM_NEWTFILTER, false);
1780-
} else {
1781-
if (tp_created)
1782-
tcf_proto_destroy(tp, NULL);
1783-
}
1813+
else if (tp_created)
1814+
tcf_proto_destroy(tp, NULL);
17841815

17851816
errout:
17861817
if (chain)
@@ -1790,6 +1821,10 @@ static int tc_new_tfilter(struct sk_buff *skb, struct nlmsghdr *n,
17901821
/* Replay the request. */
17911822
goto replay;
17921823
return err;
1824+
1825+
errout_locked:
1826+
mutex_unlock(&chain->filter_chain_lock);
1827+
goto errout;
17931828
}
17941829

17951830
static int tc_del_tfilter(struct sk_buff *skb, struct nlmsghdr *n,
@@ -1865,31 +1900,34 @@ static int tc_del_tfilter(struct sk_buff *skb, struct nlmsghdr *n,
18651900
goto errout;
18661901
}
18671902

1903+
mutex_lock(&chain->filter_chain_lock);
18681904
tp = tcf_chain_tp_find(chain, &chain_info, protocol,
18691905
prio, false);
18701906
if (!tp || IS_ERR(tp)) {
18711907
NL_SET_ERR_MSG(extack, "Filter with specified priority/protocol not found");
18721908
err = tp ? PTR_ERR(tp) : -ENOENT;
1873-
goto errout;
1909+
goto errout_locked;
18741910
} else if (tca[TCA_KIND] && nla_strcmp(tca[TCA_KIND], tp->ops->kind)) {
18751911
NL_SET_ERR_MSG(extack, "Specified filter kind does not match existing one");
18761912
err = -EINVAL;
1913+
goto errout_locked;
1914+
} else if (t->tcm_handle == 0) {
1915+
tcf_chain_tp_remove(chain, &chain_info, tp);
1916+
mutex_unlock(&chain->filter_chain_lock);
1917+
1918+
tfilter_notify(net, skb, n, tp, block, q, parent, fh,
1919+
RTM_DELTFILTER, false);
1920+
tcf_proto_destroy(tp, extack);
1921+
err = 0;
18771922
goto errout;
18781923
}
1924+
mutex_unlock(&chain->filter_chain_lock);
18791925

18801926
fh = tp->ops->get(tp, t->tcm_handle);
18811927

18821928
if (!fh) {
1883-
if (t->tcm_handle == 0) {
1884-
tcf_chain_tp_remove(chain, &chain_info, tp);
1885-
tfilter_notify(net, skb, n, tp, block, q, parent, fh,
1886-
RTM_DELTFILTER, false);
1887-
tcf_proto_destroy(tp, extack);
1888-
err = 0;
1889-
} else {
1890-
NL_SET_ERR_MSG(extack, "Specified filter handle not found");
1891-
err = -ENOENT;
1892-
}
1929+
NL_SET_ERR_MSG(extack, "Specified filter handle not found");
1930+
err = -ENOENT;
18931931
} else {
18941932
bool last;
18951933

@@ -1899,7 +1937,10 @@ static int tc_del_tfilter(struct sk_buff *skb, struct nlmsghdr *n,
18991937
if (err)
19001938
goto errout;
19011939
if (last) {
1940+
mutex_lock(&chain->filter_chain_lock);
19021941
tcf_chain_tp_remove(chain, &chain_info, tp);
1942+
mutex_unlock(&chain->filter_chain_lock);
1943+
19031944
tcf_proto_destroy(tp, extack);
19041945
}
19051946
}
@@ -1909,6 +1950,10 @@ static int tc_del_tfilter(struct sk_buff *skb, struct nlmsghdr *n,
19091950
tcf_chain_put(chain);
19101951
tcf_block_release(q, block);
19111952
return err;
1953+
1954+
errout_locked:
1955+
mutex_unlock(&chain->filter_chain_lock);
1956+
goto errout;
19121957
}
19131958

19141959
static int tc_get_tfilter(struct sk_buff *skb, struct nlmsghdr *n,
@@ -1966,8 +2011,10 @@ static int tc_get_tfilter(struct sk_buff *skb, struct nlmsghdr *n,
19662011
goto errout;
19672012
}
19682013

2014+
mutex_lock(&chain->filter_chain_lock);
19692015
tp = tcf_chain_tp_find(chain, &chain_info, protocol,
19702016
prio, false);
2017+
mutex_unlock(&chain->filter_chain_lock);
19712018
if (!tp || IS_ERR(tp)) {
19722019
NL_SET_ERR_MSG(extack, "Filter with specified priority/protocol not found");
19732020
err = tp ? PTR_ERR(tp) : -ENOENT;

‎net/sched/sch_generic.c‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1366,7 +1366,11 @@ static void mini_qdisc_rcu_func(struct rcu_head *head)
13661366
void mini_qdisc_pair_swap(struct mini_Qdisc_pair *miniqp,
13671367
struct tcf_proto *tp_head)
13681368
{
1369-
struct mini_Qdisc *miniq_old = rtnl_dereference(*miniqp->p_miniq);
1369+
/* Protected with chain0->filter_chain_lock.
1370+
* Can't access chain directly because tp_head can be NULL.
1371+
*/
1372+
struct mini_Qdisc *miniq_old =
1373+
rcu_dereference_protected(*miniqp->p_miniq, 1);
13701374
struct mini_Qdisc *miniq;
13711375

13721376
if (!tp_head) {

0 commit comments

Comments
 (0)