Mon, May 14, 2018 at 04:27:06PM CEST, vladbu@xxxxxxxxxxxx wrote: >Without rtnl lock protection it is no longer safe to use pointer to tc >action without holding reference to it. (it can be destroyed concurrently) > >Remove unsafe action idr lookup function. Instead of it, implement safe tcf >idr check function that atomically looks up action in idr and increments >its reference and bind counters. > >Implement both action search and check using new safe function. > >Signed-off-by: Vlad Buslov <vladbu@xxxxxxxxxxxx> >--- > net/sched/act_api.c | 38 ++++++++++++++++---------------------- > 1 file changed, 16 insertions(+), 22 deletions(-) > >diff --git a/net/sched/act_api.c b/net/sched/act_api.c >index 1331beb..9459cce 100644 >--- a/net/sched/act_api.c >+++ b/net/sched/act_api.c >@@ -284,44 +284,38 @@ int tcf_generic_walker(struct tc_action_net *tn, struct sk_buff *skb, > } > EXPORT_SYMBOL(tcf_generic_walker); > >-static struct tc_action *tcf_idr_lookup(u32 index, struct tcf_idrinfo *idrinfo) >+bool __tcf_idr_check(struct tc_action_net *tn, u32 index, struct tc_action **a, >+ int bind) > { >- struct tc_action *p = NULL; >+ struct tcf_idrinfo *idrinfo = tn->idrinfo; >+ struct tc_action *p; > > spin_lock_bh(&idrinfo->lock); Why "_bh" variant is necessary here? > p = idr_find(&idrinfo->action_idr, index); >+ if (p) { >+ refcount_inc(&p->tcfa_refcnt); >+ if (bind) >+ atomic_inc(&p->tcfa_bindcnt); >+ } > spin_unlock_bh(&idrinfo->lock); [...] -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html