* [PATCH] namespace:unmount pid_namespace's proc_mnt when copy_net_ns failed
@ 2012-11-02 0:38 Gao feng
2012-11-02 7:02 ` Eric W. Biederman
0 siblings, 1 reply; 6+ messages in thread
From: Gao feng @ 2012-11-02 0:38 UTC (permalink / raw)
To: linux-kernel, containers; +Cc: serge.hallyn, ebiederm, glommer, Gao feng
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@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);
--
1.7.7.6
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] namespace:unmount pid_namespace's proc_mnt when copy_net_ns failed
2012-11-02 0:38 [PATCH] namespace:unmount pid_namespace's proc_mnt when copy_net_ns failed Gao feng
@ 2012-11-02 7:02 ` Eric W. Biederman
2012-11-02 7:33 ` Gao feng
0 siblings, 1 reply; 6+ messages in thread
From: Eric W. Biederman @ 2012-11-02 7:02 UTC (permalink / raw)
To: Gao feng; +Cc: linux-kernel, containers
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'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);
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] namespace:unmount pid_namespace's proc_mnt when copy_net_ns failed
2012-11-02 7:02 ` Eric W. Biederman
@ 2012-11-02 7:33 ` Gao feng
2012-11-02 8:54 ` Eric W. Biederman
0 siblings, 1 reply; 6+ messages in thread
From: Gao feng @ 2012-11-02 7:33 UTC (permalink / raw)
To: Eric W. Biederman; +Cc: linux-kernel, containers
于 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/
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] namespace:unmount pid_namespace's proc_mnt when copy_net_ns failed
2012-11-02 7:33 ` Gao feng
@ 2012-11-02 8:54 ` Eric W. Biederman
2012-11-02 9:02 ` Gao feng
0 siblings, 1 reply; 6+ messages in thread
From: Eric W. Biederman @ 2012-11-02 8:54 UTC (permalink / raw)
To: Gao feng; +Cc: linux-kernel, containers
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] namespace:unmount pid_namespace's proc_mnt when copy_net_ns failed
2012-11-02 8:54 ` Eric W. Biederman
@ 2012-11-02 9:02 ` Gao feng
2012-11-02 9:10 ` Eric W. Biederman
0 siblings, 1 reply; 6+ messages in thread
From: Gao feng @ 2012-11-02 9:02 UTC (permalink / raw)
To: Eric W. Biederman; +Cc: linux-kernel, containers
于 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] namespace:unmount pid_namespace's proc_mnt when copy_net_ns failed
2012-11-02 9:02 ` Gao feng
@ 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: linux-kernel, containers
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
^ permalink raw reply [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 --
2012-11-02 0:38 [PATCH] namespace:unmount pid_namespace's proc_mnt when copy_net_ns failed Gao feng
2012-11-02 7:02 ` Eric W. Biederman
2012-11-02 7:33 ` Gao feng
2012-11-02 8:54 ` Eric W. Biederman
2012-11-02 9:02 ` Gao feng
2012-11-02 9:10 ` Eric W. Biederman
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox