Re: [RFC PATCH v4 00/12] Second attempt at contained helper execution

[Ian Kent <raven@themaw.net>]

On Thu, 2015-03-19 at 16:38 -0500, Eric W. Biederman wrote:
> Ian Kent <raven@themaw.net> writes:
> 
> > Here is another update to the attempt at contained helper execution.
> >
> > The main change is I've tried to incorporate Oleg's suggestions
> > of directly constructing the namespaces rather than using the
> > open/setns approach and the addition of a namespace hash store.
> >
> > I'm not particularly happy with this so far as there are a bunch
> > of ref counted objects and I've almost certainly got that wrong.
> > But also there are object lifetime problems, some I'm aware of
> > and for sure others I'm not. Also there is the integrity of the
> > thread runner process. I haven't performed a double fork on thread
> > execution, it might be painful to implement, so the thread runner
> > might end up with the wrong namespace setup if an error occurs.
> >
> > Anyway, I've decided to stop spinning my wheels with this and
> > post an update in the hope that others can offer suggestions to
> > help and, of course, point out things I've missed.
> >
> > The other change has been to the nfs and KEYS patches.
> > I've introduced the ability to get a token that can be used to
> > save namespace information for later execution and I've attempted
> > to use that for persistent namespace execution, as was discussed
> > previously.
> >
> > I'm not at all sure I've done this in a sensible way but the
> > token does need to be accessible at helper execution time which
> > is why I've done it this way.
> >
> > I definitely need advice here too. 

Thanks for offering your advice once again Eric.

> 
> As far as I can tell this patchset continues to be broken for ignoring
> my earlier advice.

Ignoring is the wrong word.

I am ignorant of aspects of the bigger picture here but working on this
is helping with that quite a bit and your continued advice is very much
needed.

> 
> This patchset provides an escape from cgroup, lsm, rlimit, and
> seccomp policy.

OK.

> 
> This patchset does not appear particularly nice in how it uses
> namespaces.

Yeah, I definitely agree with that.

> 
> The only safe and sane way to do this is to have a kernel thread with
> all of the proper attributes configured waiting around ready to start
> the user mode helper.

I originally thought that wouldn't be viable due to the potential number
of threads that would be needed.

I still think that a thread per mountpoint, as we originally discussed,
won't work but looking at nfsd, nfs and the keys code for the recent
patch series makes me think that there might not be so many threads
needed and that depends on choosing a better place to start the threads.

> 
> The problem you are trying to solve is so hard that we totally failed to
> solve it outside of the container case.  Which is why we have kthreadd.
> I will be very surprised if you can figure out how to cleanly solve the
> problem the way you are attacking it.

So the previous approach of using file_open_root()/setns_inode() is
equally broken in the same respects as you mention above? You didn't
mention on that before?

I get that the problem is hard to solve but I'd rather not give up on
it.

Maybe the token I use could relate to a previously created kernel thread
instead of namespace information.

LOL, then you can describe what I've done wrong creating the kernel
threads as well, ;)

> 
> Eric
> 
>