Re: [PATCH 11/11] pidns: Support unsharing the pid namespace.

From: Oleg Nesterov
Date: Fri Dec 21 2012 - 10:49:24 EST


On 12/20, Eric W. Biederman wrote:
>
> Oleg Nesterov <oleg@xxxxxxxxxx> writes:
>
> > So alloc_pid() becomes the only user nsproxy->pid_ns and it is not
> > necessarily equal to task_active_pid_ns(). It seems to me that this
> > adds a lot of new corner cases.
>
> I have tried to simply outlaw the most of the new corner cases as they
> simply are not interesting so there is no point implementing them,
> or thinking about them once they are outlawed.

Eric. I understand that it is too late to discuss this. And yes, I simply
do not understand the problem space, I never used containers.

But, stupid question. Let's ignore the pid_ns-specific oddities.

1. Ignoring setns(), why do we need /proc/pid/ns/ ?

2. Why setns() requires /proc/pid/ns/ ? IOW, why it can't be

sys_setns(pid_t pid, int clone_flags)
{
truct task_struct *tsk = find_task_by_vpid(pid);
struct nsproxy *target = get_nsproxy(tsk->nsproxy);

new_nsproxy = create_new_namespaces(...);

if (clone_flags & CLONE_NEWNS)
mntns_install(...);
if (clone_flags & CLONE_NEWIPC)
ipcns_install(...);
...
}

I feel I missed something trivial, but what?

> @@ -1166,6 +1166,14 @@ static struct task_struct *copy_process(unsigned long clone_flags,
> current->signal->flags & SIGNAL_UNKILLABLE)
> return ERR_PTR(-EINVAL);
>
> + /*
> + * If the children will be in a different pid namespace don't allow
> + * the creation of threads.
> + */
> + if ((clone_flags & (CLONE_THREAD|CLONE_SIGHAND|CLONE_VM|CLONE_PARENT)) &&
> + task_active_pid_ns(current) != current->nsproxy->pid_ns)
> + return ERR_PTR(-EINVAL);

Agreed, and this also removes other oddities with pthread_create().

Oleg.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/