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
| ||
|
Date: Wed, 19 Apr 2017 07:32:38 -0400 From: Jamal Hadi Salim <jhs@...atatu.com> To: Wolfgang Bumiller <w.bumiller@...xmox.com>, Cong Wang <xiyou.wangcong@...il.com> Cc: Linux Kernel Network Developers <netdev@...r.kernel.org>, "David S. Miller" <davem@...emloft.net> Subject: Re: [PATCH v2 net 2/2] net sched actions: decrement module refcount earlier On 17-04-19 04:09 AM, Wolfgang Bumiller wrote: > > diff --git a/net/sched/act_api.c b/net/sched/act_api.c > index 8cc883c063f0..c7e5e437b847 100644 > --- a/net/sched/act_api.c > +++ b/net/sched/act_api.c > @@ -555,6 +555,7 @@ struct tc_action *tcf_action_init_1(struct net *net, struct nlattr *nla, > struct nlattr *tb[TCA_ACT_MAX + 1]; > struct nlattr *kind; > int err; > + bool created; > > if (name == NULL) { > err = nla_parse_nested(tb, TCA_ACT_MAX, nla, NULL); > @@ -603,20 +604,19 @@ struct tc_action *tcf_action_init_1(struct net *net, struct nlattr *nla, > err = a_o->init(net, nla, est, &a, ovr, bind); > if (err < 0) > goto err_mod; > + created = err == ACT_P_CREATED; > > if (name == NULL && 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; > + goto err_release; > } > > if (nla_memdup_cookie(a, tb) < 0) { > err = -ENOMEM; > - tcf_hash_release(a, bind); > - goto err_mod; > + goto err_release; > } > } > > @@ -624,11 +624,14 @@ struct tc_action *tcf_action_init_1(struct net *net, struct nlattr *nla, > * if it exists and is only bound to in a_o->init() then > * ACT_P_CREATED is not returned (a zero is). > */ > - if (err != ACT_P_CREATED) > + if (!created) > module_put(a_o->owner); > > return a; > > +err_release: > + if (created) > + tcf_hash_release(a, bind); > err_mod: > module_put(a_o->owner); > err_out: > This solves one issue, but I am afraid the issue Cong mentioned is a possibility still. Lets say user did a replace and tried to also replace the cookie in that transaction. The init() succeeds but the cookie allocation fails. To be correct we'll have to undo the replace i.e something like uninit() which will restore back the old values. This is very complex and unnecessary. My suggestion: If we move the cookie allocation before init we can save it and only when init succeeds do we attach it to the action, otherwise we free it on error path. cheers, jamal
Powered by blists - more mailing lists