2008-03-23 13:47:16

by Oleg Nesterov

[permalink] [raw]
Subject: [PATCH] ptrace: it is fun to strace /sbin/init

(re-send with updated changelog)

Ptracing of /sbin/init is not allowed. Of course, this is dangerous, but may
be useful. Introduce the kernel boot parameter to allow this, so that we can't
surprise some special/secured systems.

Afaics, with the recent changes there is no kernel problems with ptracing init,
it can't lose SIGNAL_UNKILLABLE flag and be killed by accident. However, admin
should know what it does, "gdb /sbin/init 1" stops init, it can't reap zombies
or take care of /etc/inittab until continued. It is even possible to crash init
(and thus the whole system) if you wish, ptracer has full control.

The "if (pid == 1)" check in ptrace_get_task_struct() is killed, ptrace_attach
does the same check.

Signed-off-by: Oleg Nesterov <[email protected]>

--- 25/kernel/ptrace.c~5_INIT_PTRACE 2008-03-16 17:22:04.000000000 +0300
+++ 25/kernel/ptrace.c 2008-03-16 18:33:02.000000000 +0300
@@ -160,6 +160,15 @@ int ptrace_may_attach(struct task_struct
return !err;
}

+static int allow_ptrace_init;
+
+static int __init __allow_ptrace_init(char *str)
+{
+ allow_ptrace_init = 1;
+ return 1;
+}
+__setup("init_ptrace", __allow_ptrace_init);
+
int ptrace_attach(struct task_struct *task)
{
int retval;
@@ -168,7 +177,7 @@ int ptrace_attach(struct task_struct *ta
audit_ptrace(task);

retval = -EPERM;
- if (task->pid <= 1)
+ if (unlikely(is_global_init(task)) && likely(!allow_ptrace_init))
goto out;
if (same_thread_group(task, current))
goto out;
@@ -518,12 +527,6 @@ struct task_struct *ptrace_get_task_stru
{
struct task_struct *child;

- /*
- * Tracing init is not allowed.
- */
- if (pid == 1)
- return ERR_PTR(-EPERM);
-
read_lock(&tasklist_lock);
child = find_task_by_vpid(pid);
if (child)
--- 25/Documentation/kernel-parameters.txt~5_INIT_PTRACE 2008-02-15 16:58:12.000000000 +0300
+++ 25/Documentation/kernel-parameters.txt 2008-03-16 18:30:28.000000000 +0300
@@ -803,6 +803,8 @@ and is between 256 and 4096 characters.
Run specified binary instead of /sbin/init as init
process.

+ init_ptrace [KNL] Allows to ptrace init.
+
initcall_debug [KNL] Trace initcalls as they are executed. Useful
for working out where the kernel is dying during
startup.


2008-03-24 16:01:17

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH] ptrace: it is fun to strace /sbin/init

Hi!

> Ptracing of /sbin/init is not allowed. Of course, this is dangerous, but may
> be useful. Introduce the kernel boot parameter to allow this, so that we can't
> surprise some special/secured systems.
>
> Afaics, with the recent changes there is no kernel problems with ptracing init,
> it can't lose SIGNAL_UNKILLABLE flag and be killed by accident. However, admin
> should know what it does, "gdb /sbin/init 1" stops init, it can't
> reap zombies

It can't be killed, but you can stop it and not ever restart it. From
that point on, zombies will accumulate until we OOM, right?

> @@ -803,6 +803,8 @@ and is between 256 and 4096 characters.
> Run specified binary instead of /sbin/init as init
> process.
>
> + init_ptrace [KNL] Allows to ptrace init.
> +

No words about it being dangerous, but I believe it is.

If it is not, lets do the patch, but not optional.

If it is dangerous, option does not make it better.

--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2008-03-24 16:36:55

by Oleg Nesterov

[permalink] [raw]
Subject: Re: [PATCH] ptrace: it is fun to strace /sbin/init

On 03/24, Pavel Machek wrote:
>
> > Ptracing of /sbin/init is not allowed. Of course, this is dangerous, but may
> > be useful. Introduce the kernel boot parameter to allow this, so that we can't
> > surprise some special/secured systems.
> >
> > Afaics, with the recent changes there is no kernel problems with ptracing init,
> > it can't lose SIGNAL_UNKILLABLE flag and be killed by accident. However, admin
> > should know what it does, "gdb /sbin/init 1" stops init, it can't
> > reap zombies
>
> It can't be killed, but you can stop it and not ever restart it. From
> that point on, zombies will accumulate until we OOM, right?

Right (only re-parented ones and init's childs but still right),

> > @@ -803,6 +803,8 @@ and is between 256 and 4096 characters.
> > Run specified binary instead of /sbin/init as init
> > process.
> >
> > + init_ptrace [KNL] Allows to ptrace init.
> > +
>
> No words about it being dangerous, but I believe it is.

it is, admin should know what he does,

> If it is not, lets do the patch, but not optional.

This will change the default historical behaviour, I can't predict
does this matter for (say) SELinux or not. Otherwise I'd personally
prefer to always allow to ptrace init.

> If it is dangerous, option does not make it better.

ptrace() is always dangerous. ptracer can crash oracle and lose data.

/sbin/init is important, but there are other important (and sometimes
much more important) services. Why it is so special so that we can't
debug/strace it?

(how many times did you try to figure out why init does _not_ work
as expected ?)

Oleg.

2008-03-24 22:30:34

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] ptrace: it is fun to strace /sbin/init

On Sun, 23 Mar 2008 16:51:10 +0300
Oleg Nesterov <[email protected]> wrote:

> Ptracing of /sbin/init is not allowed. Of course, this is dangerous, but may
> be useful. Introduce the kernel boot parameter to allow this, so that we can't
> surprise some special/secured systems.

I dunno, is this really needed? If root wants to screw up his kernel then
he is free to do so.

And if we *really* want an extra foot-protector for this, it could be a
runtime /proc/sys/kernel/root-can-shoot-inits-foot rather than a boot-time
option?

2008-03-24 22:38:55

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH] ptrace: it is fun to strace /sbin/init


> > > @@ -803,6 +803,8 @@ and is between 256 and 4096 characters.
> > > Run specified binary instead of /sbin/init as init
> > > process.
> > >
> > > + init_ptrace [KNL] Allows to ptrace init.
> > > +
> >
> > No words about it being dangerous, but I believe it is.
>
> it is, admin should know what he does,
>
> > If it is not, lets do the patch, but not optional.
>
> This will change the default historical behaviour, I can't predict
> does this matter for (say) SELinux or not. Otherwise I'd personally
> prefer to always allow to ptrace init.
>
> > If it is dangerous, option does not make it better.
>
> ptrace() is always dangerous. ptracer can crash oracle and lose data.
>
> /sbin/init is important, but there are other important (and sometimes
> much more important) services. Why it is so special so that we can't
> debug/strace it?

Maybe. Let's kill /sbin/init protection in 2.6.26. But making it
optional is wrong.
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
pomozte zachranit klanovicky les: http://www.ujezdskystrom.info/

2008-03-24 23:02:57

by Oleg Nesterov

[permalink] [raw]
Subject: Re: [PATCH] ptrace: it is fun to strace /sbin/init

On 03/24, Andrew Morton wrote:
>
> On Sun, 23 Mar 2008 16:51:10 +0300
> Oleg Nesterov <[email protected]> wrote:
>
> > Ptracing of /sbin/init is not allowed. Of course, this is dangerous, but may
> > be useful. Introduce the kernel boot parameter to allow this, so that we can't
> > surprise some special/secured systems.
>
> I dunno, is this really needed?

Well, this is the question. I think it would be very nice to have the ability
to debug/strace init. Especially if you try to make your own distribution /
your own init.

Sometimes I see init at the top of the top's output, with this patch I have a
chance to see what's going on on my system.

> If root wants to screw up his kernel then
> he is free to do so.

I think you and Pavel are very wrong here.

First, this has nothing to do with kernel, imho. Afaics, now there are no kernel
problems with ptracing init.

And. When I was admin in my previous life, I certainly was not able to patch
the kernel and then strace/debug init.

> And if we *really* want an extra foot-protector for this, it could be a
> runtime /proc/sys/kernel/root-can-shoot-inits-foot rather than a boot-time
> option?

Even better! I agree, will re-send. I choose the boot-time paramater because
it looks like the "most safe" option.

Oleg.

2008-03-24 23:05:52

by Oleg Nesterov

[permalink] [raw]
Subject: Re: [PATCH] ptrace: it is fun to strace /sbin/init

On 03/24, Pavel Machek wrote:
>
> > /sbin/init is important, but there are other important (and sometimes
> > much more important) services. Why it is so special so that we can't
> > debug/strace it?
>
> Maybe. Let's kill /sbin/init protection in 2.6.26. But making it
> optional is wrong.

You are right, the boot parameter is silly. How about sysctl?

Stephen, do you see any security problems if we make /sbin/init
ptraceable by default?

Oleg.

2008-03-24 23:09:00

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH] ptrace: it is fun to strace /sbin/init

On Tue 2008-03-25 02:04:58, Oleg Nesterov wrote:
> On 03/24, Pavel Machek wrote:
> >
> > > /sbin/init is important, but there are other important (and sometimes
> > > much more important) services. Why it is so special so that we can't
> > > debug/strace it?
> >
> > Maybe. Let's kill /sbin/init protection in 2.6.26. But making it
> > optional is wrong.
>
> You are right, the boot parameter is silly. How about sysctl?

I'd prefer it to be hardcoded, really.

"You can kill /sbin/init" sounds right.

"You can kill /sbin/init on 2.6.26+" sounds... still quite ok.

"You can kill /sbin/init on 2.6.26+ if you have /proc/sys/foo/bar ==
1" sounds... quite wrong.

> Stephen, do you see any security problems if we make /sbin/init
> ptraceable by default?

Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
pomozte zachranit klanovicky les: http://www.ujezdskystrom.info/

2008-03-24 23:09:52

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] ptrace: it is fun to strace /sbin/init

On Tue, 25 Mar 2008 01:56:11 +0300
Oleg Nesterov <[email protected]> wrote:

> On 03/24, Andrew Morton wrote:
> >
> > On Sun, 23 Mar 2008 16:51:10 +0300
> > Oleg Nesterov <[email protected]> wrote:
> >
> > > Ptracing of /sbin/init is not allowed. Of course, this is dangerous, but may
> > > be useful. Introduce the kernel boot parameter to allow this, so that we can't
> > > surprise some special/secured systems.
> >
> > I dunno, is this really needed?
>
> Well, this is the question. I think it would be very nice to have the ability
> to debug/strace init. Especially if you try to make your own distribution /
> your own init.
>
> Sometimes I see init at the top of the top's output, with this patch I have a
> chance to see what's going on on my system.

I agree that init should be ptraceable. I'm questioning the value of a
knob which enables that ability.

Why not just unconditionally enable root's abiltiy to ptrace init?

2008-03-24 23:20:13

by Oleg Nesterov

[permalink] [raw]
Subject: Re: [PATCH] ptrace: it is fun to strace /sbin/init

On 03/25, Pavel Machek wrote:
>
> On Tue 2008-03-25 02:04:58, Oleg Nesterov wrote:
> > On 03/24, Pavel Machek wrote:
> > >
> > > > /sbin/init is important, but there are other important (and sometimes
> > > > much more important) services. Why it is so special so that we can't
> > > > debug/strace it?
> > >
> > > Maybe. Let's kill /sbin/init protection in 2.6.26. But making it
> > > optional is wrong.
> >
> > You are right, the boot parameter is silly. How about sysctl?
>
> I'd prefer it to be hardcoded, really.

Yes! me too.

> "You can kill /sbin/init" sounds right.
>
> "You can kill /sbin/init on 2.6.26+" sounds... still quite ok.
>
> "You can kill /sbin/init on 2.6.26+ if you have /proc/sys/foo/bar ==
> 1" sounds... quite wrong.

Please look at another discussion, http://marc.info/?t=120568298600007

When I did this simple patch, I was very sure it is "obviously good".
But as Stephen pointed out, we have the systems that relies on the
current behaviour, even if this behaviour is not "optimal".

Oleg.

2008-03-24 23:28:14

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH] ptrace: it is fun to strace /sbin/init

On Tue, 25 Mar 2008 02:18:59 +0300
Oleg Nesterov <[email protected]> wrote:

> Please look at another discussion, http://marc.info/?t=120568298600007

-EAMTOOLAZY. Whatever is at the above link should be in the changelog so we
can understand the reasons for the patch!

Please.

2008-03-24 23:30:26

by Oleg Nesterov

[permalink] [raw]
Subject: Re: [PATCH] ptrace: it is fun to strace /sbin/init

On 03/24, Andrew Morton wrote:
>
> On Tue, 25 Mar 2008 01:56:11 +0300
> Oleg Nesterov <[email protected]> wrote:
>
> > On 03/24, Andrew Morton wrote:
> > >
> > > On Sun, 23 Mar 2008 16:51:10 +0300
> > > Oleg Nesterov <[email protected]> wrote:
> > >
> > > > Ptracing of /sbin/init is not allowed. Of course, this is dangerous, but may
> > > > be useful. Introduce the kernel boot parameter to allow this, so that we can't
> > > > surprise some special/secured systems.
> > >
> > > I dunno, is this really needed?
> >
> > Well, this is the question. I think it would be very nice to have the ability
> > to debug/strace init. Especially if you try to make your own distribution /
> > your own init.
> >
> > Sometimes I see init at the top of the top's output, with this patch I have a
> > chance to see what's going on on my system.
>
> I agree that init should be ptraceable. I'm questioning the value of a
> knob which enables that ability.
>
> Why not just unconditionally enable root's abiltiy to ptrace init?

Ah, sorry, I misunderstood.

As for me, I think it would be right to allow to ptrace init unconditionally.
But I'd like to know what security people think, I am very much afraid there
is something I don't know/understand (like it happened with "don't panic if
/sbin/init exits or killed").

Oleg.

2008-03-24 23:39:33

by Oleg Nesterov

[permalink] [raw]
Subject: Re: [PATCH] ptrace: it is fun to strace /sbin/init

On 03/24, Andrew Morton wrote:
>
> On Tue, 25 Mar 2008 02:18:59 +0300
> Oleg Nesterov <[email protected]> wrote:
>
> > Please look at another discussion, http://marc.info/?t=120568298600007
>
> -EAMTOOLAZY. Whatever is at the above link should be in the changelog so we
> can understand the reasons for the patch!

No, no, this has nothing to do with this patch.

This was another patch, "don't panic if /sbin/init exits or killed", which
changed the current behaviour of /sbin/init unconditionally. And while I
thought this change is "obviously good" it wasn't, because we have users
which relies on the current behaviour.

Oleg.

2008-03-25 10:01:12

by Andi Kleen

[permalink] [raw]
Subject: Re: [PATCH] ptrace: it is fun to strace /sbin/init

Andrew Morton <[email protected]> writes:
>
> Why not just unconditionally enable root's abiltiy to ptrace init?

It would be fine to allow this unconditionally if there was some mechanism
to make sure someone else takes over reaping childs while init is ptraced.

I like the general idea -- i used to patch kernels to allow this too,
but it is dangerous.

-Andi

2008-03-25 12:15:26

by Stephen Smalley

[permalink] [raw]
Subject: Re: [PATCH] ptrace: it is fun to strace /sbin/init


On Tue, 2008-03-25 at 02:04 +0300, Oleg Nesterov wrote:
> On 03/24, Pavel Machek wrote:
> >
> > > /sbin/init is important, but there are other important (and sometimes
> > > much more important) services. Why it is so special so that we can't
> > > debug/strace it?
> >
> > Maybe. Let's kill /sbin/init protection in 2.6.26. But making it
> > optional is wrong.
>
> You are right, the boot parameter is silly. How about sysctl?
>
> Stephen, do you see any security problems if we make /sbin/init
> ptraceable by default?

Not an issue for SELinux (we apply an orthogonal check based on security
context, so we can already block ptrace of init independent of whether
root/CAP_SYS_PTRACE can do it). I'm not sure though as to whether
people using capabilities have ever relied on this special protection of
init (e.g. custom init spawns children with lesser capabilities and
relies on the fact that they cannot ptrace init to effectively re-gain
those capabilities, even if they possess CAP_SYS_PTRACE).

--
Stephen Smalley
National Security Agency

2008-03-25 13:40:28

by Serge E. Hallyn

[permalink] [raw]
Subject: Re: [PATCH] ptrace: it is fun to strace /sbin/init

Quoting Stephen Smalley ([email protected]):
>
> On Tue, 2008-03-25 at 02:04 +0300, Oleg Nesterov wrote:
> > On 03/24, Pavel Machek wrote:
> > >
> > > > /sbin/init is important, but there are other important (and sometimes
> > > > much more important) services. Why it is so special so that we can't
> > > > debug/strace it?
> > >
> > > Maybe. Let's kill /sbin/init protection in 2.6.26. But making it
> > > optional is wrong.
> >
> > You are right, the boot parameter is silly. How about sysctl?
> >
> > Stephen, do you see any security problems if we make /sbin/init
> > ptraceable by default?
>
> Not an issue for SELinux (we apply an orthogonal check based on security
> context, so we can already block ptrace of init independent of whether
> root/CAP_SYS_PTRACE can do it). I'm not sure though as to whether
> people using capabilities have ever relied on this special protection of
> init (e.g. custom init spawns children with lesser capabilities and
> relies on the fact that they cannot ptrace init to effectively re-gain
> those capabilities, even if they possess CAP_SYS_PTRACE).

Still thinking it through, but it seems like special casing init isn't
useful. There are likely to be other tasks with all capabilities
set which the malicious task could just as well ptrace to do his
mischief, right?

thanks,
-serge

2008-03-25 14:16:24

by Krzysztof Halasa

[permalink] [raw]
Subject: Re: [PATCH] ptrace: it is fun to strace /sbin/init

Andi Kleen <[email protected]> writes:

> It would be fine to allow this unconditionally if there was some mechanism
> to make sure someone else takes over reaping childs while init is
> ptraced.

Well, I think it isn't even necessary, unless you ptrace init for a
long time. I would simply go with the simple 1-line patch.

> I like the general idea -- i used to patch kernels to allow this too,
> but it is dangerous.

Though root-only. Root doesn't want to be limited and knows better
anyway. I think ptracing init is useful even when no one takes care
of the dead (especially if you want to trace waits).
--
Krzysztof Halasa

2008-03-25 14:20:14

by Andi Kleen

[permalink] [raw]
Subject: Re: [PATCH] ptrace: it is fun to strace /sbin/init

> > I like the general idea -- i used to patch kernels to allow this too,
> > but it is dangerous.
>
> Though root-only. Root doesn't want to be limited and knows better
> anyway.

The problem is that you can deadlock if you are not very careful.

-Andi

2008-03-25 14:30:38

by Krzysztof Halasa

[permalink] [raw]
Subject: Re: [PATCH] ptrace: it is fun to strace /sbin/init

Andi Kleen <[email protected]> writes:

> The problem is that you can deadlock if you are not very careful.

I can damage the system in trillion ways, one more is not a problem
:-)

Hopefully someone will write the details down to enable root to be
very careful.
--
Krzysztof Halasa

2008-03-25 14:32:44

by Andi Kleen

[permalink] [raw]
Subject: Re: [PATCH] ptrace: it is fun to strace /sbin/init

On Tue, Mar 25, 2008 at 03:30:29PM +0100, Krzysztof Halasa wrote:
> Andi Kleen <[email protected]> writes:
>
> > The problem is that you can deadlock if you are not very careful.
>
> I can damage the system in trillion ways, one more is not a problem
> :-)

We don't actually have that many ways for hard deadlocks even
as root (short of doing really nasty things on /dev/kmem)

> Hopefully someone will write the details down to enable root to be
> very careful.

Sure, but if the deadlocks can be avoided on the kernel level (just
making sure the children are already reaped) then it would be even
better.

-Andi

2008-03-25 14:43:53

by Stephen Smalley

[permalink] [raw]
Subject: Re: [PATCH] ptrace: it is fun to strace /sbin/init


On Tue, 2008-03-25 at 08:40 -0500, Serge E. Hallyn wrote:
> Quoting Stephen Smalley ([email protected]):
> >
> > On Tue, 2008-03-25 at 02:04 +0300, Oleg Nesterov wrote:
> > > On 03/24, Pavel Machek wrote:
> > > >
> > > > > /sbin/init is important, but there are other important (and sometimes
> > > > > much more important) services. Why it is so special so that we can't
> > > > > debug/strace it?
> > > >
> > > > Maybe. Let's kill /sbin/init protection in 2.6.26. But making it
> > > > optional is wrong.
> > >
> > > You are right, the boot parameter is silly. How about sysctl?
> > >
> > > Stephen, do you see any security problems if we make /sbin/init
> > > ptraceable by default?
> >
> > Not an issue for SELinux (we apply an orthogonal check based on security
> > context, so we can already block ptrace of init independent of whether
> > root/CAP_SYS_PTRACE can do it). I'm not sure though as to whether
> > people using capabilities have ever relied on this special protection of
> > init (e.g. custom init spawns children with lesser capabilities and
> > relies on the fact that they cannot ptrace init to effectively re-gain
> > those capabilities, even if they possess CAP_SYS_PTRACE).
>
> Still thinking it through, but it seems like special casing init isn't
> useful. There are likely to be other tasks with all capabilities
> set which the malicious task could just as well ptrace to do his
> mischief, right?

Depends on the bounding set. Didn't it used to be the case that only
init had CAP_SETPCAP (until the meaning of it was changed by the
filesystem capability support)?

Might want to double check with e.g. the vservers folks that they
weren't relying in any way on special handling of init.

--
Stephen Smalley
National Security Agency

2008-03-25 14:48:17

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH] ptrace: it is fun to strace /sbin/init

Hi!

> > > I like the general idea -- i used to patch kernels to allow this too,
> > > but it is dangerous.
> >
> > Though root-only. Root doesn't want to be limited and knows better
> > anyway.
>
> The problem is that you can deadlock if you are not very careful.

How is that different from ptracing any other process with _lot_ of
children -- like X? You'll get same zombies in both cases, leading to
same issues, no?

--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2008-03-25 14:53:11

by Andi Kleen

[permalink] [raw]
Subject: Re: [PATCH] ptrace: it is fun to strace /sbin/init

On Tue, Mar 25, 2008 at 03:47:54PM +0100, Pavel Machek wrote:
> Hi!
>
> > > > I like the general idea -- i used to patch kernels to allow this too,
> > > > but it is dangerous.
> > >
> > > Though root-only. Root doesn't want to be limited and knows better
> > > anyway.
> >
> > The problem is that you can deadlock if you are not very careful.
>
> How is that different from ptracing any other process with _lot_ of
> children -- like X? You'll get same zombies in both cases, leading to
> same issues, no?

There is no unresolvable deadlock no.

-Andi

2008-03-25 14:59:15

by Krzysztof Halasa

[permalink] [raw]
Subject: Re: [PATCH] ptrace: it is fun to strace /sbin/init

Andi Kleen <[email protected]> writes:

> Sure, but if the deadlocks can be avoided on the kernel level (just
> making sure the children are already reaped) then it would be even
> better.

Well, if you mean the kernel would do that unconditionally (thus it
won't be init's job anymore, ptrace or not) then I can only agree.
--
Krzysztof Halasa

2008-03-25 17:38:24

by Oleg Nesterov

[permalink] [raw]
Subject: Re: [PATCH] ptrace: it is fun to strace /sbin/init

On 03/25, Andi Kleen wrote:
>
> Andrew Morton <[email protected]> writes:
> >
> > Why not just unconditionally enable root's abiltiy to ptrace init?
>
> It would be fine to allow this unconditionally if there was some mechanism
> to make sure someone else takes over reaping childs while init is ptraced.

I think we can't do this. From my /etc/inittab:

c1:1235:respawn:/sbin/agetty 38400 tty1 linux

If /sbin/agetty exits, we shouldn't reap it. /sbin/init should notice
the death of login, and respawn the new child.

Thanks to all for replies! I'll re-send this patch as one-liner, without
any boot params or sysctls, and with long CC list.

I hope someone (not me) will make a final authoritative decision ;)

Oleg.

2008-03-25 18:06:23

by Serge E. Hallyn

[permalink] [raw]
Subject: Re: [PATCH] ptrace: it is fun to strace /sbin/init

Quoting Stephen Smalley ([email protected]):
>
> On Tue, 2008-03-25 at 08:40 -0500, Serge E. Hallyn wrote:
> > Quoting Stephen Smalley ([email protected]):
> > >
> > > On Tue, 2008-03-25 at 02:04 +0300, Oleg Nesterov wrote:
> > > > On 03/24, Pavel Machek wrote:
> > > > >
> > > > > > /sbin/init is important, but there are other important (and sometimes
> > > > > > much more important) services. Why it is so special so that we can't
> > > > > > debug/strace it?
> > > > >
> > > > > Maybe. Let's kill /sbin/init protection in 2.6.26. But making it
> > > > > optional is wrong.
> > > >
> > > > You are right, the boot parameter is silly. How about sysctl?
> > > >
> > > > Stephen, do you see any security problems if we make /sbin/init
> > > > ptraceable by default?
> > >
> > > Not an issue for SELinux (we apply an orthogonal check based on security
> > > context, so we can already block ptrace of init independent of whether
> > > root/CAP_SYS_PTRACE can do it). I'm not sure though as to whether
> > > people using capabilities have ever relied on this special protection of
> > > init (e.g. custom init spawns children with lesser capabilities and
> > > relies on the fact that they cannot ptrace init to effectively re-gain
> > > those capabilities, even if they possess CAP_SYS_PTRACE).
> >
> > Still thinking it through, but it seems like special casing init isn't
> > useful. There are likely to be other tasks with all capabilities
> > set which the malicious task could just as well ptrace to do his
> > mischief, right?
>
> Depends on the bounding set. Didn't it used to be the case that only
> init had CAP_SETPCAP (until the meaning of it was changed by the
> filesystem capability support)?

Not quite. CAP_SETPCAP was taken out of everyone's bounding set. But
kernel/sysctl.c allowed only init to add capabilities to the bounding
set. (Whereas CAP_SYS_MODULE was sufficient to remove them).

> Might want to double check with e.g. the vservers folks that they
> weren't relying in any way on special handling of init.

Herbert, Pavel, do you have objections to allowing ptrace of init?
(I believe Eric has already Acked the idea iirc?)

thanks,
-serge

2008-03-25 19:32:19

by Herbert Poetzl

[permalink] [raw]
Subject: Re: [PATCH] ptrace: it is fun to strace /sbin/init

On Tue, Mar 25, 2008 at 01:06:11PM -0500, Serge E. Hallyn wrote:
> Quoting Stephen Smalley ([email protected]):
> >
> > On Tue, 2008-03-25 at 08:40 -0500, Serge E. Hallyn wrote:
> > > Quoting Stephen Smalley ([email protected]):
> > > >
> > > > On Tue, 2008-03-25 at 02:04 +0300, Oleg Nesterov wrote:
> > > > > On 03/24, Pavel Machek wrote:
> > > > > >
> > > > > > > /sbin/init is important, but there are other important (and sometimes
> > > > > > > much more important) services. Why it is so special so that we can't
> > > > > > > debug/strace it?
> > > > > >
> > > > > > Maybe. Let's kill /sbin/init protection in 2.6.26. But making it
> > > > > > optional is wrong.
> > > > >
> > > > > You are right, the boot parameter is silly. How about sysctl?
> > > > >
> > > > > Stephen, do you see any security problems if we make /sbin/init
> > > > > ptraceable by default?
> > > >
> > > > Not an issue for SELinux (we apply an orthogonal check based on security
> > > > context, so we can already block ptrace of init independent of whether
> > > > root/CAP_SYS_PTRACE can do it). I'm not sure though as to whether
> > > > people using capabilities have ever relied on this special protection of
> > > > init (e.g. custom init spawns children with lesser capabilities and
> > > > relies on the fact that they cannot ptrace init to effectively re-gain
> > > > those capabilities, even if they possess CAP_SYS_PTRACE).
> > >
> > > Still thinking it through, but it seems like special casing init isn't
> > > useful. There are likely to be other tasks with all capabilities
> > > set which the malicious task could just as well ptrace to do his
> > > mischief, right?
> >
> > Depends on the bounding set. Didn't it used to be the case that only
> > init had CAP_SETPCAP (until the meaning of it was changed by the
> > filesystem capability support)?
>
> Not quite. CAP_SETPCAP was taken out of everyone's bounding set. But
> kernel/sysctl.c allowed only init to add capabilities to the bounding
> set. (Whereas CAP_SYS_MODULE was sufficient to remove them).
>
> > Might want to double check with e.g. the vservers folks that they
> > weren't relying in any way on special handling of init.
>
> Herbert, Pavel, do you have objections to allowing ptrace of init?
> (I believe Eric has already Acked the idea iirc?)

inside a guest, by default no (i.e. there simply is
no capability for that), on the host the behaviour is
unmodified .. note that there are guests without init
where the blend through init is protected in a special
way

HTH,
Herbert

> thanks,
> -serge

2008-03-25 21:55:31

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH] ptrace: it is fun to strace /sbin/init

Hi!

> > Might want to double check with e.g. the vservers folks that they
> > weren't relying in any way on special handling of init.
>
> Herbert, Pavel, do you have objections to allowing ptrace of init?
> (I believe Eric has already Acked the idea iirc?)

No problem from me...

(..but do not introduce command line option or sysctl. It is not worth it).

Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
pomozte zachranit klanovicky les: http://www.ujezdskystrom.info/

2008-03-25 22:01:57

by Roland McGrath

[permalink] [raw]
Subject: Re: [PATCH] ptrace: it is fun to strace /sbin/init

> It would be fine to allow this unconditionally if there was some mechanism
> to make sure someone else takes over reaping childs while init is ptraced.

I don't see why this particular issue is any special case. The zombie leak
is just one of many ways that the system might become unusable if root does
the wrong thing to an essential system daemon. Caveat superusor. Diddling
with init on a system you expect ever to do anything again is dangerous and
requires great care. The question of allowing an administrator to engage
in dangerous activities is orthogonal to the details of a particular danger.

With today's kernel, init can avoid any reparented zombies collecting if it
doesn't care about its own children either. That is, it can ignore SIGCHLD
or set SA_NOCLDWAIT to make all its children clean themselves up. That
doesn't help for a normal init, which does care (for respawn and logging).
(Also it's never been tried, and I'm almost sure it has a bug. But that's
supposed to be the semantics.)

That said, the orthogonal question of orphan zombies may well be worth
addressing too. Just let's not conflate it with the ptrace question.
(AFAIK there is no good reason not to make orphans just self-reap, it's
just hysterical raisins inherited from the dawn of Unix. I'm sure that
Oleg and I can work out the cleanups, but in a separate thread please.)


Thanks,
Roland

2008-03-26 15:32:16

by Andrew G. Morgan

[permalink] [raw]
Subject: Re: [PATCH] ptrace: it is fun to strace /sbin/init

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

FWIW. I completely concur.

Pavel Machek wrote:
|> Herbert, Pavel, do you have objections to allowing ptrace of init?
|> (I believe Eric has already Acked the idea iirc?)
|
| No problem from me...
|
| (..but do not introduce command line option or sysctl. It is not worth
it).

Cheers

Andrew
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.7 (Darwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFH6mw8+bHCR3gb8jsRAsKjAJ40JnoyrqGzgIZPgz5gv9uqeeiZ1wCdFKSv
snEU/yiVdWQ4cGSwbU3A8Hg=
=xArK
-----END PGP SIGNATURE-----

2008-03-26 15:54:15

by Pavel Emelyanov

[permalink] [raw]
Subject: Re: [PATCH] ptrace: it is fun to strace /sbin/init

[snip]

> Herbert, Pavel, do you have objections to allowing ptrace of init?
> (I believe Eric has already Acked the idea iirc?)

I 100% agree with the patch.

And I always say this to Oleg about all his patches :)

> thanks,
> -serge
>