Messages in this thread | | | Date | Mon, 23 Feb 2009 14:33:56 -0600 | From | "Serge E. Hallyn" <> | Subject | Re: [PATCH] Fix recursive lock in free_uid()/free_user_ns() |
| |
Quoting David Howells (dhowells@redhat.com): > free_uid() and free_user_ns() are corecursive when CONFIG_USER_SCHED=n, but > free_user_ns() is called from free_uid() by way of uid_hash_remove(), which > requires uidhash_lock to be held. free_user_ns() then calls free_uid() to > complete the destruction. > > Fix this by deferring the destruction of the user_namespace.
Crap. I think when I did this I looked at put_user_ns() I though it was calling free_user_ns() through an rcu callback (mis-read kref_put).
> Signed-off-by: David Howells <dhowells@redhat.com>
Acked-by: Serge Hallyn <serue@us.ibm.com>
Thanks, David and Alexey.
-serge
> --- > > include/linux/user_namespace.h | 1 + > kernel/user_namespace.c | 21 +++++++++++++++++---- > 2 files changed, 18 insertions(+), 4 deletions(-) > > > diff --git a/include/linux/user_namespace.h b/include/linux/user_namespace.h > index 315bcd3..cc4f453 100644 > --- a/include/linux/user_namespace.h > +++ b/include/linux/user_namespace.h > @@ -13,6 +13,7 @@ struct user_namespace { > struct kref kref; > struct hlist_head uidhash_table[UIDHASH_SZ]; > struct user_struct *creator; > + struct work_struct destroyer; > }; > > extern struct user_namespace init_user_ns; > diff --git a/kernel/user_namespace.c b/kernel/user_namespace.c > index 7908431..076c7c8 100644 > --- a/kernel/user_namespace.c > +++ b/kernel/user_namespace.c > @@ -60,12 +60,25 @@ int create_user_ns(struct cred *new) > return 0; > } > > -void free_user_ns(struct kref *kref) > +/* > + * Deferred destructor for a user namespace. This is required because > + * free_user_ns() may be called with uidhash_lock held, but we need to call > + * back to free_uid() which will want to take the lock again. > + */ > +static void free_user_ns_work(struct work_struct *work) > { > - struct user_namespace *ns; > - > - ns = container_of(kref, struct user_namespace, kref); > + struct user_namespace *ns = > + container_of(work, struct user_namespace, destroyer); > free_uid(ns->creator); > kfree(ns); > } > + > +void free_user_ns(struct kref *kref) > +{ > + struct user_namespace *ns = > + container_of(kref, struct user_namespace, kref); > + > + INIT_WORK(&ns->destroyer, free_user_ns_work); > + schedule_work(&ns->destroyer); > +} > EXPORT_SYMBOL(free_user_ns);
| |