[patch 1/1][RFC] do not sys_reboot when not in init_pid_ns

[patch 1/1][RFC] do not sys_reboot when not in init_pid_ns

[Daniel Lezcano]

[-- Attachment #1: Type: text/plain, Size: 1 bytes --]



[-- Attachment #2: do-not-shutdown-when-not-init-pid-ns.patch --]
[-- Type: text/x-diff, Size: 1384 bytes --]

Subject: disable sys_reboot when not in init_pid_ns
From: Daniel Lezcano <dlezcano-NmTC/0ZBporQT0dZR+AlfA@public.gmane.org>

This simple patch avoid to shutdown the host within a container. Without this
patch a call to the 'halt' inside a container will switch to the right runlevel
but finishing with 'shutdown -f' in the last init script with the effect of
shutting down the real host.

This patch has been tested with the lxc tools and a debian minimal container.
The 'init' process running inside the container does correctly call the 
different shutdown services and the container exits gracefully.

I didn't try with the 'init' from the upstart package. It uses an abstract
unix socket, that means this patch should work if the container is network 
isolated too.

Signed-off-by: Daniel Lezcano <dlezcano-NmTC/0ZBporQT0dZR+AlfA@public.gmane.org>
---
 kernel/sys.c |    3 +++
 1 file changed, 3 insertions(+)

Index: net-next-2.6/kernel/sys.c
===================================================================
--- net-next-2.6.orig/kernel/sys.c
+++ net-next-2.6/kernel/sys.c
@@ -355,6 +355,9 @@ asmlinkage long sys_reboot(int magic1, i
 	if (!capable(CAP_SYS_BOOT))
 		return -EPERM;
 
+	if (current->nsproxy->pid_ns != &init_pid_ns)
+		return 0;
+
 	/* For safety, we require "magic" arguments. */
 	if (magic1 != LINUX_REBOOT_MAGIC1 ||
 	    (magic2 != LINUX_REBOOT_MAGIC2 &&

[-- Attachment #3: Type: text/plain, Size: 206 bytes --]

_______________________________________________
Containers mailing list
Containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
https://lists.linux-foundation.org/mailman/listinfo/containers

Re: [patch 1/1][RFC] do not sys_reboot when not in init_pid_ns

[Serge E. Hallyn]

Quoting Daniel Hokka Zakrisson (daniel-nym3zxDgnZcAvxtiuMwx3w@public.gmane.org):
> Daniel Lezcano wrote:
> >
> 
> Wouldn't it be better to simply remove CAP_SYS_BOOT from containers
> until sys_reboot emits some signal to userspace to restart/halt the
> container? (This is what we do in Linux-VServer.)
> 
> -- 
> Daniel Hokka Zakrisson

Yeah that makes more sense to me.

Note that otherwise your patch still lets the container mess with
sys_kexec_load(), for instance.

-serge

Re: [patch 1/1][RFC] do not sys_reboot when not in init_pid_ns

[Daniel Lezcano]

Daniel Hokka Zakrisson wrote:
> Daniel Lezcano wrote:
> 
> Wouldn't it be better to simply remove CAP_SYS_BOOT from containers
> until sys_reboot emits some signal to userspace to restart/halt the
> container? (This is what we do in Linux-VServer.)

Ok, I will try, thanks.

BTW, isn't possible that a process gave CAP_SYS_BOOT capability again to 
  himself and being able to shutdown the host ? I guess I should remove 
CAP_SETPCAP too, no ?

Re: [patch 1/1][RFC] do not sys_reboot when not in init_pid_ns

[Serge E. Hallyn]

Quoting Daniel Lezcano (dlezcano-NmTC/0ZBporQT0dZR+AlfA@public.gmane.org):
> Daniel Hokka Zakrisson wrote:
> > Daniel Lezcano wrote:
> > 
> > Wouldn't it be better to simply remove CAP_SYS_BOOT from containers
> > until sys_reboot emits some signal to userspace to restart/halt the
> > container? (This is what we do in Linux-VServer.)
> 
> Ok, I will try, thanks.
> 
> BTW, isn't possible that a process gave CAP_SYS_BOOT capability again to 
>   himself and being able to shutdown the host ? I guess I should remove 
> CAP_SETPCAP too, no ?

No, remove it from your bounding set.  You can never add bits back to
that set.  prctl(PR_CAPBSET_DROP, CAP_SYS_BOOT);

-serge

Re: [patch 1/1][RFC] do not sys_reboot when not in init_pid_ns

[Daniel Lezcano]

Serge E. Hallyn wrote:
> Quoting Daniel Lezcano (dlezcano-NmTC/0ZBporQT0dZR+AlfA@public.gmane.org):
>> Daniel Hokka Zakrisson wrote:
>>> Daniel Lezcano wrote:
>>>
>>> Wouldn't it be better to simply remove CAP_SYS_BOOT from containers
>>> until sys_reboot emits some signal to userspace to restart/halt the
>>> container? (This is what we do in Linux-VServer.)
>> Ok, I will try, thanks.
>>
>> BTW, isn't possible that a process gave CAP_SYS_BOOT capability again to 
>>   himself and being able to shutdown the host ? I guess I should remove 
>> CAP_SETPCAP too, no ?
> 
> No, remove it from your bounding set.  You can never add bits back to
> that set.  prctl(PR_CAPBSET_DROP, CAP_SYS_BOOT);

Excellent, I will try that.

Thanks guys.

   -- Daniel

Re: [patch 1/1][RFC] do not sys_reboot when not in init_pid_ns

[Daniel Lezcano]

Serge E. Hallyn wrote:
> Quoting Daniel Lezcano (dlezcano-NmTC/0ZBporQT0dZR+AlfA@public.gmane.org):
>> Daniel Hokka Zakrisson wrote:
>>> Daniel Lezcano wrote:
>>>
>>> Wouldn't it be better to simply remove CAP_SYS_BOOT from containers
>>> until sys_reboot emits some signal to userspace to restart/halt the
>>> container? (This is what we do in Linux-VServer.)
>> Ok, I will try, thanks.
>>
>> BTW, isn't possible that a process gave CAP_SYS_BOOT capability again to 
>>   himself and being able to shutdown the host ? I guess I should remove 
>> CAP_SETPCAP too, no ?
> 
> No, remove it from your bounding set.  You can never add bits back to
> that set.  prctl(PR_CAPBSET_DROP, CAP_SYS_BOOT);

Tested with lxc and debian minimal, I can halt/shutdown the container 
from inside. Cool !

Thanks.
  -- Daniel

Re: [patch 1/1][RFC] do not sys_reboot when not in init_pid_ns

[Daniel Hokka Zakrisson]

Daniel Lezcano wrote:
>

Wouldn't it be better to simply remove CAP_SYS_BOOT from containers
until sys_reboot emits some signal to userspace to restart/halt the
container? (This is what we do in Linux-VServer.)

-- 
Daniel Hokka Zakrisson

Re: [patch 1/1][RFC] do not sys_reboot when not in init_pid_ns

[Dave Hansen]

On Sun, 2008-11-02 at 01:00 +0100, Daniel Lezcano wrote:
> +++ net-next-2.6/kernel/sys.c
> @@ -355,6 +355,9 @@ asmlinkage long sys_reboot(int magic1, i
>         if (!capable(CAP_SYS_BOOT))
>                 return -EPERM;
> 
> +       if (current->nsproxy->pid_ns != &init_pid_ns)
> +               return 0;
> +
>         /* For safety, we require "magic" arguments. */
>         if (magic1 != LINUX_REBOOT_MAGIC1 ||
>             (magic2 != LINUX_REBOOT_MAGIC2 &&

One problem I have with this is that it specifically defines being "in a
container" as being in a pid_ns other than the init_pid_ns.  If we're
going to go down this road, it should be at *least*:

int in_a_container(void)
{
	return current->nsproxy->pid_ns != &init_pid_ns;
}

But, this also sucks because we don't want to be introducing new code
paths all over the kernel for the "container" case.  What we'll end up
with little craplets like this spread all over:

	if (in_a_container()) {
		/* don't ever test this code path */
	}

:)

So I think we should avoid what you're trying to do here like the
plague.

-- Dave