On Sun, Jan 15, 2017 at 7:14 AM, Jamal Hadi Salim <j...@mojatatu.com> wrote: > diff --git a/net/sched/act_api.c b/net/sched/act_api.c > index 2095c83..e10456ef6f 100644 > --- a/net/sched/act_api.c > +++ b/net/sched/act_api.c > @@ -900,8 +900,6 @@ static int tca_action_flush(struct net *net, struct > nlattr *nla, > goto err; > } > act->order = i; > - if (event == RTM_GETACTION) > - act->tcfa_refcnt++; > list_add_tail(&act->list, &actions); > } > > @@ -914,7 +912,8 @@ static int tca_action_flush(struct net *net, struct > nlattr *nla, > return ret; > } > err: > - tcf_action_destroy(&actions, 0); > + if (event != RTM_GETACTION) > + tcf_action_destroy(&actions, 0);
Why this check for RTM_GETACTION? It does not make sense at least for the error case, that is, when tcf_action_get_1() fails in the middle of the loop, all the previous ones should be destroyed no matter it's GETACTION or DELACTION... Also, for the non-error case of GETACTION, they should be destroyed too after dumping to user-space, otherwise there is no way to use them since 'actions' is local to that function. I thought in commit aecc5cefc389 you grab that refcnt on purpose.
