[PATCH] prctl: add PR_[GS]ET_KILLABLE

Subsystems: the rest

STALE2887d

16 messages, 5 authors, 2018-09-06 · open the first message on its own page

[PATCH] prctl: add PR_[GS]ET_KILLABLE

From: Jürg Billeter <hidden>
Date: 2018-07-30 08:01:13

PR_SET_KILLABLE clears the SIGNAL_UNKILLABLE flag. This allows
CLONE_NEWPID tasks to restore normal signal behavior, opting out of the
special signal protection for init processes.

This is required for job control in a shell that uses CLONE_NEWPID for
child processes.

This prctl does not allow setting the SIGNAL_UNKILLABLE flag, only
clearing.

Signed-off-by: Jürg Billeter <redacted>
---
 include/uapi/linux/prctl.h |  4 ++++
 kernel/sys.c               | 11 +++++++++++
 2 files changed, 15 insertions(+)
diff --git a/include/uapi/linux/prctl.h b/include/uapi/linux/prctl.h
index c0d7ea0bf5b6..92afb63da727 100644
--- a/include/uapi/linux/prctl.h
+++ b/include/uapi/linux/prctl.h
@@ -219,4 +219,8 @@ struct prctl_mm_map {
 # define PR_SPEC_DISABLE		(1UL << 2)
 # define PR_SPEC_FORCE_DISABLE		(1UL << 3)
 
+/* Control SIGNAL_UNKILLABLE */
+#define PR_GET_KILLABLE			54
+#define PR_SET_KILLABLE			55
+
 #endif /* _LINUX_PRCTL_H */
diff --git a/kernel/sys.c b/kernel/sys.c
index 38509dc1f77b..264de630d548 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -2484,6 +2484,17 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3,
 			return -EINVAL;
 		error = arch_prctl_spec_ctrl_set(me, arg2, arg3);
 		break;
+	case PR_GET_KILLABLE:
+		if (arg3 || arg4 || arg5)
+			return -EINVAL;
+		error = put_user(!(me->signal->flags & SIGNAL_UNKILLABLE),
+				 (int __user *)arg2);
+		break;
+	case PR_SET_KILLABLE:
+		if (arg2 != 1 || arg3 || arg4 || arg5)
+			return -EINVAL;
+		me->signal->flags &= ~SIGNAL_UNKILLABLE;
+		break;
 	default:
 		error = -EINVAL;
 		break;
-- 
2.18.0

Re: [PATCH] prctl: add PR_[GS]ET_KILLABLE

From: Oleg Nesterov <oleg@redhat.com>
Date: 2018-07-30 10:17:04

On 07/30, Jürg Billeter wrote:
This is required for job control in a shell that uses CLONE_NEWPID for
child processes.
Could you explain in more details?
+	case PR_SET_KILLABLE:
+		if (arg2 != 1 || arg3 || arg4 || arg5)
+			return -EINVAL;
+		me->signal->flags &= ~SIGNAL_UNKILLABLE;
this needs spin_lock_irq(me->sighand->siglock).

Oleg.

Re: [PATCH] prctl: add PR_[GS]ET_KILLABLE

From: Jürg Billeter <hidden>
Date: 2018-07-30 19:33:08

On Mon, 2018-07-30 at 12:17 +0200, Oleg Nesterov wrote:
On 07/30, Jürg Billeter wrote:
quoted
This is required for job control in a shell that uses CLONE_NEWPID for
child processes.
Could you explain in more details?
The SIGNAL_UNKILLABLE flag, which is implicitly set for tasks cloned
with CLONE_NEWPID, has the effect of ignoring all signals (from
userspace) if the corresponding handler is set to SIG_DFL. The only
exceptions are SIGKILL and SIGSTOP and they are only accepted if raised
from an ancestor namespace.

SIGINT, SIGQUIT and SIGTSTP are used in job control for ^C, ^\, ^Z.
While a task with the SIGNAL_UNKILLABLE flag could install handlers for
these signals, this is not sufficient to implement a shell that uses
CLONE_NEWPID for child processes:

 * As SIGSTOP is ignored when raised from the SIGNAL_UNKILLABLE process
   itself, I don't think it's possible to implement the stop action in
   a custom SIGTSTP handler.
 * Many applications do not install handlers for these signals and
   thus, job control won't work properly with unmodified applications.

Job control in a shell is just an example. There are other scenarios,
of course, where applications rely on the default actions as described
in signal(7), and PID isolation may be useful. In my opinion, the
kernel support for preventing accidental killing of the "init" process
should really be optional and this new prctl provides this without
breaking backward compatibility.
quoted
+	case PR_SET_KILLABLE:
+		if (arg2 != 1 || arg3 || arg4 || arg5)
+			return -EINVAL;
+		me->signal->flags &= ~SIGNAL_UNKILLABLE;
this needs spin_lock_irq(me->sighand->siglock).
Thanks for the review, will fix this for v2.

Jürg

Re: [PATCH] prctl: add PR_[GS]ET_KILLABLE

From: Thomas Gleixner <hidden>
Date: 2018-07-30 19:39:41

On Mon, 30 Jul 2018, Jürg Billeter wrote:
On Mon, 2018-07-30 at 12:17 +0200, Oleg Nesterov wrote:
quoted
On 07/30, Jürg Billeter wrote:
quoted
This is required for job control in a shell that uses CLONE_NEWPID for
child processes.
Could you explain in more details?
The SIGNAL_UNKILLABLE flag, which is implicitly set for tasks cloned
with CLONE_NEWPID, has the effect of ignoring all signals (from
userspace) if the corresponding handler is set to SIG_DFL. The only
exceptions are SIGKILL and SIGSTOP and they are only accepted if raised
from an ancestor namespace.

SIGINT, SIGQUIT and SIGTSTP are used in job control for ^C, ^\, ^Z.
While a task with the SIGNAL_UNKILLABLE flag could install handlers for
these signals, this is not sufficient to implement a shell that uses
CLONE_NEWPID for child processes:

 * As SIGSTOP is ignored when raised from the SIGNAL_UNKILLABLE process
   itself, I don't think it's possible to implement the stop action in
   a custom SIGTSTP handler.
 * Many applications do not install handlers for these signals and
   thus, job control won't work properly with unmodified applications.

Job control in a shell is just an example. There are other scenarios,
of course, where applications rely on the default actions as described
in signal(7), and PID isolation may be useful. In my opinion, the
kernel support for preventing accidental killing of the "init" process
should really be optional and this new prctl provides this without
breaking backward compatibility.
This makes sense and exactly that information needs to be in the changelog.

Thanks,

	tglx

[PATCH v2] prctl: add PR_[GS]ET_KILLABLE

From: Jürg Billeter <hidden>
Date: 2018-07-31 07:04:10

PR_SET_KILLABLE clears the SIGNAL_UNKILLABLE flag. This allows
CLONE_NEWPID tasks to restore normal signal behavior, opting out of the
special signal protection for init processes. This prctl does not allow
setting the SIGNAL_UNKILLABLE flag, only clearing.

The SIGNAL_UNKILLABLE flag, which is implicitly set for tasks cloned
with CLONE_NEWPID, has the effect of ignoring all signals (from
userspace) if the corresponding handler is set to SIG_DFL. The only
exceptions are SIGKILL and SIGSTOP and they are only accepted if raised
from an ancestor namespace.

SIGINT, SIGQUIT and SIGTSTP are used in job control for ^C, ^\, ^Z.
While a task with the SIGNAL_UNKILLABLE flag could install handlers for
these signals, this is not sufficient to implement a shell that uses
CLONE_NEWPID for child processes:

 * As SIGSTOP is ignored when raised from the SIGNAL_UNKILLABLE process
   itself, it's not possible to implement the stop action in a custom
   SIGTSTP handler.
 * Many applications do not install handlers for these signals and
   thus, job control won't work properly with unmodified applications.

There are other scenarios besides job control in a shell where
applications rely on the default actions as described in signal(7) and
PID isolation may be useful. This new prctl makes the signal protection
for "init" processes optional, without breaking backward compatibility.

Signed-off-by: Jürg Billeter <redacted>
---
 v2: Hold siglock for PR_SET_KILLABLE, expand commit message.

 include/uapi/linux/prctl.h |  4 ++++
 kernel/sys.c               | 13 +++++++++++++
 2 files changed, 17 insertions(+)
diff --git a/include/uapi/linux/prctl.h b/include/uapi/linux/prctl.h
index c0d7ea0bf5b6..92afb63da727 100644
--- a/include/uapi/linux/prctl.h
+++ b/include/uapi/linux/prctl.h
@@ -219,4 +219,8 @@ struct prctl_mm_map {
 # define PR_SPEC_DISABLE		(1UL << 2)
 # define PR_SPEC_FORCE_DISABLE		(1UL << 3)
 
+/* Control SIGNAL_UNKILLABLE */
+#define PR_GET_KILLABLE			54
+#define PR_SET_KILLABLE			55
+
 #endif /* _LINUX_PRCTL_H */
diff --git a/kernel/sys.c b/kernel/sys.c
index 38509dc1f77b..92c9322cfb98 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -2484,6 +2484,19 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3,
 			return -EINVAL;
 		error = arch_prctl_spec_ctrl_set(me, arg2, arg3);
 		break;
+	case PR_GET_KILLABLE:
+		if (arg3 || arg4 || arg5)
+			return -EINVAL;
+		error = put_user(!(me->signal->flags & SIGNAL_UNKILLABLE),
+				 (int __user *)arg2);
+		break;
+	case PR_SET_KILLABLE:
+		if (arg2 != 1 || arg3 || arg4 || arg5)
+			return -EINVAL;
+		spin_lock_irq(&me->sighand->siglock);
+		me->signal->flags &= ~SIGNAL_UNKILLABLE;
+		spin_unlock_irq(&me->sighand->siglock);
+		break;
 	default:
 		error = -EINVAL;
 		break;
-- 
2.18.0

Re: [PATCH v2] prctl: add PR_[GS]ET_KILLABLE

From: Oleg Nesterov <oleg@redhat.com>
Date: 2018-07-31 14:39:56

On 07/31, Jürg Billeter wrote:
PR_SET_KILLABLE clears the SIGNAL_UNKILLABLE flag. This allows
CLONE_NEWPID tasks to restore normal signal behavior, opting out of the
special signal protection for init processes. This prctl does not allow
setting the SIGNAL_UNKILLABLE flag, only clearing.

The SIGNAL_UNKILLABLE flag, which is implicitly set for tasks cloned
with CLONE_NEWPID, has the effect of ignoring all signals (from
userspace) if the corresponding handler is set to SIG_DFL. The only
exceptions are SIGKILL and SIGSTOP and they are only accepted if raised
from an ancestor namespace.

SIGINT, SIGQUIT and SIGTSTP are used in job control for ^C, ^\, ^Z.
While a task with the SIGNAL_UNKILLABLE flag could install handlers for
these signals, this is not sufficient to implement a shell that uses
CLONE_NEWPID for child processes:
Ah. My question wasn't clear, sorry.

Could you explain your use-case? Why a shell wants to use CLONE_NEWPID?
And what do we actually want in, say, ^Z case? Just stop the child reaper
or may be it would be better to stop the whole pid namespace?
 * As SIGSTOP is ignored when raised from the SIGNAL_UNKILLABLE process
   itself, it's not possible to implement the stop action in a custom
   SIGTSTP handler.
Yes. So may be we actually want to change __isig() paths to use
SEND_SIG_FORCED (this is not that simple), or perhaps we can change
__send_signal() to not drop SIGSTOP sent to itself, or may be we can even
introduce SIG_DFL_EVEN_IF_INIT, I dunno.
 * Many applications do not install handlers for these signals and
   thus, job control won't work properly with unmodified applications.
I can't understand this. An application should be changed anyway to do
PR_SET_KILLABLE?


Let me clarify. I am not arguing with this patch, probably it makes sense in
any case. I am just trying to understand your real motivation for this change.


+	case PR_SET_KILLABLE:
+		if (arg2 != 1 || arg3 || arg4 || arg5)
+			return -EINVAL;
+		spin_lock_irq(&me->sighand->siglock);
+		me->signal->flags &= ~SIGNAL_UNKILLABLE;
+		spin_unlock_irq(&me->sighand->siglock);
OK, but then you need to change the CLONE_PARENT/SIGNAL_UNKILLABLE check
in copy_process().

Oleg.

Re: [PATCH v2] prctl: add PR_[GS]ET_KILLABLE

From: Jürg Billeter <hidden>
Date: 2018-07-31 16:13:03

On Tue, 2018-07-31 at 16:39 +0200, Oleg Nesterov wrote:
On 07/31, Jürg Billeter wrote:
quoted
SIGINT, SIGQUIT and SIGTSTP are used in job control for ^C, ^\, ^Z.
While a task with the SIGNAL_UNKILLABLE flag could install handlers for
these signals, this is not sufficient to implement a shell that uses
CLONE_NEWPID for child processes:
Ah. My question wasn't clear, sorry.

Could you explain your use-case? Why a shell wants to use
CLONE_NEWPID?
To guarantee that there won't be any runaway processes, i.e., ensure
that no descendants (background helper daemons or misbehaving
processes) survive when the child process is terminated. And to prevent
children from killing their ancestors. This is not something that can
be always-on in all shells, but it could be an option for users that
want this control/isolation.
And what do we actually want in, say, ^Z case? Just stop the child reaper
or may be it would be better to stop the whole pid namespace?
Stopping the whole PID namespace would be interesting, however, I think
this should be discussed separately if and when there is a proposal to
support this. For now the process group is stopped, same as without PID
namespaces.
quoted
 * As SIGSTOP is ignored when raised from the SIGNAL_UNKILLABLE process
   itself, it's not possible to implement the stop action in a custom
   SIGTSTP handler.
Yes. So may be we actually want to change __isig() paths to use
SEND_SIG_FORCED (this is not that simple), or perhaps we can change
__send_signal() to not drop SIGSTOP sent to itself, or may be we can even
introduce SIG_DFL_EVEN_IF_INIT, I dunno.
In my opinion, my patch is much simpler and also more general as it
covers all scenarios where regular signal handling is required or
desired for "init" processes, with minimal code changes (after
PR_SET_KILLABLE, binaries that expect SIG_DFL to work can be executed
without changes).
quoted
 * Many applications do not install handlers for these signals and
   thus, job control won't work properly with unmodified applications.
I can't understand this. An application should be changed anyway to do
PR_SET_KILLABLE?
PR_SET_KILLABLE can be called (e.g., by the shell) between clone() and
execve(). (Some applications may have issues running as subreaper but
that's a separate matter, signal handling will work as expected).
quoted
+	case PR_SET_KILLABLE:
+		if (arg2 != 1 || arg3 || arg4 || arg5)
+			return -EINVAL;
+		spin_lock_irq(&me->sighand->siglock);
+		me->signal->flags &= ~SIGNAL_UNKILLABLE;
+		spin_unlock_irq(&me->sighand->siglock);
OK, but then you need to change the CLONE_PARENT/SIGNAL_UNKILLABLE check
in copy_process().
Good point, need a different check for the PID namespace root process
in copy_process().

Thanks,
Jürg

Re: [PATCH v2] prctl: add PR_[GS]ET_KILLABLE

From: Oleg Nesterov <oleg@redhat.com>
Date: 2018-08-01 14:19:20

On 07/31, Jürg Billeter wrote:
quoted
Could you explain your use-case? Why a shell wants to use
CLONE_NEWPID?
To guarantee that there won't be any runaway processes, i.e., ensure
that no descendants (background helper daemons or misbehaving
processes) survive when the child process is terminated.
We already have PR_SET_CHILD_SUBREAPER.

Perhaps we can finally add PR_KILL_MY_DESCENDANTS_ON_EXIT? This was already
discussed some time ago, but I can't find the previous discussion... Simple
to implement.
And to prevent
children from killing their ancestors.
OK, this is the only reason for CLONE_NEWPID which I can understand so far.
Not that I understand why this is that useful ;)
quoted
quoted
 * As SIGSTOP is ignored when raised from the SIGNAL_UNKILLABLE process
   itself, it's not possible to implement the stop action in a custom
   SIGTSTP handler.
Yes. So may be we actually want to change __isig() paths to use
SEND_SIG_FORCED (this is not that simple), or perhaps we can change
__send_signal() to not drop SIGSTOP sent to itself, or may be we can even
introduce SIG_DFL_EVEN_IF_INIT, I dunno.
In my opinion, my patch is much simpler and also more general as it
Yes, yes, let me repeat that I am not arguing with your patch, I am just trying
to understand what
quoted
I can't understand this. An application should be changed anyway to do
PR_SET_KILLABLE?
PR_SET_KILLABLE can be called (e.g., by the shell) between clone() and
execve().
OK, this is true.

Oleg.

Re: [PATCH v2] prctl: add PR_[GS]ET_KILLABLE

From: Jürg Billeter <hidden>
Date: 2018-08-03 10:15:33

On Wed, 2018-08-01 at 16:19 +0200, Oleg Nesterov wrote:
On 07/31, Jürg Billeter wrote:
quoted
quoted
Could you explain your use-case? Why a shell wants to use
CLONE_NEWPID?
To guarantee that there won't be any runaway processes, i.e., ensure
that no descendants (background helper daemons or misbehaving
processes) survive when the child process is terminated.
We already have PR_SET_CHILD_SUBREAPER.

Perhaps we can finally add PR_KILL_MY_DESCENDANTS_ON_EXIT? This was already
discussed some time ago, but I can't find the previous discussion... Simple
to implement.
This would definitely be an option. You mentioned it last October in
the PR_SET_PDEATHSIG_PROC discussion¹. However, as PID namespaces
already exist and appear to be a good fit for the most part, I think it
makes sense to just add the missing pieces to PID namespaces instead of
duplicating part of the PID namespace functionality.

Also, based on Eric's comment in that other discussion about
no_new_privs not being allowed to increase the attack surface,
PR_KILL_MY_DESCENDANTS_ON_EXIT might require CAP_SYS_ADMIN as well (due
to setuid children). In which case the only potential benefit would be
that it still allows the child to kill arbitrary processes, as far as I
can tell.
quoted
And to prevent children from killing their ancestors.
OK, this is the only reason for CLONE_NEWPID which I can understand so far.
Not that I understand why this is that useful ;)
The overall goal is increasing isolation between (some) child processes
and the rest of the system. Isolation from runaway processes and
isolation from signals are independent aspects and it could be useful
to control them independently. However, I also expect it to be common
that both are wanted at the same time.

Jürg

¹ https://lkml.org/lkml/2017/10/5/546

Re: [PATCH v2] prctl: add PR_[GS]ET_KILLABLE

From: Oleg Nesterov <oleg@redhat.com>
Date: 2018-08-03 12:14:12

On 08/03, Jürg Billeter wrote:
On Wed, 2018-08-01 at 16:19 +0200, Oleg Nesterov wrote:
quoted
On 07/31, Jürg Billeter wrote:
quoted
quoted
Could you explain your use-case? Why a shell wants to use
CLONE_NEWPID?
To guarantee that there won't be any runaway processes, i.e., ensure
that no descendants (background helper daemons or misbehaving
processes) survive when the child process is terminated.
We already have PR_SET_CHILD_SUBREAPER.

Perhaps we can finally add PR_KILL_MY_DESCENDANTS_ON_EXIT? This was already
discussed some time ago, but I can't find the previous discussion... Simple
to implement.
This would definitely be an option. You mentioned it last October in
the PR_SET_PDEATHSIG_PROC discussion¹. However, as PID namespaces
already exist and appear to be a good fit for the most part,
Sure, if CLONE_NEWPID fits your needs you can use it,
I think it
makes sense to just add the missing pieces to PID namespaces instead of
duplicating part of the PID namespace functionality.
Again, I am not arguing with your change.

PR_KILL_MY_DESCENDANTS_ON_EXIT can make sense just like PR_SET_CHILD_SUBREAPER
even if PID namespace functionality implies both. Simply because CLONE_NEWPID
is not necessarily the best tool, if nothing else you do not necessarily want
the pid isolation.
Also, based on Eric's comment in that other discussion about
no_new_privs not being allowed to increase the attack surface,
PR_KILL_MY_DESCENDANTS_ON_EXIT might require CAP_SYS_ADMIN as well (due
to setuid children).
No, no, the exiting parent should simply do group_send_sig_info(SIGKILL)
for every descendant and rely on check_kill_permission().

OK, lets forget it for now.

Oleg.

Re: [PATCH] prctl: add PR_[GS]ET_KILLABLE

From: Jann Horn <jannh@google.com>
Date: 2018-07-31 16:26:32

On Mon, Jul 30, 2018 at 10:01 AM Jürg Billeter [off-list ref] wrote:
PR_SET_KILLABLE clears the SIGNAL_UNKILLABLE flag. This allows
CLONE_NEWPID tasks to restore normal signal behavior, opting out of the
special signal protection for init processes.

This is required for job control in a shell that uses CLONE_NEWPID for
child processes.

This prctl does not allow setting the SIGNAL_UNKILLABLE flag, only
clearing.

Signed-off-by: Jürg Billeter <redacted>
---
[...]
quoted hunk
diff --git a/kernel/sys.c b/kernel/sys.c
index 38509dc1f77b..264de630d548 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
[...]
+       case PR_SET_KILLABLE:
+               if (arg2 != 1 || arg3 || arg4 || arg5)
+                       return -EINVAL;
+               me->signal->flags &= ~SIGNAL_UNKILLABLE;
+               break;
I don't have an opinion on this patchset otherwise, but should this
prctl maybe block PR_SET_KILLABLE if you're actually the real init
process? This seems like it could potentially lead to weird things.

This code in kernel/fork.c seems to rely on the fact that global init
is SIGNAL_UNKILLABLE, and probably also leads to weirdness if
container init is non-SIGNAL_UNKILLABLE:

        /*
         * Siblings of global init remain as zombies on exit since they are
         * not reaped by their parent (swapper). To solve this and to avoid
         * multi-rooted process trees, prevent global and container-inits
         * from creating siblings.
         */
        if ((clone_flags & CLONE_PARENT) &&
                                current->signal->flags & SIGNAL_UNKILLABLE)
                return ERR_PTR(-EINVAL);

Re: [PATCH] prctl: add PR_[GS]ET_KILLABLE

From: Jürg Billeter <hidden>
Date: 2018-08-01 07:44:10

On Tue, 2018-07-31 at 18:26 +0200, Jann Horn wrote:
On Mon, Jul 30, 2018 at 10:01 AM Jürg Billeter [off-list ref] wrote:

[...]
quoted
diff --git a/kernel/sys.c b/kernel/sys.c
index 38509dc1f77b..264de630d548 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
[...]
quoted
+       case PR_SET_KILLABLE:
+               if (arg2 != 1 || arg3 || arg4 || arg5)
+                       return -EINVAL;
+               me->signal->flags &= ~SIGNAL_UNKILLABLE;
+               break;
I don't have an opinion on this patchset otherwise, but should this
prctl maybe block PR_SET_KILLABLE if you're actually the real init
process? This seems like it could potentially lead to weird things.
While I don't expect global init to use this, I can't think of a good
reason to disallow it in the kernel. Do you have specific concerns or
is the code in kernel/fork.c the only reason? I prefer avoiding special
cases unless really required.
This code in kernel/fork.c seems to rely on the fact that global init
is SIGNAL_UNKILLABLE, and probably also leads to weirdness if
container init is non-SIGNAL_UNKILLABLE:
Yes, Oleg has mentioned this as well. I have to change copy_process()
to directly check for the PID namespace root process instead of
checking for SIGNAL_UNKILLABLE.

Jürg

Re: [PATCH] prctl: add PR_[GS]ET_KILLABLE

From: Jann Horn <jannh@google.com>
Date: 2018-08-01 07:57:11

On Wed, Aug 1, 2018 at 9:44 AM Jürg Billeter [off-list ref] wrote:
On Tue, 2018-07-31 at 18:26 +0200, Jann Horn wrote:
quoted
On Mon, Jul 30, 2018 at 10:01 AM Jürg Billeter [off-list ref] wrote:

[...]
quoted
diff --git a/kernel/sys.c b/kernel/sys.c
index 38509dc1f77b..264de630d548 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
[...]
quoted
+       case PR_SET_KILLABLE:
+               if (arg2 != 1 || arg3 || arg4 || arg5)
+                       return -EINVAL;
+               me->signal->flags &= ~SIGNAL_UNKILLABLE;
+               break;
I don't have an opinion on this patchset otherwise, but should this
prctl maybe block PR_SET_KILLABLE if you're actually the real init
process? This seems like it could potentially lead to weird things.
While I don't expect global init to use this, I can't think of a good
reason to disallow it in the kernel. Do you have specific concerns or
is the code in kernel/fork.c the only reason?
No, I don't have any other specific concerns.
I prefer avoiding special cases unless really required.

[PATCH v3 1/2] fork: do not rely on SIGNAL_UNKILLABLE for init check

From: Jürg Billeter <hidden>
Date: 2018-08-03 14:41:52

copy_process() currently checks the SIGNAL_UNKILLABLE flag to determine
whether to accept CLONE_PARENT. In preparation for allowing init
processes to opt out of SIGNAL_UNKILLABLE, directly check whether the
process is an init process with is_child_reaper().

Signed-off-by: Jürg Billeter <redacted>
---
 kernel/fork.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/fork.c b/kernel/fork.c
index 1b27babc4c78..c019ce461556 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1646,7 +1646,7 @@ static __latent_entropy struct task_struct *copy_process(
 	 * from creating siblings.
 	 */
 	if ((clone_flags & CLONE_PARENT) &&
-				current->signal->flags & SIGNAL_UNKILLABLE)
+				is_child_reaper(task_tgid(current)))
 		return ERR_PTR(-EINVAL);
 
 	/*
-- 
2.18.0

[PATCH v3 2/2] prctl: add PR_[GS]ET_KILLABLE

From: Jürg Billeter <hidden>
Date: 2018-08-03 14:41:49

PR_SET_KILLABLE clears the SIGNAL_UNKILLABLE flag. This allows
CLONE_NEWPID tasks to restore normal signal behavior, opting out of the
special signal protection for init processes. This prctl does not allow
setting the SIGNAL_UNKILLABLE flag, only clearing.

The SIGNAL_UNKILLABLE flag, which is implicitly set for tasks cloned
with CLONE_NEWPID, has the effect of ignoring all signals (from
userspace) if the corresponding handler is set to SIG_DFL. The only
exceptions are SIGKILL and SIGSTOP and they are only accepted if raised
from an ancestor namespace.

SIGINT, SIGQUIT and SIGTSTP are used in job control for ^C, ^\, ^Z.
While a task with the SIGNAL_UNKILLABLE flag could install handlers for
these signals, this is not sufficient to implement a shell that uses
CLONE_NEWPID for child processes:

 * As SIGSTOP is ignored when raised from the SIGNAL_UNKILLABLE process
   itself, it's not possible to implement the stop action in a custom
   SIGTSTP handler.
 * Many applications do not install handlers for these signals and
   thus, job control won't work properly with unmodified applications.

There are other scenarios besides job control in a shell where
applications rely on the default actions as described in signal(7) and
PID isolation may be useful. This new prctl makes the signal protection
for "init" processes optional, without breaking backward compatibility.

Signed-off-by: Jürg Billeter <redacted>
---
 include/uapi/linux/prctl.h |  4 ++++
 kernel/sys.c               | 13 +++++++++++++
 2 files changed, 17 insertions(+)
diff --git a/include/uapi/linux/prctl.h b/include/uapi/linux/prctl.h
index c0d7ea0bf5b6..92afb63da727 100644
--- a/include/uapi/linux/prctl.h
+++ b/include/uapi/linux/prctl.h
@@ -219,4 +219,8 @@ struct prctl_mm_map {
 # define PR_SPEC_DISABLE		(1UL << 2)
 # define PR_SPEC_FORCE_DISABLE		(1UL << 3)
 
+/* Control SIGNAL_UNKILLABLE */
+#define PR_GET_KILLABLE			54
+#define PR_SET_KILLABLE			55
+
 #endif /* _LINUX_PRCTL_H */
diff --git a/kernel/sys.c b/kernel/sys.c
index 38509dc1f77b..92c9322cfb98 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -2484,6 +2484,19 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3,
 			return -EINVAL;
 		error = arch_prctl_spec_ctrl_set(me, arg2, arg3);
 		break;
+	case PR_GET_KILLABLE:
+		if (arg3 || arg4 || arg5)
+			return -EINVAL;
+		error = put_user(!(me->signal->flags & SIGNAL_UNKILLABLE),
+				 (int __user *)arg2);
+		break;
+	case PR_SET_KILLABLE:
+		if (arg2 != 1 || arg3 || arg4 || arg5)
+			return -EINVAL;
+		spin_lock_irq(&me->sighand->siglock);
+		me->signal->flags &= ~SIGNAL_UNKILLABLE;
+		spin_unlock_irq(&me->sighand->siglock);
+		break;
 	default:
 		error = -EINVAL;
 		break;
-- 
2.18.0

Re: [PATCH v3 2/2] prctl: add PR_[GS]ET_KILLABLE

From: Andrew Morton <akpm@linux-foundation.org>
Date: 2018-09-06 22:42:12

On Fri,  3 Aug 2018 16:40:21 +0200 Jürg Billeter [off-list ref] wrote:
PR_SET_KILLABLE clears the SIGNAL_UNKILLABLE flag. This allows
CLONE_NEWPID tasks to restore normal signal behavior, opting out of the
special signal protection for init processes. This prctl does not allow
setting the SIGNAL_UNKILLABLE flag, only clearing.

The SIGNAL_UNKILLABLE flag, which is implicitly set for tasks cloned
with CLONE_NEWPID, has the effect of ignoring all signals (from
userspace) if the corresponding handler is set to SIG_DFL. The only
exceptions are SIGKILL and SIGSTOP and they are only accepted if raised
from an ancestor namespace.

SIGINT, SIGQUIT and SIGTSTP are used in job control for ^C, ^\, ^Z.
While a task with the SIGNAL_UNKILLABLE flag could install handlers for
these signals, this is not sufficient to implement a shell that uses
CLONE_NEWPID for child processes:

 * As SIGSTOP is ignored when raised from the SIGNAL_UNKILLABLE process
   itself, it's not possible to implement the stop action in a custom
   SIGTSTP handler.
 * Many applications do not install handlers for these signals and
   thus, job control won't work properly with unmodified applications.

There are other scenarios besides job control in a shell where
applications rely on the default actions as described in signal(7) and
PID isolation may be useful. This new prctl makes the signal protection
for "init" processes optional, without breaking backward compatibility.
This one is above my pay grade.  Eric & Oleg: could you please provide
input?
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help