[PATCH 00/10] Removing SEND_SIG_FORCED

[PATCH 00/10] Removing SEND_SIG_FORCED

[Eric W. Biederman]

SEND_SIG_FORCED has two functions.  It forces a pid namespace init to
receive a signal it would ordinarily ignore, and it causes a siginfo to
not be allocated for a signal.

This patchset makes __send_signal a little bit smarter so that it can
detect when to apply these optimizations and the senders of signals
don't have to worry about them.

The coupling of forcing a signal to the pid namespace init and not
allocating siginfo resulted in serveral minor bugs where a signal
sent by the kernel was marked SI_USER suggesting another userspace
process sent that signal.  I have cc'd linux-api in case anyone cares
about these minor userspace visible differences.

Unless someone notices a bug I intend to merge these changes through my
tree during the next merge window.   While there are numerous fixes here
none of them appear to be the kind that fixes real world problems so I
don't see any urgency here.

Please look read and send my your review.  I will be out for about a
week so I will address any comments when I get back.

Eric W. Biederman (10):
      signal: Always ignore SIGKILL and SIGSTOP sent to the global init
      signal: Properly deliver SIGILL from uprobes
      signal: Properly deliver SIGSEGV from x86 uprobes
      signal: Always deliver the kernel's SIGKILL and SIGSTOP to a pid namespace init
      signal: send_sig_all no longer needs SEND_SIG_FORCED
      signal: Remove the siginfo paramater from kernel_dqueue_signal
      signal: Don't send siginfo to kthreads.
      signal: Never allocate siginfo for SIGKILL or SIGSTOP
      signal: Use SEND_SIG_PRIV not SEND_SIG_FORCED with SIGKILL and SIGSTOP
      signal: Remove SEND_SIG_FORCED

 arch/x86/kernel/uprobes.c                    |  2 +-
 drivers/tty/sysrq.c                          |  2 +-
 drivers/usb/gadget/function/f_mass_storage.c |  4 ++--
 fs/jffs2/background.c                        |  2 +-
 include/linux/sched/signal.h                 |  5 ++---
 include/trace/events/signal.h                |  3 +--
 kernel/events/uprobes.c                      |  4 ++--
 kernel/pid_namespace.c                       |  2 +-
 kernel/ptrace.c                              |  4 ++--
 kernel/signal.c                              | 14 +++++++++-----
 mm/oom_kill.c                                |  4 ++--
 11 files changed, 24 insertions(+), 22 deletions(-)

Eric

[PATCH 00/10] Removing SEND_SIG_FORCED

[Eric W. Biederman]

SEND_SIG_FORCED has two functions.  It forces a pid namespace init to
receive a signal it would ordinarily ignore, and it causes a siginfo to
not be allocated for a signal.

This patchset makes __send_signal a little bit smarter so that it can
detect when to apply these optimizations and the senders of signals
don't have to worry about them.

The coupling of forcing a signal to the pid namespace init and not
allocating siginfo resulted in serveral minor bugs where a signal
sent by the kernel was marked SI_USER suggesting another userspace
process sent that signal.  I have cc'd linux-api in case anyone cares
about these minor userspace visible differences.

Unless someone notices a bug I intend to merge these changes through my
tree during the next merge window.   While there are numerous fixes here
none of them appear to be the kind that fixes real world problems so I
don't see any urgency here.

Please look read and send my your review.  I will be out for about a
week so I will address any comments when I get back.

Eric W. Biederman (10):
      signal: Always ignore SIGKILL and SIGSTOP sent to the global init
      signal: Properly deliver SIGILL from uprobes
      signal: Properly deliver SIGSEGV from x86 uprobes
      signal: Always deliver the kernel's SIGKILL and SIGSTOP to a pid namespace init
      signal: send_sig_all no longer needs SEND_SIG_FORCED
      signal: Remove the siginfo paramater from kernel_dqueue_signal
      signal: Don't send siginfo to kthreads.
      signal: Never allocate siginfo for SIGKILL or SIGSTOP
      signal: Use SEND_SIG_PRIV not SEND_SIG_FORCED with SIGKILL and SIGSTOP
      signal: Remove SEND_SIG_FORCED

 arch/x86/kernel/uprobes.c                    |  2 +-
 drivers/tty/sysrq.c                          |  2 +-
 drivers/usb/gadget/function/f_mass_storage.c |  4 ++--
 fs/jffs2/background.c                        |  2 +-
 include/linux/sched/signal.h                 |  5 ++---
 include/trace/events/signal.h                |  3 +--
 kernel/events/uprobes.c                      |  4 ++--
 kernel/pid_namespace.c                       |  2 +-
 kernel/ptrace.c                              |  4 ++--
 kernel/signal.c                              | 14 +++++++++-----
 mm/oom_kill.c                                |  4 ++--
 11 files changed, 24 insertions(+), 22 deletions(-)

Eric

[PATCH 01/10] signal: Always ignore SIGKILL and SIGSTOP sent to the global init

[Eric W. Biederman]

If the first process started (aka /sbin/init) receives a SIGKILL it
will panic the system if it is delivered.  Making the system unusable
and undebugable.  It isn't much better if the first process started
receives SIGSTOP.

So always ignore SIGSTOP and SIGKILL sent to init.

This is done in a separate clause in sig_task_ignored as force_sig_info
can clear SIG_UNKILLABLE and this protection should work even then.

Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
 kernel/signal.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/kernel/signal.c b/kernel/signal.c
index 5843c541fda9..b33264bb2064 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -78,6 +78,10 @@ static bool sig_task_ignored(struct task_struct *t, int sig, bool force)
 
 	handler = sig_handler(t, sig);
 
+	/* SIGKILL and SIGSTOP may not be sent to the global init */
+	if (unlikely(is_global_init(t) && sig_kernel_only(sig)))
+		return true;
+
 	if (unlikely(t->signal->flags & SIGNAL_UNKILLABLE) &&
 	    handler == SIG_DFL && !(force && sig_kernel_only(sig)))
 		return true;
-- 
2.17.1

[PATCH 02/10] signal: Properly deliver SIGILL from uprobes

[Eric W. Biederman]

For userspace to tell the difference between a random signal and an
exception, the exception must include siginfo information.

Using SEND_SIG_FORCED for SIGILL is thus wrong, and it will result
in userspace seeing si_code == SI_USER (like a random signal) instead
of si_code == SI_KERNEL or a more specific si_code as all exceptions
deliver.

Therefore replace force_sig_info(SIGILL, SEND_SIG_FORCE, current)
with force_sig(SIG_ILL, current) which gets this right and is
shorter and easier to type.

Fixes: 014940bad8e4 ("uprobes/x86: Send SIGILL if arch_uprobe_post_xol() fails")
Fixes: 0b5256c7f173 ("uprobes: Send SIGILL if handle_trampoline() fails")
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
 kernel/events/uprobes.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index 3207a4d26849..2bf792d22087 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -1858,7 +1858,7 @@ static void handle_trampoline(struct pt_regs *regs)
 
  sigill:
 	uprobe_warn(current, "handle uretprobe, sending SIGILL.");
-	force_sig_info(SIGILL, SEND_SIG_FORCED, current);
+	force_sig(SIGILL, current);
 
 }
 
@@ -1966,7 +1966,7 @@ static void handle_singlestep(struct uprobe_task *utask, struct pt_regs *regs)
 
 	if (unlikely(err)) {
 		uprobe_warn(current, "execute the probed insn, sending SIGILL.");
-		force_sig_info(SIGILL, SEND_SIG_FORCED, current);
+		force_sig(SIGILL, current);
 	}
 }
 
-- 
2.17.1

[PATCH 03/10] signal: Properly deliver SIGSEGV from x86 uprobes

[Eric W. Biederman]

For userspace to tell the difference between an random signal
and an exception, the exception must include siginfo information.

Using SEND_SIG_FORCED for SIGSEGV is thus wrong, and it will result in
userspace seeing si_code == SI_USER (like a random signal) instead of
si_code == SI_KERNEL or a more specific si_code as all exceptions
deliver.

Therefore replace force_sig_info(SIGSEGV, SEND_SIG_FORCE, current)
with force_sig(SIG_SEGV, current) which gets this right and is shorter
and easier to type.

Fixes: 791eca10107f ("uretprobes/x86: Hijack return address")
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
 arch/x86/kernel/uprobes.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kernel/uprobes.c b/arch/x86/kernel/uprobes.c
index deb576b23b7c..843feb94a950 100644
--- a/arch/x86/kernel/uprobes.c
+++ b/arch/x86/kernel/uprobes.c
@@ -1086,7 +1086,7 @@ arch_uretprobe_hijack_return_addr(unsigned long trampoline_vaddr, struct pt_regs
 		pr_err("return address clobbered: pid=%d, %%sp=%#lx, %%ip=%#lx\n",
 		       current->pid, regs->sp, regs->ip);
 
-		force_sig_info(SIGSEGV, SEND_SIG_FORCED, current);
+		force_sig(SIGSEGV, current);
 	}
 
 	return -1;
-- 
2.17.1

[PATCH 04/10] signal: Always deliver the kernel's SIGKILL and SIGSTOP to a pid namespace init

[Eric W. Biederman]

Instead of playing whack-a-mole and changing SEND_SIG_PRIV to
SEND_SIG_FORCED throughout the kernel to ensure a pid namespace init
gets signals sent by the kernel, stop allowing a pid namespace init to
ignore SIGKILL or SIGSTOP sent by the kernel.  A pid namespace init is
only supposed to be able to ignore signals sent from itself and
children with SIG_DFL.

Fixes: 921cf9f63089 ("signals: protect cinit from unblocked SIG_DFL signals")
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
 kernel/signal.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/signal.c b/kernel/signal.c
index b33264bb2064..8081ab79e97d 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -1039,7 +1039,7 @@ static int __send_signal(int sig, struct siginfo *info, struct task_struct *t,
 
 	result = TRACE_SIGNAL_IGNORED;
 	if (!prepare_signal(sig, t,
-			from_ancestor_ns || (info == SEND_SIG_FORCED)))
+			from_ancestor_ns || (info == SEND_SIG_PRIV) || (info == SEND_SIG_FORCED)))
 		goto ret;
 
 	pending = (type != PIDTYPE_PID) ? &t->signal->shared_pending : &t->pending;
-- 
2.17.1

[PATCH 05/10] signal: send_sig_all no longer needs SEND_SIG_FORCED

[Eric W. Biederman]

Now that send_signal always delivers SEND_SIG_PRIV signals to a pid
namespace init it is no longer necessary to use SEND_SIG_FORCED when
calling do_send_sig_info to ensure that pid namespace inits are
signaled and possibly killed.  Using SEND_SIG_PRIV is sufficient.

So use SEND_SIG_PRIV so that userspace when it receives a SIGTERM can
tell that the kernel sent the signal and not some random userspace
application.

Fixes: b82c32872db2 ("sysrq: use SEND_SIG_FORCED instead of force_sig()")
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
 drivers/tty/sysrq.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/tty/sysrq.c b/drivers/tty/sysrq.c
index 06ed20dd01ba..ad1ee5d01b53 100644
--- a/drivers/tty/sysrq.c
+++ b/drivers/tty/sysrq.c
@@ -348,7 +348,7 @@ static void send_sig_all(int sig)
 		if (is_global_init(p))
 			continue;
 
-		do_send_sig_info(sig, SEND_SIG_FORCED, p, PIDTYPE_MAX);
+		do_send_sig_info(sig, SEND_SIG_PRIV, p, PIDTYPE_MAX);
 	}
 	read_unlock(&tasklist_lock);
 }
-- 
2.17.1

[PATCH 06/10] signal: Remove the siginfo paramater from kernel_dqueue_signal

[Eric W. Biederman]

None of the callers use the it so remove it.

Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
 drivers/usb/gadget/function/f_mass_storage.c | 2 +-
 fs/jffs2/background.c                        | 2 +-
 include/linux/sched/signal.h                 | 4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/gadget/function/f_mass_storage.c b/drivers/usb/gadget/function/f_mass_storage.c
index ca8a4b53c59f..70038a475c9f 100644
--- a/drivers/usb/gadget/function/f_mass_storage.c
+++ b/drivers/usb/gadget/function/f_mass_storage.c
@@ -2311,7 +2311,7 @@ static void handle_exception(struct fsg_common *common)
 	 * into a high-priority EXIT exception.
 	 */
 	for (;;) {
-		int sig = kernel_dequeue_signal(NULL);
+		int sig = kernel_dequeue_signal();
 		if (!sig)
 			break;
 		if (sig != SIGUSR1) {
diff --git a/fs/jffs2/background.c b/fs/jffs2/background.c
index 453a6a1fff34..2b4d5013dc5d 100644
--- a/fs/jffs2/background.c
+++ b/fs/jffs2/background.c
@@ -125,7 +125,7 @@ static int jffs2_garbage_collect_thread(void *_c)
 			if (try_to_freeze())
 				goto again;
 
-			signr = kernel_dequeue_signal(NULL);
+			signr = kernel_dequeue_signal();
 
 			switch(signr) {
 			case SIGSTOP:
diff --git a/include/linux/sched/signal.h b/include/linux/sched/signal.h
index 1be35729c2c5..9b6968cbde14 100644
--- a/include/linux/sched/signal.h
+++ b/include/linux/sched/signal.h
@@ -272,14 +272,14 @@ extern void ignore_signals(struct task_struct *);
 extern void flush_signal_handlers(struct task_struct *, int force_default);
 extern int dequeue_signal(struct task_struct *tsk, sigset_t *mask, siginfo_t *info);
 
-static inline int kernel_dequeue_signal(siginfo_t *info)
+static inline int kernel_dequeue_signal(void)
 {
 	struct task_struct *tsk = current;
 	siginfo_t __info;
 	int ret;
 
 	spin_lock_irq(&tsk->sighand->siglock);
-	ret = dequeue_signal(tsk, &tsk->blocked, info ?: &__info);
+	ret = dequeue_signal(tsk, &tsk->blocked, &__info);
 	spin_unlock_irq(&tsk->sighand->siglock);
 
 	return ret;
-- 
2.17.1

[PATCH 07/10] signal: Don't send siginfo to kthreads.

[Eric W. Biederman]

Today kernel threads never dequeue siginfo so it is pointless to
enqueue siginfo for them.  The usb gadget mass storage driver goes
one farther and uses SEND_SIG_FORCED to guarantee that no siginfo is
even enqueued.

Generalize the optimization of the usb mass storage driver and never
perform an unnecessary allocation when delivering signals to kthreads.

Switch the mass storage driver from sending signals with
SEND_SIG_FORCED to SEND_SIG_PRIV.  As using SEND_SIG_FORCED is now
unnecessary.

Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
 drivers/usb/gadget/function/f_mass_storage.c | 2 +-
 kernel/signal.c                              | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/gadget/function/f_mass_storage.c b/drivers/usb/gadget/function/f_mass_storage.c
index 70038a475c9f..cb402e7a1e9b 100644
--- a/drivers/usb/gadget/function/f_mass_storage.c
+++ b/drivers/usb/gadget/function/f_mass_storage.c
@@ -403,7 +403,7 @@ static void raise_exception(struct fsg_common *common, enum fsg_state new_state)
 		common->exception_req_tag = common->ep0_req_tag;
 		common->state = new_state;
 		if (common->thread_task)
-			send_sig_info(SIGUSR1, SEND_SIG_FORCED,
+			send_sig_info(SIGUSR1, SEND_SIG_PRIV,
 				      common->thread_task);
 	}
 	spin_unlock_irqrestore(&common->lock, flags);
diff --git a/kernel/signal.c b/kernel/signal.c
index 8081ab79e97d..20931a892ace 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -1057,7 +1057,7 @@ static int __send_signal(int sig, struct siginfo *info, struct task_struct *t,
 	 * fast-pathed signals for kernel-internal things like SIGSTOP
 	 * or SIGKILL.
 	 */
-	if (info == SEND_SIG_FORCED)
+	if ((info == SEND_SIG_FORCED) || (t->flags & PF_KTHREAD))
 		goto out_set;
 
 	/*
-- 
2.17.1

[PATCH 08/10] signal: Never allocate siginfo for SIGKILL or SIGSTOP

[Eric W. Biederman]

The SIGKILL and SIGSTOP signals are never delivered to userspace so
queued siginfo for these signals can never be observed.  Therefore
remove the chance of failure by never even attempting to allocate
siginfo in those cases.

Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
 kernel/signal.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/kernel/signal.c b/kernel/signal.c
index 20931a892ace..d7d1adf735f4 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -1054,10 +1054,11 @@ static int __send_signal(int sig, struct siginfo *info, struct task_struct *t,
 
 	result = TRACE_SIGNAL_DELIVERED;
 	/*
-	 * fast-pathed signals for kernel-internal things like SIGSTOP
-	 * or SIGKILL.
+	 * Skip useless siginfo allocation for SIGKILL SIGSTOP,
+	 * and kernel threads.
 	 */
-	if ((info == SEND_SIG_FORCED) || (t->flags & PF_KTHREAD))
+	if ((info == SEND_SIG_FORCED) ||
+	    sig_kernel_only(sig) || (t->flags & PF_KTHREAD))
 		goto out_set;
 
 	/*
-- 
2.17.1

[PATCH 09/10] signal: Use SEND_SIG_PRIV not SEND_SIG_FORCED with SIGKILL and SIGSTOP

[Eric W. Biederman]

Now that siginfo is never allocated for SIGKILL and SIGSTOP there is
no difference between SEND_SIG_PRIV and SEND_SIG_FORCED for SIGKILL
and SIGSTOP.  This makes SEND_SIG_FORCED unnecessary and redundant in
the presence of SIGKILL and SIGSTOP.  Therefore change users of
SEND_SIG_FORCED that are sending SIGKILL or SIGSTOP to use
SEND_SIG_PRIV instead.

This removes the last users of SEND_SIG_FORCED.

Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
 kernel/pid_namespace.c | 2 +-
 kernel/ptrace.c        | 4 ++--
 mm/oom_kill.c          | 4 ++--
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/kernel/pid_namespace.c b/kernel/pid_namespace.c
index 2a2ac53d8b8b..c8d53397bbdd 100644
--- a/kernel/pid_namespace.c
+++ b/kernel/pid_namespace.c
@@ -216,7 +216,7 @@ void zap_pid_ns_processes(struct pid_namespace *pid_ns)
 	idr_for_each_entry_continue(&pid_ns->idr, pid, nr) {
 		task = pid_task(pid, PIDTYPE_PID);
 		if (task && !__fatal_signal_pending(task))
-			send_sig_info(SIGKILL, SEND_SIG_FORCED, task);
+			send_sig_info(SIGKILL, SEND_SIG_PRIV, task);
 	}
 	read_unlock(&tasklist_lock);
 	rcu_read_unlock();
diff --git a/kernel/ptrace.c b/kernel/ptrace.c
index 21fec73d45d4..45f77a1b9c97 100644
--- a/kernel/ptrace.c
+++ b/kernel/ptrace.c
@@ -396,7 +396,7 @@ static int ptrace_attach(struct task_struct *task, long request,
 
 	/* SEIZE doesn't trap tracee on attach */
 	if (!seize)
-		send_sig_info(SIGSTOP, SEND_SIG_FORCED, task);
+		send_sig_info(SIGSTOP, SEND_SIG_PRIV, task);
 
 	spin_lock(&task->sighand->siglock);
 
@@ -563,7 +563,7 @@ void exit_ptrace(struct task_struct *tracer, struct list_head *dead)
 
 	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);
+			send_sig_info(SIGKILL, SEND_SIG_PRIV, p);
 
 		if (__ptrace_detach(tracer, p))
 			list_add(&p->ptrace_entry, dead);
diff --git a/mm/oom_kill.c b/mm/oom_kill.c
index b5b25e4dcbbb..3bcfeaaeed87 100644
--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c
@@ -858,7 +858,7 @@ static void __oom_kill_process(struct task_struct *victim)
 	 * in order to prevent the OOM victim from depleting the memory
 	 * reserves from the user space under its control.
 	 */
-	do_send_sig_info(SIGKILL, SEND_SIG_FORCED, victim, PIDTYPE_TGID);
+	do_send_sig_info(SIGKILL, SEND_SIG_PRIV, victim, PIDTYPE_TGID);
 	mark_oom_victim(victim);
 	pr_err("Killed process %d (%s) total-vm:%lukB, anon-rss:%lukB, file-rss:%lukB, shmem-rss:%lukB\n",
 		task_pid_nr(victim), victim->comm, K(victim->mm->total_vm),
@@ -896,7 +896,7 @@ static void __oom_kill_process(struct task_struct *victim)
 		 */
 		if (unlikely(p->flags & PF_KTHREAD))
 			continue;
-		do_send_sig_info(SIGKILL, SEND_SIG_FORCED, p, PIDTYPE_TGID);
+		do_send_sig_info(SIGKILL, SEND_SIG_PRIV, p, PIDTYPE_TGID);
 	}
 	rcu_read_unlock();
 
-- 
2.17.1

[PATCH 10/10] signal: Remove SEND_SIG_FORCED

[Eric W. Biederman]

There are no more users of SEND_SIG_FORCED so it may be safely removed.

Remove the definition of SEND_SIG_FORCED, it's use in is_si_special,
it's use in TP_STORE_SIGINFO, and it's use in __send_signal as without
any users the uses of SEND_SIG_FORCED are now unncessary.

This makes the code simpler, easier to understand and use.  Users of
signal sending functions now no longer need to ask themselves do I
need to use SEND_SIG_FORCED.

Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
 include/linux/sched/signal.h  | 1 -
 include/trace/events/signal.h | 3 +--
 kernel/signal.c               | 7 +++----
 3 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/include/linux/sched/signal.h b/include/linux/sched/signal.h
index 9b6968cbde14..9e07f3521549 100644
--- a/include/linux/sched/signal.h
+++ b/include/linux/sched/signal.h
@@ -477,7 +477,6 @@ static inline int kill_cad_pid(int sig, int priv)
 /* These can be the second arg to send_sig_info/send_group_sig_info.  */
 #define SEND_SIG_NOINFO ((struct siginfo *) 0)
 #define SEND_SIG_PRIV	((struct siginfo *) 1)
-#define SEND_SIG_FORCED	((struct siginfo *) 2)
 
 /*
  * True if we are on the alternate signal stack.
diff --git a/include/trace/events/signal.h b/include/trace/events/signal.h
index 86582923d51c..3deeed50ffd0 100644
--- a/include/trace/events/signal.h
+++ b/include/trace/events/signal.h
@@ -11,8 +11,7 @@
 
 #define TP_STORE_SIGINFO(__entry, info)				\
 	do {							\
-		if (info == SEND_SIG_NOINFO ||			\
-		    info == SEND_SIG_FORCED) {			\
+		if (info == SEND_SIG_NOINFO) {			\
 			__entry->errno	= 0;			\
 			__entry->code	= SI_USER;		\
 		} else if (info == SEND_SIG_PRIV) {		\
diff --git a/kernel/signal.c b/kernel/signal.c
index d7d1adf735f4..ec136fda457a 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -736,7 +736,7 @@ static void flush_sigqueue_mask(sigset_t *mask, struct sigpending *s)
 
 static inline int is_si_special(const struct siginfo *info)
 {
-	return info <= SEND_SIG_FORCED;
+	return info <= SEND_SIG_PRIV;
 }
 
 static inline bool si_fromuser(const struct siginfo *info)
@@ -1039,7 +1039,7 @@ static int __send_signal(int sig, struct siginfo *info, struct task_struct *t,
 
 	result = TRACE_SIGNAL_IGNORED;
 	if (!prepare_signal(sig, t,
-			from_ancestor_ns || (info == SEND_SIG_PRIV) || (info == SEND_SIG_FORCED)))
+			from_ancestor_ns || (info == SEND_SIG_PRIV)))
 		goto ret;
 
 	pending = (type != PIDTYPE_PID) ? &t->signal->shared_pending : &t->pending;
@@ -1057,8 +1057,7 @@ static int __send_signal(int sig, struct siginfo *info, struct task_struct *t,
 	 * Skip useless siginfo allocation for SIGKILL SIGSTOP,
 	 * and kernel threads.
 	 */
-	if ((info == SEND_SIG_FORCED) ||
-	    sig_kernel_only(sig) || (t->flags & PF_KTHREAD))
+	if (sig_kernel_only(sig) || (t->flags & PF_KTHREAD))
 		goto out_set;
 
 	/*
-- 
2.17.1

Re: [PATCH 00/10] Removing SEND_SIG_FORCED

[Thomas Gleixner]

On Mon, 3 Sep 2018, Eric W. Biederman wrote:
 
> SEND_SIG_FORCED has two functions.  It forces a pid namespace init to
> receive a signal it would ordinarily ignore, and it causes a siginfo to
> not be allocated for a signal.
> 
> This patchset makes __send_signal a little bit smarter so that it can
> detect when to apply these optimizations and the senders of signals
> don't have to worry about them.
> 
> The coupling of forcing a signal to the pid namespace init and not
> allocating siginfo resulted in serveral minor bugs where a signal
> sent by the kernel was marked SI_USER suggesting another userspace
> process sent that signal.  I have cc'd linux-api in case anyone cares
> about these minor userspace visible differences.
> 
> Unless someone notices a bug I intend to merge these changes through my
> tree during the next merge window.   While there are numerous fixes here
> none of them appear to be the kind that fixes real world problems so I
> don't see any urgency here.
> 
> Please look read and send my your review.  I will be out for about a
> week so I will address any comments when I get back.

Went through it carefuly and could not spot anything. Great work!

Reviewed-by: Thomas Gleixner <tglx@linutronix.de>

[PATCH 0/4] signal: Cleanups

[Eric W. Biederman]

What follows are some small cleanups to signal handling functions that
arose from my previous work on sorting out signals during fork.

There are two cases where I did not previously mark functions
that sent signals to a group of processes.  It was safe because
they sent SIGKILL but it is still a good idea to mark that they send
signals to a group of processes to be consistent.

After that I just have the trivial cleanups of removing
specific_send_sig_info which confuses more than helps when I read the
code, and moving the exports of signal functions next to their
callers so that future cleanups are easier.

On my wish list is to continue this work and ensure all of the signal
sending functions are being used correctly and to reduce the set of
signal sending functions so that there is less confusion in how to use
them.

I took a quick look at limiting force_sig to only the callers
that use it to transmit a synchronous exception.  Unfortunately I
discovered that where there is one small bug there are others so those
use cases need something more than a brainless mass conversion to fix.

So I am first going to focus on finishing on cleaning up the signal
sending functions for exceptions that send siginfo.  I have just a few
architectures left and I just need to double check that I didn't do
anything stupid and post those changes for review.

If anyone sees any issues with these patches let me know.  Otherwise I
am going to merge them into my siginfo tree.

Eric W. Biederman (4):
      tty_io: Use group_send_sig_info in __do_SACK to note it is a session being killed
      signal: Use group_send_sig_info to kill all processes in a pid namespace
      signal: Remove specific_send_sig_info
      signal: Pair exports with their functions

 drivers/tty/tty_io.c   |  6 +++---
 kernel/pid_namespace.c |  2 +-
 kernel/signal.c        | 25 +++++++++----------------
 3 files changed, 13 insertions(+), 20 deletions(-)

Eric

[PATCH 1/4] tty_io: Use group_send_sig_info in __do_SACK to note it is a session being killed

[Eric W. Biederman]

Replace send_sig and force_sig in __do_SAK with group_send_sig_info
the general helper for sending a signal to a process group.  This is
wordier but it allows specifying PIDTYPE_SID so that the signal code
knows the signal went to a session.

Both force_sig() and send_sig(..., 1) specify SEND_SIG_PRIV and the
new call of group_send_sig_info does that explicitly.  This is enough
to ensure even a pid namespace init is killed.

The global init remains unkillable.  The guarantee that __do_SAK tries
to provide is a clean path to login to a machine.  As the global init is
unkillable, if it chooses to hold open a tty it can violate this
guarantee.  A technique other than killing processes would be needed
to provide this guarantee to userspace.

The only difference between force_sig and send_sig when sending
SIGKILL is that SIGNAL_UNKILLABLE is cleared.  This has no affect on
the processing of a signal sent with SEND_SIG_PRIV by any process, making
it unnecessary, and not behavior that needs to be preserved.

force_sig was used originally because it did not take as many locks as
send_sig.  Today send_sig, force_sig and group_send_sig_info take the
same locks when delivering a signal.

group_send_sig_info also contains a permission check that force_sig
and send_sig do not.  However the presence of SEND_SIG_PRIV makes the
permission check a noop.  So the permission check does not result
in any behavioral differences.

Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
 drivers/tty/tty_io.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/tty_io.c b/drivers/tty/tty_io.c
index 32bc3e3fe4d3..6553247a761f 100644
--- a/drivers/tty/tty_io.c
+++ b/drivers/tty/tty_io.c
@@ -2738,7 +2738,7 @@ void __do_SAK(struct tty_struct *tty)
 	do_each_pid_task(session, PIDTYPE_SID, p) {
 		tty_notice(tty, "SAK: killed process %d (%s): by session\n",
 			   task_pid_nr(p), p->comm);
-		send_sig(SIGKILL, p, 1);
+		group_send_sig_info(SIGKILL, SEND_SIG_PRIV, p, PIDTYPE_SID);
 	} while_each_pid_task(session, PIDTYPE_SID, p);
 
 	/* Now kill any processes that happen to have the tty open */
@@ -2746,7 +2746,7 @@ void __do_SAK(struct tty_struct *tty)
 		if (p->signal->tty == tty) {
 			tty_notice(tty, "SAK: killed process %d (%s): by controlling tty\n",
 				   task_pid_nr(p), p->comm);
-			send_sig(SIGKILL, p, 1);
+			group_send_sig_info(SIGKILL, SEND_SIG_PRIV, p, PIDTYPE_SID);
 			continue;
 		}
 		task_lock(p);
@@ -2754,7 +2754,7 @@ void __do_SAK(struct tty_struct *tty)
 		if (i != 0) {
 			tty_notice(tty, "SAK: killed process %d (%s): by fd#%d\n",
 				   task_pid_nr(p), p->comm, i - 1);
-			force_sig(SIGKILL, p);
+			group_send_sig_info(SIGKILL, SEND_SIG_PRIV, p, PIDTYPE_SID);
 		}
 		task_unlock(p);
 	} while_each_thread(g, p);
-- 
2.17.1

[PATCH 2/4] signal: Use group_send_sig_info to kill all processes in a pid namespace

[Eric W. Biederman]

Replace send_sig_info in zap_pid_ns_processes with
group_send_sig_info.  This makes more sense as the entire process
group is being killed.  More importantly this allows the kill of those
processes with PIDTYPE_MAX to indicate all of the process in the pid
namespace are being signaled.  This is needed for fork to detect when
signals are sent to a group of processes.

Admittedly fork has another case to catch SIGKILL but the principle remains
that it is desirable to know when a group of processes is being signaled.

Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
 kernel/pid_namespace.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/pid_namespace.c b/kernel/pid_namespace.c
index c8d53397bbdd..aa6e72fb7c08 100644
--- a/kernel/pid_namespace.c
+++ b/kernel/pid_namespace.c
@@ -216,7 +216,7 @@ void zap_pid_ns_processes(struct pid_namespace *pid_ns)
 	idr_for_each_entry_continue(&pid_ns->idr, pid, nr) {
 		task = pid_task(pid, PIDTYPE_PID);
 		if (task && !__fatal_signal_pending(task))
-			send_sig_info(SIGKILL, SEND_SIG_PRIV, task);
+			group_send_sig_info(SIGKILL, SEND_SIG_PRIV, task, PIDTYPE_MAX);
 	}
 	read_unlock(&tasklist_lock);
 	rcu_read_unlock();
-- 
2.17.1

[PATCH 3/4] signal: Remove specific_send_sig_info

[Eric W. Biederman]

This function is static and it only has two callers.  As
specific_send_sig_info is only called twice remembering what
specific_send_sig_info does when reading the code is difficutl and it
makes it hard to see which sending sending functions are equivalent to
which others.

So remove specific_send_sig_info to make the code easier to read.

Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
 kernel/signal.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/kernel/signal.c b/kernel/signal.c
index ec136fda457a..99e91163c9a3 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -1199,12 +1199,6 @@ __group_send_sig_info(int sig, struct siginfo *info, struct task_struct *p)
 	return send_signal(sig, info, p, PIDTYPE_TGID);
 }
 
-static int
-specific_send_sig_info(int sig, struct siginfo *info, struct task_struct *t)
-{
-	return send_signal(sig, info, t, PIDTYPE_PID);
-}
-
 int do_send_sig_info(int sig, struct siginfo *info, struct task_struct *p,
 			enum pid_type type)
 {
@@ -1254,7 +1248,7 @@ force_sig_info(int sig, struct siginfo *info, struct task_struct *t)
 	 */
 	if (action->sa.sa_handler == SIG_DFL && !t->ptrace)
 		t->signal->flags &= ~SIGNAL_UNKILLABLE;
-	ret = specific_send_sig_info(sig, info, t);
+	ret = send_signal(sig, info, t, PIDTYPE_PID);
 	spin_unlock_irqrestore(&t->sighand->siglock, flags);
 
 	return ret;
@@ -2330,7 +2324,7 @@ static int ptrace_signal(int signr, siginfo_t *info)
 
 	/* If the (new) signal is now blocked, requeue it.  */
 	if (sigismember(&current->blocked, signr)) {
-		specific_send_sig_info(signr, info, current);
+		send_signal(signr, info, current, PIDTYPE_PID);
 		signr = 0;
 	}
 
-- 
2.17.1

[PATCH 4/4] signal: Pair exports with their functions

[Eric W. Biederman]

For readability and consistency with the other exports in
kernel/signal.c pair the exports of signal sending functions with
their functions, instead of having the exports in one big clump.

Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
 kernel/signal.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/kernel/signal.c b/kernel/signal.c
index 99e91163c9a3..e16278710b36 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -176,6 +176,7 @@ void recalc_sigpending(void)
 		clear_thread_flag(TIF_SIGPENDING);
 
 }
+EXPORT_SYMBOL(recalc_sigpending);
 
 void calculate_sigpending(void)
 {
@@ -466,6 +467,7 @@ void flush_signals(struct task_struct *t)
 	flush_sigqueue(&t->signal->shared_pending);
 	spin_unlock_irqrestore(&t->sighand->siglock, flags);
 }
+EXPORT_SYMBOL(flush_signals);
 
 #ifdef CONFIG_POSIX_TIMERS
 static void __flush_itimer_signals(struct sigpending *pending)
@@ -684,6 +686,7 @@ int dequeue_signal(struct task_struct *tsk, sigset_t *mask, siginfo_t *info)
 #endif
 	return signr;
 }
+EXPORT_SYMBOL_GPL(dequeue_signal);
 
 /*
  * Tell a process that it has a new active signal..
@@ -1490,6 +1493,7 @@ int send_sig_info(int sig, struct siginfo *info, struct task_struct *p)
 
 	return do_send_sig_info(sig, info, p, PIDTYPE_PID);
 }
+EXPORT_SYMBOL(send_sig_info);
 
 #define __si_special(priv) \
 	((priv) ? SEND_SIG_PRIV : SEND_SIG_NOINFO)
@@ -1499,11 +1503,13 @@ send_sig(int sig, struct task_struct *p, int priv)
 {
 	return send_sig_info(sig, __si_special(priv), p);
 }
+EXPORT_SYMBOL(send_sig);
 
 void force_sig(int sig, struct task_struct *p)
 {
 	force_sig_info(sig, SEND_SIG_PRIV, p);
 }
+EXPORT_SYMBOL(force_sig);
 
 /*
  * When things go south during signal handling, we
@@ -2634,14 +2640,6 @@ void exit_signals(struct task_struct *tsk)
 	}
 }
 
-EXPORT_SYMBOL(recalc_sigpending);
-EXPORT_SYMBOL_GPL(dequeue_signal);
-EXPORT_SYMBOL(flush_signals);
-EXPORT_SYMBOL(force_sig);
-EXPORT_SYMBOL(send_sig);
-EXPORT_SYMBOL(send_sig_info);
-EXPORT_SYMBOL(sigprocmask);
-
 /*
  * System call entry points.
  */
@@ -2735,6 +2733,7 @@ int sigprocmask(int how, sigset_t *set, sigset_t *oldset)
 	__set_current_blocked(&newset);
 	return 0;
 }
+EXPORT_SYMBOL(sigprocmask);
 
 /**
  *  sys_rt_sigprocmask - change the list of currently blocked signals
-- 
2.17.1