do not sys_reboot when not in init_pid_ns

RFC PATCH In review Daniel Lezcano 4 participants 8 messages Nov 2, 2008 – Nov 4, 2008 4 messages not in the archive

lore

Share & tools

lore

Daniel Lezcano

author

2008-11-02 00:00 UTC #0 lore raw

RFC PATCH 1/1 do not sys_reboot when not in init_pid_ns

Subject: disable sys_reboot when not in init_pid_ns
From: Daniel Lezcano <dlezcano-NmTC/[email protected]>

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/[email protected]>

---
 kernel/sys.c |    3 +++
 1 file changed, 3 insertions(+)

kernel/sys.c +3 -0

Index: 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 &&

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

A message that is not in the archive (sent off-list or not yet received): 490CEDA0.6020800-NmTC/[email protected]

Daniel Hokka Zakrisson

2008-11-02 00:16 UTC #2 lore raw

Quoted 1 line · Daniel Lezcano

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.)

Signature · 1 line

Daniel Hokka Zakrisson

Dave Hansen

2008-11-03 18:59 UTC #3 lore raw

In reply to a message not in the archive

kernel/sys.c asmlinkage long sys_reboot(int magic1, i · line 363

Quoted 11 lines · Daniel Lezcano

+++ net-next-2.6/kernel/sys.c

kernel/sys.c +3 -0

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

A message that is not in the archive (sent off-list or not yet received): 54333.2001:16d8:ff15:101:219:d2ff:fed5:8193.1225584965.squirrel@intranet

Serge E. Hallyn

2008-11-02 23:04 UTC #5 lore raw

Quoted 9 lines · Daniel Hokka Zakrisson

Quoted 1 line · Daniel Lezcano
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.)
Signature · 1 line
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

Daniel Lezcano

author

2008-11-04 20:40 UTC #6 lore raw

In reply to a message not in the archive

Quoted 5 lines · 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.)

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 ?

A message that is not in the archive (sent off-list or not yet received): 4910B34B.7070901-NmTC/[email protected]

Serge E. Hallyn

2008-11-04 21:01 UTC #8 lore raw

Quoted 7 lines · Daniel Lezcano

Quoted 5 lines · 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.)

Quoted 5 lines

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

A message that is not in the archive (sent off-list or not yet received): 20081104210134.GA6238-r/[email protected]

Daniel Lezcano

author

2008-11-04 21:39 UTC #10 lore raw

Quoted 7 lines · Serge E. Hallyn

Quoted 6 lines · Daniel Lezcano
Quoted 5 lines · 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.)

Quoted 8 lines

Quoted 5 lines
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

Daniel Lezcano

author

2008-11-04 22:14 UTC #11 lore raw

In reply to a message not in the archive

Quoted 7 lines · Serge E. Hallyn

Quoted 6 lines · Daniel Lezcano
Quoted 5 lines · 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.)

Quoted 8 lines

Quoted 5 lines
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