[PATCH] namespace:unmount pid_namespace's proc_mnt when copy_net_ns failed

[PATCH] namespace:unmount pid_namespace's proc_mnt when copy_net_ns failed

[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

Re: [PATCH] namespace:unmount pid_namespace's proc_mnt when copy_net_ns failed

[Eric W. Biederman]

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);

Re: [PATCH] namespace:unmount pid_namespace's proc_mnt when copy_net_ns failed

[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'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);

Re: [PATCH] namespace:unmount pid_namespace's proc_mnt when copy_net_ns failed

[Gao feng]

于 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

Re: [PATCH] namespace:unmount pid_namespace's proc_mnt when copy_net_ns failed

[Gao feng]

于 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/
> 

Re: [PATCH] namespace:unmount pid_namespace's proc_mnt when copy_net_ns failed

[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?

Eric
_______________________________________________
Containers mailing list
Containers@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/containers

Re: [PATCH] namespace:unmount pid_namespace's proc_mnt when copy_net_ns failed

[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?

Eric

Re: [PATCH] namespace:unmount pid_namespace's proc_mnt when copy_net_ns failed

[Gao feng]

于 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

Re: [PATCH] namespace:unmount pid_namespace's proc_mnt when copy_net_ns failed

[Gao feng]

于 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

Re: [PATCH] namespace:unmount pid_namespace's proc_mnt when copy_net_ns failed

[Eric W. Biederman]

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

Re: [PATCH] namespace:unmount pid_namespace's proc_mnt when copy_net_ns failed

[Eric W. Biederman]

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

[PATCH] namespace:unmount pid_namespace's proc_mnt when copy_net_ns failed

[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-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