PT_EXITKILL (Was: pdeath_signal)

PT_EXITKILL (Was: pdeath_signal)

[Oleg Nesterov]

(add lkml/cc's)

On 11/07, Amnon Shiloh wrote:
>
> > Quoting Oleg Nesterov (oleg@redhat.com):
> > >
> > > On 11/06, Amnon Shiloh wrote:
> > > >
> > > > What I would IDEALLY like to have is a call, probably a ptrace option,
> > > > where the parent can request: "If I am ever to terminate or be killed,
> > > > then my ptraced son MUST die as well".
> > >
> > > Perhaps this makes sense...
> > >
> > > Chris, iirc you also suggested something like this? And the patch is
> > > trivial.
> > >
> > > Oleg.
> > >
> > > --- x/kernel/ptrace.c
> > > +++ x/kernel/ptrace.c
> > > @@ -393,8 +393,12 @@ static bool __ptrace_detach(struct task_
> > >
> > >  	__ptrace_unlink(p);
> > >
> > > -	if (p->exit_state != EXIT_ZOMBIE)
> > > +	if (p->exit_state != EXIT_ZOMBIE) {
> > > +		if ((tracer->flags & PF_EXITING) &&
> > > +		    (p->ptrace & PT_KILL_IF_TRACER_EXITS))
> > > +			send_sig_info(SIGKILL, SEND_SIG_FORCED, p);

No. This is wrong.

We should send SIGKILL before __ptrace_unlink() which clears ->ptrace.
Otherwise (in theory) the tracee can raise its capabilities in between.

Lets change exit_ptrace() to do this, see the patch at the end.

> That would be just wonderful, just what I need
> - it will solve me so much pain!

OK. Please see the untested/uncompiled (but trivial) patch below

	- it adds PTRACE_O_EXITKILL. A better name?

	- A better numeric value? Note that the new option is not equal to
	  the last-ptrace-option << 1. Because currently all options have
	  the event, and the new one starts the eventless group. 1 << 16
	  means we have the room for 8 more events.

	- it needs the convincing changelog for akpm

> Speaking of things I need, here is another:
>
> I have a SUID-root service, which ordinary users can launch.
> This service keeps its original real-UID so that its calling user
> can send it signals, which is fine because it catches them all and
> handles them appropriately.
>
> It is not even a problem if the user kills my service using SIGKILL
> (because that closes all its external sockets), but my service is
> helpless against a SIGSTOP because it cannot be caught and stopping
> the service in an abrupt, non-orderly fashion might disrupt other users.
> (currently I solve this by having another central service watch all instances
> of my service periodically

Well, this central service can ptrace them and nack SIGSTOP... I agree
this doesn't look nice too, but:

> What I wish is that I could request (using "prctl" or whatever):
> "If a non-privileged user sends me a SIGSTOP, then let it be converted into...",

I hope we won't do this ;) But I am not going to argue if you convince
other people.

To me it would be better to simply allow to catch SIGSTOP, but I hope
we won't do this too.

Oleg.


--- x/include/uapi/linux/ptrace.h
+++ x/include/uapi/linux/ptrace.h
@@ -73,7 +73,10 @@
 #define PTRACE_O_TRACEEXIT	(1 << PTRACE_EVENT_EXIT)
 #define PTRACE_O_TRACESECCOMP	(1 << PTRACE_EVENT_SECCOMP)
 
-#define PTRACE_O_MASK		0x000000ff
+/* eventless options */
+#define PTRACE_O_EXITKILL	(1 << 16)
+
+#define PTRACE_O_MASK		(0x000000ff | PTRACE_O_EXITKILL)
 
 #include <asm/ptrace.h>
 
--- x/include/linux/ptrace.h
+++ x/include/linux/ptrace.h
@@ -32,6 +32,8 @@
 #define PT_TRACE_EXIT		PT_EVENT_FLAG(PTRACE_EVENT_EXIT)
 #define PT_TRACE_SECCOMP	PT_EVENT_FLAG(PTRACE_EVENT_SECCOMP)
 
+#define PT_EXITKILL		(PTRACE_O_EXITKILL << PT_OPT_FLAG_SHIFT)
+
 /* single stepping state bits (used on ARM and PA-RISC) */
 #define PT_SINGLESTEP_BIT	31
 #define PT_SINGLESTEP		(1<<PT_SINGLESTEP_BIT)
--- x/kernel/ptrace.c
+++ x/kernel/ptrace.c
@@ -457,6 +457,9 @@ void exit_ptrace(struct task_struct *tra
 		return;
 
 	list_for_each_entry_safe(p, n, &tracer->ptraced, ptrace_entry) {
+		if (unlikely(p->ptrace & PT_EXITKILL))
+			send_sig_info(SIGKILL, SEND_SIG_FORCED, p);
+
 		if (__ptrace_detach(tracer, p))
 			list_add(&p->ptrace_entry, &ptrace_dead);
 	}

Re: PT_EXITKILL (Was: pdeath_signal)

[Amnon Shiloh]

Dear Oleg,

Thanks for the patch, I tried it and it works nicely!

(except that I have no "include/uapi/" directory for
"include/uapi/linux/ptrace.h", so I applied that part
of the patch to "include/linux/ptrace.h" as well).

Also, I just noticed that this new option (PTRACE_O_EXITKILL) is not
safe with ptrace(PTRACE_TRACEME)+execve, because the parent may die
before even having a chance to set PTRACE_O_EXITKILL!

However, that's easy to fix by using PTRACE_ATTACH instead.

On that other matter:

> > What I wish is that I could request (using "prctl" or whatever):
> > "If a non-privileged user sends me a SIGSTOP, then let it be converted into...",
> 
> I hope we won't do this ;) But I am not going to argue if you convince
> other people.
> 
> To me it would be better to simply allow to catch SIGSTOP, but I hope
> we won't do this too.

I don't think anyone should seriously contemplate catching SIGSTOP -
that would break so many applications, including mine.

Now about "convincing", I have that application that really needs this
feature, and I believe that others may be in the same predicament, which is:

1. The application is a SUID-root service, available to ordinary users.
2. Users who started this application are allowed at any time to signal
   or kill their instance(s) of this application.
3. It is alright for the application to be killed by SIGKILL.
4. The application catches and does something useful and positive with
   all other signals sent to it by the invoking user, including SIGTSTP,
   SIGTTIN and SIGTTOU.
5. If the application is unpreparedly stopped by SIGSTOP, which it cannot
   catch, then this may disrupt other instances of this application by
   other users (including, in my case, on other computers connected with
   the application by TCP/IP sockets).

What I ask is simple and can be so easily implemented, essentially in
"kernel/signal.c":

static int check_kill_permission(int sig, struct siginfo *info,
				 struct task_struct *t)
{
	struct pid *sid;
	int error;

	if (!valid_signal(sig))
		return -EINVAL;

	if (!si_fromuser(info))
		return 0;

	error = audit_signal_info(sig, t); /* Let audit system see the signal */
	if (error)
		return error;

+	if (sig == SIGSTOP && (t->flags & PF_NO_SIGSTOP) && !capable(CAP_KILL))
+		return -EPERM;
+
	if (!same_thread_group(current, t) &&
	    !kill_ok_by_cred(t)) {
		switch (sig) {
...

In addition, only the super-user (or CAP_SYS_ADMIN, etc.) should be
allowed to set/reset PF_NO_SIGSTOP (and if taking up a precious PF_xxx
flag is a problem, then the flag could be somewhere else).

Thanks,
Amnon.


Oleg Nesterov wrote:
> 
> (add lkml/cc's)
> 
> On 11/07, Amnon Shiloh wrote:
> >
> > > Quoting Oleg Nesterov (oleg@redhat.com):
> > > >
> > > > On 11/06, Amnon Shiloh wrote:
> > > > >
> > > > > What I would IDEALLY like to have is a call, probably a ptrace option,
> > > > > where the parent can request: "If I am ever to terminate or be killed,
> > > > > then my ptraced son MUST die as well".
> > > >
> > > > Perhaps this makes sense...
> > > >
> > > > Chris, iirc you also suggested something like this? And the patch is
> > > > trivial.
> > > >
> > > > Oleg.
> > > >
> > > > --- x/kernel/ptrace.c
> > > > +++ x/kernel/ptrace.c
> > > > @@ -393,8 +393,12 @@ static bool __ptrace_detach(struct task_
> > > >
> > > >  	__ptrace_unlink(p);
> > > >
> > > > -	if (p->exit_state != EXIT_ZOMBIE)
> > > > +	if (p->exit_state != EXIT_ZOMBIE) {
> > > > +		if ((tracer->flags & PF_EXITING) &&
> > > > +		    (p->ptrace & PT_KILL_IF_TRACER_EXITS))
> > > > +			send_sig_info(SIGKILL, SEND_SIG_FORCED, p);
> 
> No. This is wrong.
> 
> We should send SIGKILL before __ptrace_unlink() which clears ->ptrace.
> Otherwise (in theory) the tracee can raise its capabilities in between.
> 
> Lets change exit_ptrace() to do this, see the patch at the end.
> 
> > That would be just wonderful, just what I need
> > - it will solve me so much pain!
> 
> OK. Please see the untested/uncompiled (but trivial) patch below
> 
> 	- it adds PTRACE_O_EXITKILL. A better name?
> 
> 	- A better numeric value? Note that the new option is not equal to
> 	  the last-ptrace-option << 1. Because currently all options have
> 	  the event, and the new one starts the eventless group. 1 << 16
> 	  means we have the room for 8 more events.
> 
> 	- it needs the convincing changelog for akpm
> 
> > Speaking of things I need, here is another:
> >
> > I have a SUID-root service, which ordinary users can launch.
> > This service keeps its original real-UID so that its calling user
> > can send it signals, which is fine because it catches them all and
> > handles them appropriately.
> >
> > It is not even a problem if the user kills my service using SIGKILL
> > (because that closes all its external sockets), but my service is
> > helpless against a SIGSTOP because it cannot be caught and stopping
> > the service in an abrupt, non-orderly fashion might disrupt other users.
> > (currently I solve this by having another central service watch all instances
> > of my service periodically
> 
> Well, this central service can ptrace them and nack SIGSTOP... I agree
> this doesn't look nice too, but:
> 
> > What I wish is that I could request (using "prctl" or whatever):
> > "If a non-privileged user sends me a SIGSTOP, then let it be converted into...",
> 
> I hope we won't do this ;) But I am not going to argue if you convince
> other people.
> 
> To me it would be better to simply allow to catch SIGSTOP, but I hope
> we won't do this too.
> 
> Oleg.
> 
> 
> --- x/include/uapi/linux/ptrace.h
> +++ x/include/uapi/linux/ptrace.h
> @@ -73,7 +73,10 @@
>  #define PTRACE_O_TRACEEXIT	(1 << PTRACE_EVENT_EXIT)
>  #define PTRACE_O_TRACESECCOMP	(1 << PTRACE_EVENT_SECCOMP)
>  
> -#define PTRACE_O_MASK		0x000000ff
> +/* eventless options */
> +#define PTRACE_O_EXITKILL	(1 << 16)
> +
> +#define PTRACE_O_MASK		(0x000000ff | PTRACE_O_EXITKILL)
>  
>  #include <asm/ptrace.h>
>  
> --- x/include/linux/ptrace.h
> +++ x/include/linux/ptrace.h
> @@ -32,6 +32,8 @@
>  #define PT_TRACE_EXIT		PT_EVENT_FLAG(PTRACE_EVENT_EXIT)
>  #define PT_TRACE_SECCOMP	PT_EVENT_FLAG(PTRACE_EVENT_SECCOMP)
>  
> +#define PT_EXITKILL		(PTRACE_O_EXITKILL << PT_OPT_FLAG_SHIFT)
> +
>  /* single stepping state bits (used on ARM and PA-RISC) */
>  #define PT_SINGLESTEP_BIT	31
>  #define PT_SINGLESTEP		(1<<PT_SINGLESTEP_BIT)
> --- x/kernel/ptrace.c
> +++ x/kernel/ptrace.c
> @@ -457,6 +457,9 @@ void exit_ptrace(struct task_struct *tra
>  		return;
>  
>  	list_for_each_entry_safe(p, n, &tracer->ptraced, ptrace_entry) {
> +		if (unlikely(p->ptrace & PT_EXITKILL))
> +			send_sig_info(SIGKILL, SEND_SIG_FORCED, p);
> +
>  		if (__ptrace_detach(tracer, p))
>  			list_add(&p->ptrace_entry, &ptrace_dead);
>  	}
> 
> 

Re: PT_EXITKILL (Was: pdeath_signal)

[Oleg Nesterov]

Hi Amnon,

On 11/08, Amnon Shiloh wrote:
>
> Thanks for the patch, I tried it and it works nicely!

OK, thanks. I'll wait for other comments a bit and then send
the patch to Andrew.

> Also, I just noticed that this new option (PTRACE_O_EXITKILL) is not
> safe with ptrace(PTRACE_TRACEME)+execve, because the parent may die
> before even having a chance to set PTRACE_O_EXITKILL!

Not safe? I guess you meant that PTRACE_O_EXITKILL can't prevent
this race? Yes sure. Or the parent can die before it does
PTRACE_O_EXITKILL.

> However, that's easy to fix by using PTRACE_ATTACH instead.

Or the child can check getppid() == saved_parent_pid after PTRACE_TRACEME.

But if you switch to PTRACE_ATTACH, I'd suggest to use PTRACE_SEIZE.
Note that, in particular, this allows you to specify the options at
attach.

As for PF_NO_SIGSTOP, I'll write another email.

Oleg.

Re: PT_EXITKILL (Was: pdeath_signal)

[Amnon Shiloh]

Hi Again,

> Or the child can check getppid() == saved_parent_pid after PTRACE_TRACEME.

In fact that's what I did when I  tested the new option - the child ran:

	volatile long ready = 0;
	int saved_parent_pid = getpid();

	if(!(son = fork())
	{
		while(!ready)
		{
			sched_yield();
			if(getppid() != saved_parent_pid)
				exit(1);
		}
		execve(...);
	}
	ptrace(PTRACE_ATTACH, son, 0, 0);
	waitpid(son, ...);
	ptrace(PTRACE_SETOPTIONS, son, 0, PTRACE_P_TRACEFORK|PTRACE_O_EXITKILL);
	ptrace(PTRACE_POKEDATA, son, &ready, 1);
	/* continue tracing */

I also checked that PTRACE_O_EXITKILL is inherited to grandchildren.

Yes, I may also use PTRACE_SEIZE, but since it is not yet documented,
I will need to read the kernel code more to make extra sure it has no
unwanted side effects.

Best Regards,
Amnon.


Oleg Nesterov wrote:
> 
> Hi Amnon,
> 
> On 11/08, Amnon Shiloh wrote:
> >
> > Thanks for the patch, I tried it and it works nicely!
> 
> OK, thanks. I'll wait for other comments a bit and then send
> the patch to Andrew.
> 
> > Also, I just noticed that this new option (PTRACE_O_EXITKILL) is not
> > safe with ptrace(PTRACE_TRACEME)+execve, because the parent may die
> > before even having a chance to set PTRACE_O_EXITKILL!
> 
> Not safe? I guess you meant that PTRACE_O_EXITKILL can't prevent
> this race? Yes sure. Or the parent can die before it does
> PTRACE_O_EXITKILL.
> 
> > However, that's easy to fix by using PTRACE_ATTACH instead.
> 
> Or the child can check getppid() == saved_parent_pid after PTRACE_TRACEME.
> 
> But if you switch to PTRACE_ATTACH, I'd suggest to use PTRACE_SEIZE.
> Note that, in particular, this allows you to specify the options at
> attach.
> 
> As for PF_NO_SIGSTOP, I'll write another email.
> 
> Oleg.
> 
> 

[PATCH 0/1] ptrace: introduce PTRACE_O_EXITKILL

[Oleg Nesterov]

On 11/08, Oleg Nesterov wrote:
>
> On 11/08, Amnon Shiloh wrote:
> >
> > Thanks for the patch, I tried it and it works nicely!
>
> OK, thanks. I'll wait for other comments a bit and then send
> the patch to Andrew.

OK, nobody cares, so I am sending the same patch to Andrew.
The only change is that the new option uses (1 << 20) rather
than 16 so that we have a bit more room for the new events.

Oleg.

[PATCH 1/1] ptrace: introduce PTRACE_O_EXITKILL

[Oleg Nesterov]

Ptrace jailers want to be sure that the tracee can never escape
from the control. However if the tracer dies unexpectedly the
tracee continues to run in potentially unsafe mode.

Add the new ptrace option PTRACE_O_EXITKILL. If the tracer exits
it sends SIGKILL to every tracee which has this bit set.

Note that the new option is not equal to the last-option << 1.
Because currently all options have the event, and the new one
starts the eventless group. It uses the random 20 bit, so we have
the room for 12 more events, but we can also add the new eventless
options below this one.

Suggested-and-tested-by: Amnon Shiloh <u3557@miso.sublimeip.com>
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
 include/linux/ptrace.h      |    2 ++
 include/uapi/linux/ptrace.h |    5 ++++-
 kernel/ptrace.c             |    3 +++
 3 files changed, 9 insertions(+), 1 deletions(-)

diff --git a/include/linux/ptrace.h b/include/linux/ptrace.h
index e0ff468..62db1a1 100644
--- a/include/linux/ptrace.h
+++ b/include/linux/ptrace.h
@@ -32,6 +32,8 @@
 #define PT_TRACE_EXIT		PT_EVENT_FLAG(PTRACE_EVENT_EXIT)
 #define PT_TRACE_SECCOMP	PT_EVENT_FLAG(PTRACE_EVENT_SECCOMP)
 
+#define PT_EXITKILL		(PTRACE_O_EXITKILL << PT_OPT_FLAG_SHIFT)
+
 /* single stepping state bits (used on ARM and PA-RISC) */
 #define PT_SINGLESTEP_BIT	31
 #define PT_SINGLESTEP		(1<<PT_SINGLESTEP_BIT)
diff --git a/include/uapi/linux/ptrace.h b/include/uapi/linux/ptrace.h
index 1ef6c05..022ab18 100644
--- a/include/uapi/linux/ptrace.h
+++ b/include/uapi/linux/ptrace.h
@@ -73,7 +73,10 @@
 #define PTRACE_O_TRACEEXIT	(1 << PTRACE_EVENT_EXIT)
 #define PTRACE_O_TRACESECCOMP	(1 << PTRACE_EVENT_SECCOMP)
 
-#define PTRACE_O_MASK		0x000000ff
+/* eventless options */
+#define PTRACE_O_EXITKILL	(1 << 20)
+
+#define PTRACE_O_MASK		(0x000000ff | PTRACE_O_EXITKILL)
 
 #include <asm/ptrace.h>
 
diff --git a/kernel/ptrace.c b/kernel/ptrace.c
index 1f5e55d..ec8118a 100644
--- a/kernel/ptrace.c
+++ b/kernel/ptrace.c
@@ -457,6 +457,9 @@ void exit_ptrace(struct task_struct *tracer)
 		return;
 
 	list_for_each_entry_safe(p, n, &tracer->ptraced, ptrace_entry) {
+		if (unlikely(p->ptrace & PT_EXITKILL))
+			send_sig_info(SIGKILL, SEND_SIG_FORCED, p);
+
 		if (__ptrace_detach(tracer, p))
 			list_add(&p->ptrace_entry, &ptrace_dead);
 	}
-- 
1.5.5.1

PF_NO_SIGSTOP (Was: PT_EXITKILL)

[Oleg Nesterov]

On 11/08, Amnon Shiloh wrote:
>
> > > What I wish is that I could request (using "prctl" or whatever):
> > > "If a non-privileged user sends me a SIGSTOP, then let it be converted into...",
> >
> > I hope we won't do this ;) But I am not going to argue if you convince
> > other people.
> >
> > To me it would be better to simply allow to catch SIGSTOP, but I hope
> > we won't do this too.
>
> I don't think anyone should seriously contemplate catching SIGSTOP -
> that would break so many applications, including mine.
>
> Now about "convincing", I have that application that really needs this
> feature, and I believe that others may be in the same predicament, which is:
>
> 1. The application is a SUID-root service, available to ordinary users.
> 2. Users who started this application are allowed at any time to signal
>    or kill their instance(s) of this application.

Is this the only reason why this service keeps its original real-UID?
(see below)

> 3. It is alright for the application to be killed by SIGKILL.
> 4. The application catches and does something useful and positive with
>    all other signals sent to it by the invoking user, including SIGTSTP,
>    SIGTTIN and SIGTTOU.
> 5. If the application is unpreparedly stopped by SIGSTOP, which it cannot
>    catch, then this may disrupt other instances of this application by
>    other users (including, in my case, on other computers connected with
>    the application by TCP/IP sockets).
>
> What I ask is simple and can be so easily implemented, essentially in
> "kernel/signal.c":
>
> static int check_kill_permission(int sig, struct siginfo *info,
> 				 struct task_struct *t)
> {
> ...
> +	if (sig == SIGSTOP && (t->flags & PF_NO_SIGSTOP) && !capable(CAP_KILL))
                               ^^^^^^^^^^^^^^^^^^^^^^^^
No, this is not enough. At least PF_NO_SIGSTOP should be per-process,
not per-thread. But I agree, it is simpe to implement.

So once again, no need to convince me ;) I try to never argue with
the new features, even if personally I do not really like this idea.
If someone acks your idea I will be happy to help with the patch.


And I have another idea... Not that I like it very much, but it looks
simple and maybe useful.

What if we introduce SA_NOSECURITY? So that if an application does

	sa.sa_flags = SA_NOSECURITY | ...;

	sigaction(SIG, &sa, NULL);

then sys_kill/etc bypasses security checks.

This way your service can run as root and still recieve the signals
from the ordinary users. Yes, except SIGKILL/SIGSTOP.

Oleg.

Re: PF_NO_SIGSTOP (Was: PT_EXITKILL)

[Amnon Shiloh]

Hi Oleg,

> Is this the only reason why this service keeps its original real-UID?
> (see below)

Allowing the user who invoked my service to send it signals is the main
reason for keeping the original real-UID, but not the only one.

> What if we introduce SA_NOSECURITY? So that if an application does
> 
> 	sa.sa_flags = SA_NOSECURITY | ...;
> 
> 	sigaction(SIG, &sa, NULL);
> 
> then sys_kill/etc bypasses security checks.
> 
> This way your service can run as root and still recieve the signals
> from the ordinary users. Yes, except SIGKILL/SIGSTOP.

Well, I see two more problems here:
1. I actually would like to be able to receive SIGKILL - it is legitimate
   and not uncommon for the calling user to kill the service.
   (also to stop it, but they can use SIGTSTP just the same,
    then the service may suspend itself in an orderly fashion)
2. I must not allow OTHER users to signal the service.
   - I suppose this can be handled in most cases by checking the "si_uid"
   field of the siginfo, but a user may kill a process if either their
   real-uid or effective-uid allows it, so "si_uid" may not show it if
   they are different (then I would discard the signal and the killer
   will not even receive an EPERM error).

> > +	if (sig == SIGSTOP && (t->flags & PF_NO_SIGSTOP) && !capable(CAP_KILL))
>                                ^^^^^^^^^^^^^^^^^^^^^^^^
> No, this is not enough. At least PF_NO_SIGSTOP should be per-process,
> not per-thread. But I agree, it is simpe to implement.

Oh, I didn't think of that because my service is not multi-threaded.
As far as I am concerned, I don't care whether it is per-process or
per-thread, but I agree: if you stop just one thread in a critical
section or in the middle of a mutex operation, the other threads are
not going to be that happy...

Whether per-process or per-thread, this should be easier to implement
and have a clearer interface than "SA_NOSECURITY".

Best Regards,
Amnon.


> On 11/08, Amnon Shiloh wrote:
> >
> > > > What I wish is that I could request (using "prctl" or whatever):
> > > > "If a non-privileged user sends me a SIGSTOP, then let it be converted into...",
> > >
> > > I hope we won't do this ;) But I am not going to argue if you convince
> > > other people.
> > >
> > > To me it would be better to simply allow to catch SIGSTOP, but I hope
> > > we won't do this too.
> >
> > I don't think anyone should seriously contemplate catching SIGSTOP -
> > that would break so many applications, including mine.
> >
> > Now about "convincing", I have that application that really needs this
> > feature, and I believe that others may be in the same predicament, which is:
> >
> > 1. The application is a SUID-root service, available to ordinary users.
> > 2. Users who started this application are allowed at any time to signal
> >    or kill their instance(s) of this application.
> 
> Is this the only reason why this service keeps its original real-UID?
> (see below)
> 
> > 3. It is alright for the application to be killed by SIGKILL.
> > 4. The application catches and does something useful and positive with
> >    all other signals sent to it by the invoking user, including SIGTSTP,
> >    SIGTTIN and SIGTTOU.
> > 5. If the application is unpreparedly stopped by SIGSTOP, which it cannot
> >    catch, then this may disrupt other instances of this application by
> >    other users (including, in my case, on other computers connected with
> >    the application by TCP/IP sockets).
> >
> > What I ask is simple and can be so easily implemented, essentially in
> > "kernel/signal.c":
> >
> > static int check_kill_permission(int sig, struct siginfo *info,
> > 				 struct task_struct *t)
> > {
> > ...
> > +	if (sig == SIGSTOP && (t->flags & PF_NO_SIGSTOP) && !capable(CAP_KILL))
>                                ^^^^^^^^^^^^^^^^^^^^^^^^
> No, this is not enough. At least PF_NO_SIGSTOP should be per-process,
> not per-thread. But I agree, it is simpe to implement.
> 
> So once again, no need to convince me ;) I try to never argue with
> the new features, even if personally I do not really like this idea.
> If someone acks your idea I will be happy to help with the patch.
> 
> 
> And I have another idea... Not that I like it very much, but it looks
> simple and maybe useful.
> 
> What if we introduce SA_NOSECURITY? So that if an application does
> 
> 	sa.sa_flags = SA_NOSECURITY | ...;
> 
> 	sigaction(SIG, &sa, NULL);
> 
> then sys_kill/etc bypasses security checks.
> 
> This way your service can run as root and still recieve the signals
> from the ordinary users. Yes, except SIGKILL/SIGSTOP.
> 
> Oleg.
> 
> 

Re: PT_EXITKILL (Was: pdeath_signal)

[Pedro Alves]

On 11/07/2012 03:09 PM, Oleg Nesterov wrote:

>> > > > > What I would IDEALLY like to have is a call, probably a ptrace option,
>> > > > > where the parent can request: "If I am ever to terminate or be killed,
>> > > > > then my ptraced son MUST die as well".
>> > >
>> > > Perhaps this makes sense...
>> > >
>> > > Chris, iirc you also suggested something like this? And the patch is
>> > > trivial.

(...)

> OK. Please see the untested/uncompiled (but trivial) patch below
> 
> 	- it adds PTRACE_O_EXITKILL. A better name?
> 
> 	- A better numeric value? Note that the new option is not equal to
> 	  the last-ptrace-option << 1. Because currently all options have
> 	  the event, and the new one starts the eventless group. 1 << 16
> 	  means we have the room for 8 more events.
> 
> 	- it needs the convincing changelog for akpm


If this isn't inherited by the ptrace child's children, a fork child can
end up detached if the tracer dies before it had a chance of setting
the PTRACE_O_EXITKILL on the new auto-attached child.

Which sounds like another argument for PTRACE_O_INHERIT, as in:
http://sourceware.org/ml/archer/2011-q1/msg00026.html

(it sounds like you need to use PTRACE_SEIZE+options too to plug
the race between PTRACE_ME/PTRACE_ATTACH and
setting PTRACE_SETOPTIONS).

(For completeness, Windows' age old equivalent,
DebugSetProcessKillOnExit, it a tracer option, not tracee option, though
that's not as flexible.)

-- 
Pedro Alves

Re: PT_EXITKILL (Was: pdeath_signal)

[Oleg Nesterov]

On 11/08, Pedro Alves wrote:
>
> On 11/07/2012 03:09 PM, Oleg Nesterov wrote:
>
> > OK. Please see the untested/uncompiled (but trivial) patch below
> >
> > 	- it adds PTRACE_O_EXITKILL. A better name?
> >
> > 	- A better numeric value? Note that the new option is not equal to
> > 	  the last-ptrace-option << 1. Because currently all options have
> > 	  the event, and the new one starts the eventless group. 1 << 16
> > 	  means we have the room for 8 more events.
> >
> > 	- it needs the convincing changelog for akpm
>
>
> If this isn't inherited by the ptrace child's children, a fork child can
> end up detached if the tracer dies before it had a chance of setting
> the PTRACE_O_EXITKILL on the new auto-attached child.

It is copied like the other options.

> Which sounds like another argument for PTRACE_O_INHERIT, as in:
> http://sourceware.org/ml/archer/2011-q1/msg00026.html

	The point of PTRACE_O_INHERIT would be to attach newly-created threads and
	children without causing an event stop and the attendant overhead.

this is another thing, I guess.

> (it sounds like you need to use PTRACE_SEIZE+options too to plug
> the race between PTRACE_ME/PTRACE_ATTACH and
> setting PTRACE_SETOPTIONS).

Agreed, PTRACE_SEIZE+options is better.

> (For completeness, Windows' age old equivalent,
> DebugSetProcessKillOnExit, it a tracer option, not tracee option, though
> that's not as flexible.)

Thanks ;)

Oleg.

Re: PT_EXITKILL (Was: pdeath_signal)

[Pedro Alves]

On 11/08/2012 12:44 PM, Oleg Nesterov wrote:
> On 11/08, Pedro Alves wrote:

>> If this isn't inherited by the ptrace child's children, a fork child can
>> end up detached if the tracer dies before it had a chance of setting
>> the PTRACE_O_EXITKILL on the new auto-attached child.
> 
> It is copied like the other options.

Oh, you're right.  I got confused - GDB has code to always set options
on the fork children after PTRACE_EVENT_(V)FORK.   Dunno where that came from.

>> Which sounds like another argument for PTRACE_O_INHERIT, as in:
>> http://sourceware.org/ml/archer/2011-q1/msg00026.html
> 
> 	The point of PTRACE_O_INHERIT would be to attach newly-created threads and
> 	children without causing an event stop and the attendant overhead.
> 
> this is another thing, I guess.

Yes, yes.

-- 
Pedro Alves