public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Daniel Lezcano <daniel.lezcano@free.fr>
To: Oleg Nesterov <oleg@redhat.com>
Cc: akpm@linux-foundation.org, containers@lists.linux-foundation.org,
	linux-kernel@vger.kernel.org, xemul@openvz.org,
	sukadev@us.ibm.com, ebiederm@xmission.com
Subject: Re: [PATCH 1/2] pidns: Don't allow new pids after the namespace is dead.
Date: Thu, 17 Feb 2011 00:21:12 +0100	[thread overview]
Message-ID: <4D5C5BE8.5020803@free.fr> (raw)
In-Reply-To: <20110215183028.GA15568@redhat.com>

On 02/15/2011 07:30 PM, Oleg Nesterov wrote:
> On 02/15, Daniel Lezcano wrote:
>> In the case of unsharing or joining a pid namespace, it becomes
>> possible to attempt to allocate a pid after zap_pid_namespace has
>> killed everything in the namespace.  Close the hole for now by simply
>> not allowing any of those pid allocations to succeed.
> Daniel, please explain more. It seems, a long ago I knew the reason
> for this patch, but now I can't recall and can't understand this change.

The idea behind unsharing the pid namespace is the current pid is not 
mapped in the newly created pid namespace and appears as the pid 0. When 
it forks, the child process becomes the init pid of the new pid 
namespace. When this pid namespace dies because the init pid exited, the 
parent process (aka pid 0) can no longer fork because the pid namespace 
is flagged dead. This is what does this patch.

The next patch allows a single process to spawn different processes in 
different pid namespace. You can argue we can already do that with 
clone(CLONE_NEWPID). That's true. But if we are able to unshare the pid 
namespace, then the next patchset (which will come right after this one) 
will allow to attach a process to a namespace and the implementation 
will be very simple and consistent with attaching to any namespace.

>> --- a/include/linux/pid_namespace.h
>> +++ b/include/linux/pid_namespace.h
>> @@ -20,6 +20,7 @@ struct pid_namespace {
>>   	struct kref kref;
>>   	struct pidmap pidmap[PIDMAP_ENTRIES];
>>   	int last_pid;
>> +	atomic_t dead;
> Why atomic_t? It is used as a plain boolean.
>
> And I can't unde

I think Eric used an atomic because it is lockless with alloc_pid vs 
zap_pid_ns_processes.

>> --- a/kernel/pid.c
>> +++ b/kernel/pid.c
>> @@ -282,6 +282,10 @@ struct pid *alloc_pid(struct pid_namespace *ns)
>>   	struct pid_namespace *tmp;
>>   	struct upid *upid;
>>
>> +	pid = NULL;
>> +	if (atomic_read(&ns->dead))
>> +		goto out;
>> +
> So why this is needed?
>
> If we see ns->dead != 0 we are already killed by zap_pid_ns_processes()
> which sets ns->dead = 1.

The current process unshares the pid namespace.
When it forks, the child process is the pid 1. When this one exits, the 
zap_pid_ns_processes is called and tag the pid namespace as dead. The 
current process can no longer fork.

   -- Daniel

  reply	other threads:[~2011-02-16 23:21 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-02-15 16:53 [PATCH 1/2] pidns: Don't allow new pids after the namespace is dead Daniel Lezcano
2011-02-15 16:53 ` [PATCH 2/2] pidns: Support unsharing the pid namespace Daniel Lezcano
2011-02-15 19:01   ` Oleg Nesterov
2011-02-15 19:15     ` [PATCH 0/1] Was: " Oleg Nesterov
2011-02-15 19:17       ` [PATCH 1/1][3rd resend] sys_unshare: remove the dead CLONE_THREAD/SIGHAND/VM code Oleg Nesterov
2011-02-21  0:17         ` Serge E. Hallyn
2011-02-16 23:47     ` [PATCH 2/2] pidns: Support unsharing the pid namespace Daniel Lezcano
2011-02-17 20:29       ` Oleg Nesterov
2011-02-17 22:35         ` Greg Kurz
2011-02-18 14:40           ` Oleg Nesterov
2011-02-24  1:12   ` Rob Landley
2011-02-15 18:30 ` [PATCH 1/2] pidns: Don't allow new pids after the namespace is dead Oleg Nesterov
2011-02-16 23:21   ` Daniel Lezcano [this message]
2011-02-17 20:54     ` Oleg Nesterov

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=4D5C5BE8.5020803@free.fr \
    --to=daniel.lezcano@free.fr \
    --cc=akpm@linux-foundation.org \
    --cc=containers@lists.linux-foundation.org \
    --cc=ebiederm@xmission.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=oleg@redhat.com \
    --cc=sukadev@us.ibm.com \
    --cc=xemul@openvz.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox