| Date | Thu, 8 Oct 2015 17:06:57 +0200 | Subject | Re: [PATCH 38/44] kdbus: Fix error path in kdbus_user_lookup() | From | David Herrmann <> |
| |
Hi
On Thu, Oct 8, 2015 at 1:32 PM, Sergei Zviagintsev <sergei@s15v.net> wrote: > If idr_alloc() fails, we shouldn't call idr_remove() as latter produces > warning when called on non-allocated ids. Split cleanup code into three > parts for differrent cleanup scenarios before and after idr_alloc(). > > Signed-off-by: Sergei Zviagintsev <sergei@s15v.net> > --- > ipc/kdbus/domain.c | 20 ++++++++++---------- > 1 file changed, 10 insertions(+), 10 deletions(-) > > diff --git a/ipc/kdbus/domain.c b/ipc/kdbus/domain.c > index ac9f760c150d..31cd09fb572f 100644 > --- a/ipc/kdbus/domain.c > +++ b/ipc/kdbus/domain.c > @@ -208,7 +208,7 @@ struct kdbus_user *kdbus_user_lookup(struct kdbus_domain *domain, kuid_t uid) > u = kzalloc(sizeof(*u), GFP_KERNEL); > if (!u) { > ret = -ENOMEM; > - goto exit; > + goto exit_unlock; > } > > kref_init(&u->kref); > @@ -225,7 +225,7 @@ struct kdbus_user *kdbus_user_lookup(struct kdbus_domain *domain, kuid_t uid) > ret = idr_alloc(&domain->user_idr, u, __kuid_val(uid), > __kuid_val(uid) + 1, GFP_KERNEL); > if (ret < 0) > - goto exit; > + goto exit_free; > } > } > > @@ -235,19 +235,19 @@ struct kdbus_user *kdbus_user_lookup(struct kdbus_domain *domain, kuid_t uid) > */ > ret = ida_simple_get(&domain->user_ida, 1, 0, GFP_KERNEL); > if (ret < 0) > - goto exit; > + goto exit_idr; >
Why not simply assign "u->uid = uid;" _after_ doing the idr operations?
Thanks David
> u->id = ret; > mutex_unlock(&domain->lock); > return u; > > -exit: > - if (u) { > - if (uid_valid(u->uid)) > - idr_remove(&domain->user_idr, __kuid_val(u->uid)); > - kdbus_domain_unref(u->domain); > - kfree(u); > - } > +exit_idr: > + if (uid_valid(u->uid)) > + idr_remove(&domain->user_idr, __kuid_val(u->uid)); > +exit_free: > + kdbus_domain_unref(u->domain); > + kfree(u); > +exit_unlock: > mutex_unlock(&domain->lock); > return ERR_PTR(ret); > } > -- > 1.8.3.1 >
|