Re: [PATCH] Move console redirect to pid namespace

[ebiederm@xmission.com (Eric W. Biederman)]

Corey Minyard <tcminyard@gmail.com> writes:

> On 02/13/2013 01:08 PM, Eric W. Biederman wrote:
>> Bruno Prémont <bonbons@linux-vserver.org> writes:
>>
>>> CCing containers list
>>>
>>> On Fri, 08 February 2013 minyard@acm.org wrote:
>>>> From: Corey Minyard <cminyard@mvista.com>
>>>>
>>>> The console redirect - ioctl(fd, TIOCCONS) - is not in a namespace,
>>>> thus a container can do a redirect and grab all the I/O on the host
>>>> and all container consoles.
>>>>
>>>> This change puts the redirect in the pid namespace.
>>>>
>>>> Signed-off-by: Corey Minyard <cminyard@mvista.com>
>>>> ---
>>>>
>>>> I'm pretty sure this patch is not correct, but I'm not quite sure the
>>>> best way to fix this.  I'm not 100% sure that the pid namespace is the
>>>> right place, but it seemed the most reasonable of all the choices.  The
>>>> other obvious choice is the mount namespace, but it didn't seem as good
>>>> a fit.
>>> With recent changes, tying it to init user namespace might even be
>>> better.
>> With recent changes this is tied to the initial user namespace.  So the
>> simple solution to this and so many other similiar security problems is
>> to run your container in a user namespace.
>>
>> The permission check currently is capable(CAP_SYS_ADMIN) which requires
>> the caller to have the CAP_SYS_ADMIN in the initial user namespace.
>
> I'm not sure I follow.  Are these changes in k.org, or in another
> repository someplace?

In k.org. 3.7 would work. 3.8-rcX would work even better.

root in a user namespace does not have permission to call TIOCCONS.

>> Is there a desire to have TIOCCONS not just fail in a container but to
>> have TIOCCONS work in a container specific way?
>
> Well, my desire is for the host console to work properly if a
> container uses TIOCCONS :-).  It seems to me that the most consistent
> way to handle this is to have TIOCCONS in a container redirect the
> container's console.

Last I looked people were creating a regulary pty and using that in
/dev/ for their containers.  So the emperical evidence is that TIOCCONS
is not needed.  What case are you looking at that needs TIOCCONS?

If there is good cause we can make TIOCCONS work but we need a
compelling case beyond root in a container can do bad things.

>>>> The other problem is that I don't think you can call fput() from
>>>> destroy_pid_namespace().  That can be called from interrupt context,
>>>> and I don't think fput() is safe there.  I know it's not safe in 3.4
>>>> with the RT patch applied.  However, the only way I've come up with to
>>>> fix it is to add a workqueue, and that seems a bit heavy for this.
>> Actually getting destroy_pid_namespace out of interrupt context wouldn't
>> be the worst thing in the world.
>
> I would agree, but it would still require something like a workqueue.
> Is there a better mechanism?

It might be as simple as finding all of the put_pids and moving them out
of spin_lock critical sections.  I don't know that we drop pids in
actual interrupt context.

Eric