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/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
