Skip to content

Commit 00b06da

Browse files
committed
signal: Add SA_IMMUTABLE to ensure forced siganls do not get changed
As Andy pointed out that there are races between force_sig_info_to_task and sigaction[1] when force_sig_info_task. As Kees discovered[2] ptrace is also able to change these signals. In the case of seeccomp killing a process with a signal it is a security violation to allow the signal to be caught or manipulated. Solve this problem by introducing a new flag SA_IMMUTABLE that prevents sigaction and ptrace from modifying these forced signals. This flag is carefully made kernel internal so that no new ABI is introduced. Longer term I think this can be solved by guaranteeing short circuit delivery of signals in this case. Unfortunately reliable and guaranteed short circuit delivery of these signals is still a ways off from being implemented, tested, and merged. So I have implemented a much simpler alternative for now. [1] https://lkml.kernel.org/r/[email protected] [2] https://lkml.kernel.org/r/202110281136.5CE65399A7@keescook Cc: [email protected] Fixes: 307d522 ("signal/seccomp: Refactor seccomp signal and coredump generation") Tested-by: Andrea Righi <[email protected]> Tested-by: Kees Cook <[email protected]> Signed-off-by: "Eric W. Biederman" <[email protected]>
1 parent e21294a commit 00b06da

File tree

3 files changed

+11
-1
lines changed

3 files changed

+11
-1
lines changed

include/linux/signal_types.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,9 @@ struct ksignal {
7070
int sig;
7171
};
7272

73+
/* Used to kill the race between sigaction and forced signals */
74+
#define SA_IMMUTABLE 0x00800000
75+
7376
#ifndef __ARCH_UAPI_SA_FLAGS
7477
#ifdef SA_RESTORER
7578
#define __ARCH_UAPI_SA_FLAGS SA_RESTORER

include/uapi/asm-generic/signal-defs.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
#define SA_UNSUPPORTED 0x00000400
4646
#define SA_EXPOSE_TAGBITS 0x00000800
4747
/* 0x00010000 used on mips */
48+
/* 0x00800000 used for internal SA_IMMUTABLE */
4849
/* 0x01000000 used on x86 */
4950
/* 0x02000000 used on x86 */
5051
/*

kernel/signal.c

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1336,6 +1336,7 @@ force_sig_info_to_task(struct kernel_siginfo *info, struct task_struct *t, bool
13361336
blocked = sigismember(&t->blocked, sig);
13371337
if (blocked || ignored || sigdfl) {
13381338
action->sa.sa_handler = SIG_DFL;
1339+
action->sa.sa_flags |= SA_IMMUTABLE;
13391340
if (blocked) {
13401341
sigdelset(&t->blocked, sig);
13411342
recalc_sigpending_and_wake(t);
@@ -2760,7 +2761,8 @@ bool get_signal(struct ksignal *ksig)
27602761
if (!signr)
27612762
break; /* will return 0 */
27622763

2763-
if (unlikely(current->ptrace) && signr != SIGKILL) {
2764+
if (unlikely(current->ptrace) && (signr != SIGKILL) &&
2765+
!(sighand->action[signr -1].sa.sa_flags & SA_IMMUTABLE)) {
27642766
signr = ptrace_signal(signr, &ksig->info);
27652767
if (!signr)
27662768
continue;
@@ -4110,6 +4112,10 @@ int do_sigaction(int sig, struct k_sigaction *act, struct k_sigaction *oact)
41104112
k = &p->sighand->action[sig-1];
41114113

41124114
spin_lock_irq(&p->sighand->siglock);
4115+
if (k->sa.sa_flags & SA_IMMUTABLE) {
4116+
spin_unlock_irq(&p->sighand->siglock);
4117+
return -EINVAL;
4118+
}
41134119
if (oact)
41144120
*oact = *k;
41154121

0 commit comments

Comments
 (0)