Re: [RFC PATCH 3/8] kmod - teach call_usermodehelper() to use a namespace

[Ian Kent]

On Mon, 2015-02-16 at 19:24 +0100, Oleg Nesterov wrote:
> On 02/16, Oleg Nesterov wrote:
> >
> > On 02/16, Ian Kent wrote:
> > >
> > > On Tue, 2015-02-10 at 17:55 +0100, Oleg Nesterov wrote:
> > > > On 02/10, Ian Kent wrote:
> > > > >
> > > > > On Mon, 2015-02-09 at 17:03 +0100, Oleg Nesterov wrote:
> > > > > >
> > > > > > I understand. but I still can't understand why we can't implement something
> > > > > > like
> > > > > > 	enter_ns(struct nsproxy *p)
> > > > > > 	{
> > > > > > 		new_nsproxy = create_new_namespaces(...);
> > > > > >
> > > > > > 		p->mnt_ns->ns->ops->install(new_nsproxy, ...);
> > > > > > 		p->pid_ns_for_children->ns->ops->install(new_nsproxy, ...);
> > > > > > 		...
> > > > > >
> > > > > > 		switch_task_namespaces(new_nsproxy);
> > > > > > 	}
> > > > > >
> > > > > > Why we should abuse fs/proc ?
> > > > >
> > > > > That sounds like a much better approach.
> > > > > Your saying just take a reference to the nsproxy from the located
> > > > > process and use it instead, right?
> > > >
> > > > Yes,
> > >
> > > I'm still not sure if this can be done (at least without surgery to the
> > > namespace implementation) and I think I've been here before which is
> > > what lead to the file_open_root() approach.
> > >
> > > The difficulty is the second parameter to the install() call above, the
> > > struct ns_common. In setns() it's obtained from the procfs file inode
> > > and the file open is what's used to obtain each namespace type (in the
> > > form of a struct ns_common) from a process context different from
> > > current, current being the thread runner process.
> > >
> > > I might still be able to work out a (viable) way to obtain the
> > > appropriate ns_common struct in each case without a file open but it's
> > > hard to see how.
> >
> > Not sure I understand... Every "namespace" pointer "struct nsproxy" has
> > the "struct ns_common ns" you need? So you can do (for example)
> >
> > 	p->mnt_ns->ns->ops->install(new_nsproxy, &p->mnt_ns->ns);
> >
> > or I missed something? (userns differs, you need cred->user_ns, of course).
> >
> >
> > Perhaps I missed something, but this doesn't look like a problem...
> 
> And in case this wasn't clear we obviously need to pass nsproxy/cred to
> sub_info->init().
> 
> There is nothing magical in /proc or I am totally confused. Look at ns_get_path(),
> it (to simplify) does
> 
> 	inode->i_private = ns_ops->get(task);
> 
> and every ->get() method returns &ns->ns.

Yep, my confusion was with which nsproxy is the needed nsproxy and your
right, there isn't anything special about setting the inode nsproxy.

It looks like your comments expose the file open as a really complicated
way of getting back the nsproxy(->ns) that we (will) already have from
the located task. I should look a little further to confirm that but, in
hindsight, it seems obvious now.

And I think that's what your trying so hard to get through to me, ;)

> 
> 
> But again, again:
> 
> > The real problem is that , let me repeat, is that pidns_install() does not
> > and can't change active_pid_ns. So I think that kernel_thread_in_ns() probably
> > make more sense.
> 
> Yes...

Yes, fortunately I got that when you first mentioned it.
Your point here was easy to verify and understand.

Thanks for your persistence.
Ian


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