Hi Jamal, On Sun, Jan 22, 2017 at 03:25:50PM -0500, Jamal Hadi Salim wrote:
... > diff --git a/net/sched/act_api.c b/net/sched/act_api.c > index cd08df9..58cf1c5 100644 > --- a/net/sched/act_api.c > +++ b/net/sched/act_api.c > @@ -24,6 +24,7 @@ > #include <net/net_namespace.h> > #include <net/sock.h> > #include <net/sch_generic.h> > +#include <net/pkt_cls.h> > #include <net/act_api.h> > #include <net/netlink.h> > > @@ -33,6 +34,8 @@ static void free_tcf(struct rcu_head *head) > > free_percpu(p->cpu_bstats); > free_percpu(p->cpu_qstats); > + kfree(p->act_cookie->data); Does the above need to be protected by a check for p->act_cookie being non-NULL? > + kfree(p->act_cookie); > kfree(p); > } > ... > @@ -575,6 +584,33 @@ struct tc_action *tcf_action_init_1(struct net *net, > struct nlattr *nla, > if (err < 0) > goto err_mod; > > + if (tb[TCA_ACT_COOKIE]) { > + int cklen = nla_len(tb[TCA_ACT_COOKIE]); > + > + if (cklen > TC_COOKIE_MAX_SIZE) { > + err = -EINVAL; > + tcf_hash_release(a, bind); > + goto err_mod; > + } > + > + a->act_cookie = kzalloc(sizeof(*a->act_cookie), GFP_KERNEL); > + if (!a->act_cookie) { > + err = -ENOMEM; > + tcf_hash_release(a, bind); > + goto err_mod; > + } > + > + a->act_cookie->data = nla_memdup(tb[TCA_ACT_COOKIE], > + GFP_KERNEL); > + if (!a->act_cookie->data) { > + err = -ENOMEM; > + kfree(a->act_cookie); > + tcf_hash_release(a, bind); > + goto err_mod; > + } > + a->act_cookie->len = cklen; FWIW, the above looks correct but it also looks like the error handling could be done less verbosely if the logic was moved to a separate function. > + } > + > /* module count goes up only when brand new policy is created > * if it exists and is only bound to in a_o->init() then > * ACT_P_CREATED is not returned (a zero is). > -- > 1.9.1 >