All of lore.kernel.org
 help / color / mirror / Atom feed
From: Gao feng <gaofeng-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>
To: "Eric W. Biederman" <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
Cc: containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Subject: Re: [PATCH] namespace:unmount pid_namespace's proc_mnt when copy_net_ns failed
Date: Fri, 02 Nov 2012 15:33:15 +0800	[thread overview]
Message-ID: <5093773B.5010706@cn.fujitsu.com> (raw)
In-Reply-To: <87ehkcij1a.fsf-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>

于 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

WARNING: multiple messages have this Message-ID (diff)
From: Gao feng <gaofeng@cn.fujitsu.com>
To: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: linux-kernel@vger.kernel.org, containers@lists.linux-foundation.org
Subject: Re: [PATCH] namespace:unmount pid_namespace's proc_mnt when copy_net_ns failed
Date: Fri, 02 Nov 2012 15:33:15 +0800	[thread overview]
Message-ID: <5093773B.5010706@cn.fujitsu.com> (raw)
In-Reply-To: <87ehkcij1a.fsf@xmission.com>

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


  parent reply	other threads:[~2012-11-02  7:33 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-11-02  0:38 [PATCH] namespace:unmount pid_namespace's proc_mnt when copy_net_ns failed Gao feng
     [not found] ` <1351816703-8805-1-git-send-email-gaofeng-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>
2012-11-02  7:02   ` Eric W. Biederman
2012-11-02  7:02     ` Eric W. Biederman
     [not found]     ` <87ehkcij1a.fsf-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
2012-11-02  7:33       ` Gao feng [this message]
2012-11-02  7:33         ` Gao feng
     [not found]         ` <5093773B.5010706-BthXqXjhjHXQFUHtdCDX3A@public.gmane.org>
2012-11-02  8:54           ` Eric W. Biederman
2012-11-02  8:54             ` Eric W. Biederman
     [not found]             ` <87wqy4fkqx.fsf-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
2012-11-02  9:02               ` Gao feng
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  9:10                     ` Eric W. Biederman
  -- strict thread matches above, loose matches on Subject: below --
2012-11-02  0:38 Gao feng

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5093773B.5010706@cn.fujitsu.com \
    --to=gaofeng-bthxqxjhjhxqfuhtdcdx3a@public.gmane.org \
    --cc=containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org \
    --cc=ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org \
    --cc=linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.