Quoting Cedric Le Goater ([EMAIL PROTECTED]): > Cedric Le Goater wrote: > > Pavel Emelyanov wrote: > >> Looks sane :) > >> > >> [snip] > >> > >>> Index: 2.6.23-rc6-mm1/kernel/exit.c > >>> =================================================================== > >>> --- 2.6.23-rc6-mm1.orig/kernel/exit.c > >>> +++ 2.6.23-rc6-mm1/kernel/exit.c > >>> @@ -408,6 +408,8 @@ void daemonize(const char *name, ...) > >>> current->fs = fs; > >>> atomic_inc(&fs->count); > >>> > >>> + if (current->nsproxy != init_task.nsproxy) > >>> + get_nsproxy(init_task.nsproxy); > >>> switch_task_namespaces(current, init_task.nsproxy); > >> shouldn't we make the switch under this if() as well? > > > > right. we can probably simplify switch_task_namespaces() and remove : > > > > if (ns == new) > > return; > > > > I'll cook a better one today. > > So I removed this test in > > * daemonize() bc it is already done > * sys_unshare() bc the nsproxy is always new one > * exit_task_namespaces() bc it is called with NULL and the > task will die right after that. > > C. > > > make-access-to-tasks-nsproxy-lighter.patch breaks unshare() > > when called from unshare(), switch_task_namespaces() takes an > extra refcount on the nsproxy, leading to a memory leak of > nsproxy objects. > > Now the problem is that we still need that extra ref when called > from daemonize(). Here's an ugly fix for it. > > Signed-off-by: Cedric Le Goater <[EMAIL PROTECTED]> > Cc: Serge E. Hallyn <[EMAIL PROTECTED]>
Looks good. Thanks for catching the leak. Acked-by: Serge E. Hallyn <[EMAIL PROTECTED]> > Cc: Pavel Emelyanov <[EMAIL PROTECTED]> > Cc: Eric W. Biederman <[EMAIL PROTECTED]> > Cc: Oleg Nesterov <[EMAIL PROTECTED]> > Cc: Paul E. McKenney <[EMAIL PROTECTED]> > > --- > include/linux/nsproxy.h | 5 +++++ > kernel/exit.c | 5 ++++- > kernel/nsproxy.c | 9 --------- > 3 files changed, 9 insertions(+), 10 deletions(-) > > Index: 2.6.23-rc6-mm1/kernel/nsproxy.c > =================================================================== > --- 2.6.23-rc6-mm1.orig/kernel/nsproxy.c > +++ 2.6.23-rc6-mm1/kernel/nsproxy.c > @@ -25,11 +25,6 @@ static struct kmem_cache *nsproxy_cachep > > struct nsproxy init_nsproxy = INIT_NSPROXY(init_nsproxy); > > -static inline void get_nsproxy(struct nsproxy *ns) > -{ > - atomic_inc(&ns->count); > -} > - > /* > * creates a copy of "orig" with refcount 1. > */ > @@ -205,11 +200,7 @@ void switch_task_namespaces(struct task_ > might_sleep(); > > ns = p->nsproxy; > - if (ns == new) > - return; > > - if (new) > - get_nsproxy(new); > rcu_assign_pointer(p->nsproxy, new); > > if (ns && atomic_dec_and_test(&ns->count)) { > Index: 2.6.23-rc6-mm1/kernel/exit.c > =================================================================== > --- 2.6.23-rc6-mm1.orig/kernel/exit.c > +++ 2.6.23-rc6-mm1/kernel/exit.c > @@ -408,7 +408,10 @@ void daemonize(const char *name, ...) > current->fs = fs; > atomic_inc(&fs->count); > > - switch_task_namespaces(current, init_task.nsproxy); > + if (current->nsproxy != init_task.nsproxy) { > + get_nsproxy(init_task.nsproxy); > + switch_task_namespaces(current, init_task.nsproxy); > + } > > exit_files(current); > current->files = init_task.files; > Index: 2.6.23-rc6-mm1/include/linux/nsproxy.h > =================================================================== > --- 2.6.23-rc6-mm1.orig/include/linux/nsproxy.h > +++ 2.6.23-rc6-mm1/include/linux/nsproxy.h > @@ -77,6 +77,11 @@ static inline void put_nsproxy(struct ns > } > } > > +static inline void get_nsproxy(struct nsproxy *ns) > +{ > + atomic_inc(&ns->count); > +} > + > #ifdef CONFIG_CONTAINER_NS > int ns_container_clone(struct task_struct *tsk); > #else - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/