* Re: [PATCH] namespace:unmount pid_namespace's proc_mnt when copy_net_ns failed [not found] ` <1351816703-8805-1-git-send-email-gaofeng-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org> @ 2012-11-02 7:02 ` Eric W. Biederman [not found] ` <87ehkcij1a.fsf-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org> 0 siblings, 1 reply; 6+ messages in thread From: Eric W. Biederman @ 2012-11-02 7:02 UTC (permalink / raw) To: Gao feng Cc: containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, linux-kernel-u79uwXL29TY76Z2rM5mHXA Gao feng <gaofeng-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org> writes: > we should call pid_ns_release_proc to unmount pid_namespace's > proc_mnt when copy_net_ns failed in function create_new_namespaces. > > otherwise,the proc_mnt will not be freed and because the super_block > of proc_mnt also add the reference of the pid_namespace,so this > pid_namespace will never be released too. Ouch! Have you encountered this failure in practice or is this just from review? I'm trying to gauge the severity of this leak. Eric > Signed-off-by: Gao feng <gaofeng-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org> > --- > kernel/nsproxy.c | 5 ++++- > 1 files changed, 4 insertions(+), 1 deletions(-) > > diff --git a/kernel/nsproxy.c b/kernel/nsproxy.c > index b576f7f..d536480 100644 > --- a/kernel/nsproxy.c > +++ b/kernel/nsproxy.c > @@ -99,8 +99,11 @@ static struct nsproxy *create_new_namespaces(unsigned long flags, > return new_nsp; > > out_net: > - if (new_nsp->pid_ns) > + if (new_nsp->pid_ns) { > + if (flags & CLONE_NEWPID) > + pid_ns_release_proc(new_nsp->pid_ns); > put_pid_ns(new_nsp->pid_ns); > + } > out_pid: > if (new_nsp->ipc_ns) > put_ipc_ns(new_nsp->ipc_ns); ^ permalink raw reply [flat|nested] 6+ messages in thread
[parent not found: <87ehkcij1a.fsf-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>]
* Re: [PATCH] namespace:unmount pid_namespace's proc_mnt when copy_net_ns failed [not found] ` <87ehkcij1a.fsf-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org> @ 2012-11-02 7:33 ` Gao feng [not found] ` <5093773B.5010706-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org> 0 siblings, 1 reply; 6+ messages in thread From: Gao feng @ 2012-11-02 7:33 UTC (permalink / raw) To: Eric W. Biederman Cc: containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, linux-kernel-u79uwXL29TY76Z2rM5mHXA 于 2012年11月02日 15:02, Eric W. Biederman 写道: > Gao feng <gaofeng@cn.fujitsu.com> writes: > >> we should call pid_ns_release_proc to unmount pid_namespace's >> proc_mnt when copy_net_ns failed in function create_new_namespaces. >> >> otherwise,the proc_mnt will not be freed and because the super_block >> of proc_mnt also add the reference of the pid_namespace,so this >> pid_namespace will never be released too. > > Ouch! > > Have you encountered this failure in practice or is this just from > review? I add some printk in pid_ns_release_proc,it's not called in above case. when copy_net_ns failed,this pid_namespace is not used by any task, so proc_flush_task can't call pid_ns_release_proc to umount this pidns->proc_mnt. it's the only chance we can unmount this pindns->proc_mnt. With this patch,everything runs well. Thanks Gao > > I'm trying to gauge the severity of this leak. > > Eric > > >> Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com> >> --- >> kernel/nsproxy.c | 5 ++++- >> 1 files changed, 4 insertions(+), 1 deletions(-) >> >> diff --git a/kernel/nsproxy.c b/kernel/nsproxy.c >> index b576f7f..d536480 100644 >> --- a/kernel/nsproxy.c >> +++ b/kernel/nsproxy.c >> @@ -99,8 +99,11 @@ static struct nsproxy *create_new_namespaces(unsigned long flags, >> return new_nsp; >> >> out_net: >> - if (new_nsp->pid_ns) >> + if (new_nsp->pid_ns) { >> + if (flags & CLONE_NEWPID) >> + pid_ns_release_proc(new_nsp->pid_ns); >> put_pid_ns(new_nsp->pid_ns); >> + } >> out_pid: >> if (new_nsp->ipc_ns) >> put_ipc_ns(new_nsp->ipc_ns); > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ > _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/containers ^ permalink raw reply [flat|nested] 6+ messages in thread
[parent not found: <5093773B.5010706-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>]
* Re: [PATCH] namespace:unmount pid_namespace's proc_mnt when copy_net_ns failed [not found] ` <5093773B.5010706-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org> @ 2012-11-02 8:54 ` Eric W. Biederman [not found] ` <87wqy4fkqx.fsf-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org> 0 siblings, 1 reply; 6+ messages in thread From: Eric W. Biederman @ 2012-11-02 8:54 UTC (permalink / raw) To: Gao feng Cc: containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, linux-kernel-u79uwXL29TY76Z2rM5mHXA Gao feng <gaofeng@cn.fujitsu.com> writes: > 于 2012年11月02日 15:02, Eric W. Biederman 写道: >> Gao feng <gaofeng@cn.fujitsu.com> writes: >> >>> we should call pid_ns_release_proc to unmount pid_namespace's >>> proc_mnt when copy_net_ns failed in function create_new_namespaces. >>> >>> otherwise,the proc_mnt will not be freed and because the super_block >>> of proc_mnt also add the reference of the pid_namespace,so this >>> pid_namespace will never be released too. >> >> Ouch! >> >> Have you encountered this failure in practice or is this just from >> review? > > I add some printk in pid_ns_release_proc,it's not called in above case. > when copy_net_ns failed,this pid_namespace is not used by any task, > so proc_flush_task can't call pid_ns_release_proc to umount this pidns->proc_mnt. > it's the only chance we can unmount this pindns->proc_mnt. > > With this patch,everything runs well. I have reviewed the code and I don't doubt that this is necessary. What caused you to look into this failure? Is there some semi-practical real world case that someone is hitting? Eric _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/containers ^ permalink raw reply [flat|nested] 6+ messages in thread
[parent not found: <87wqy4fkqx.fsf-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>]
* Re: [PATCH] namespace:unmount pid_namespace's proc_mnt when copy_net_ns failed [not found] ` <87wqy4fkqx.fsf-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org> @ 2012-11-02 9:02 ` Gao feng [not found] ` <50938C2D.90908-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org> 0 siblings, 1 reply; 6+ messages in thread From: Gao feng @ 2012-11-02 9:02 UTC (permalink / raw) To: Eric W. Biederman Cc: containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, linux-kernel-u79uwXL29TY76Z2rM5mHXA 于 2012年11月02日 16:54, Eric W. Biederman 写道: > Gao feng <gaofeng@cn.fujitsu.com> writes: > >> 于 2012年11月02日 15:02, Eric W. Biederman 写道: >>> Gao feng <gaofeng@cn.fujitsu.com> writes: >>> >>>> we should call pid_ns_release_proc to unmount pid_namespace's >>>> proc_mnt when copy_net_ns failed in function create_new_namespaces. >>>> >>>> otherwise,the proc_mnt will not be freed and because the super_block >>>> of proc_mnt also add the reference of the pid_namespace,so this >>>> pid_namespace will never be released too. >>> >>> Ouch! >>> >>> Have you encountered this failure in practice or is this just from >>> review? >> >> I add some printk in pid_ns_release_proc,it's not called in above case. >> when copy_net_ns failed,this pid_namespace is not used by any task, >> so proc_flush_task can't call pid_ns_release_proc to umount this pidns->proc_mnt. >> it's the only chance we can unmount this pindns->proc_mnt. >> >> With this patch,everything runs well. > > I have reviewed the code and I don't doubt that this is necessary. > > What caused you to look into this failure? Is there some semi-practical > real world case that someone is hitting? > So far,there is no case hitting this problem. Thanks _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/containers ^ permalink raw reply [flat|nested] 6+ messages in thread
[parent not found: <50938C2D.90908-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>]
* Re: [PATCH] namespace:unmount pid_namespace's proc_mnt when copy_net_ns failed [not found] ` <50938C2D.90908-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org> @ 2012-11-02 9:10 ` Eric W. Biederman 0 siblings, 0 replies; 6+ messages in thread From: Eric W. Biederman @ 2012-11-02 9:10 UTC (permalink / raw) To: Gao feng Cc: containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA, linux-kernel-u79uwXL29TY76Z2rM5mHXA Gao feng <gaofeng@cn.fujitsu.com> writes: > 于 2012年11月02日 16:54, Eric W. Biederman 写道: >> Gao feng <gaofeng@cn.fujitsu.com> writes: >> >>> 于 2012年11月02日 15:02, Eric W. Biederman 写道: >>>> Gao feng <gaofeng@cn.fujitsu.com> writes: >>>> >>>>> we should call pid_ns_release_proc to unmount pid_namespace's >>>>> proc_mnt when copy_net_ns failed in function create_new_namespaces. >>>>> >>>>> otherwise,the proc_mnt will not be freed and because the super_block >>>>> of proc_mnt also add the reference of the pid_namespace,so this >>>>> pid_namespace will never be released too. >>>> >>>> Ouch! >>>> >>>> Have you encountered this failure in practice or is this just from >>>> review? >>> >>> I add some printk in pid_ns_release_proc,it's not called in above case. >>> when copy_net_ns failed,this pid_namespace is not used by any task, >>> so proc_flush_task can't call pid_ns_release_proc to umount this pidns->proc_mnt. >>> it's the only chance we can unmount this pindns->proc_mnt. >>> >>> With this patch,everything runs well. >> >> I have reviewed the code and I don't doubt that this is necessary. >> >> What caused you to look into this failure? Is there some semi-practical >> real world case that someone is hitting? >> > > So far,there is no case hitting this problem. I think what I want to do is to push the changes in my userns development tree instead of solving it the way you have solved it. The way things are currently structured I don't think we can be certain of finding all of the corner cases. Would you be interesting in confirming that problem does not exist in my userns development branch? Eric _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/containers ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] namespace:unmount pid_namespace's proc_mnt when copy_net_ns failed @ 2012-11-02 0:38 Gao feng 0 siblings, 0 replies; 6+ messages in thread From: Gao feng @ 2012-11-02 0:38 UTC (permalink / raw) To: linux-kernel-u79uwXL29TY76Z2rM5mHXA, containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA Cc: ebiederm-aS9lmoZGLiVWk0Htik3J/w we should call pid_ns_release_proc to unmount pid_namespace's proc_mnt when copy_net_ns failed in function create_new_namespaces. otherwise,the proc_mnt will not be freed and because the super_block of proc_mnt also add the reference of the pid_namespace,so this pid_namespace will never be released too. Signed-off-by: Gao feng <gaofeng-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org> --- kernel/nsproxy.c | 5 ++++- 1 files changed, 4 insertions(+), 1 deletions(-) diff --git a/kernel/nsproxy.c b/kernel/nsproxy.c index b576f7f..d536480 100644 --- a/kernel/nsproxy.c +++ b/kernel/nsproxy.c @@ -99,8 +99,11 @@ static struct nsproxy *create_new_namespaces(unsigned long flags, return new_nsp; out_net: - if (new_nsp->pid_ns) + if (new_nsp->pid_ns) { + if (flags & CLONE_NEWPID) + pid_ns_release_proc(new_nsp->pid_ns); put_pid_ns(new_nsp->pid_ns); + } out_pid: if (new_nsp->ipc_ns) put_ipc_ns(new_nsp->ipc_ns); -- 1.7.7.6 ^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-11-02 9:10 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1351816703-8805-1-git-send-email-gaofeng@cn.fujitsu.com>
[not found] ` <1351816703-8805-1-git-send-email-gaofeng-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>
2012-11-02 7:02 ` [PATCH] namespace:unmount pid_namespace's proc_mnt when copy_net_ns failed Eric W. Biederman
[not found] ` <87ehkcij1a.fsf-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
2012-11-02 7:33 ` Gao feng
[not found] ` <5093773B.5010706-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>
2012-11-02 8:54 ` Eric W. Biederman
[not found] ` <87wqy4fkqx.fsf-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
2012-11-02 9:02 ` Gao feng
[not found] ` <50938C2D.90908-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>
2012-11-02 9:10 ` Eric W. Biederman
2012-11-02 0:38 Gao feng
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox