linux-kernel - [PATCH] Tolerate signal > 32 in sig_fatal (apparent bug detected by UBSAN)

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Date:	Fri, 29 Apr 2016 21:41:26 -0700
From:	Adam Richter <adamrichter4@...il.com>
To:	linux-kernel@...r.kernel.org
Subject: [PATCH] Tolerate signal > 32 in sig_fatal (apparent bug detected by UBSAN)

I apologize if there is some more specialized mailing list to which I
should have first directed this unreviewed patch.  Please feel free to
redirect me.

The undefined behavior sanitizer ("UBSAN") on 32-bit i386 detected a
case in linux-4.6-rc5/kernel/signal/signal.c where complete_signal
called the sig_fatal test macro on a signal greater than SIGRTMIN (32
on all architectures, I think), which causes it to attempt to shift 32
bits or more of a value that is only 32 bits long.

If the behavior of shifting a 32 bit value by 32 bits or more is shift
it by that value mod 32, then I suspect that this may prevent signals
32+SIGKILL, 32+SIGSUSP and 32+SIGCONT from being caught.  I have not
tried to produce a test program to actually show that this results in
incorrect behavior, but, in case anyone wants to try, I'll mention
that the situation where UBSAN caught the problem appeared to
originate from user level call to tgkill, judging by the stack trace:

__sigqueue_alloc+0x7f/0x190
complete_signal+0x2d6/0x3e0
? release_pages+0x11f/0x4c0
 __send_signal+0x20c/0x740
send_signal+0x34/0x80
do_send_sig_info+0x3a/0x80
do_send_specific+0x63/0xa0
do_tkill+0xb5/0x130
SyS_tgkill+0x1e/0x30

I think the simplest solution to this is pretty trivial, which would
be to add to sig_fatal the same "sig < SIGRTMIN" safeguard that is the
other sig_ test macros that include/linux/signal.h defines.  However,
it happens that sig_fatal uses the macro siginmask, and all other
users siginmask already have the check, so I propose the following
patch, which moves that test into the siginmask macro and removes it
from the callers of siginmask.

Signed-off-by: Adam Richter <adamrichter4@...il.com>

---

diff --git a/include/linux/signal.h b/include/linux/signal.h
index 92557bb..332aa2a 100644
--- a/include/linux/signal.h
+++ b/include/linux/signal.h
@@ -385,7 +385,7 @@ int unhandled_signal(struct task_struct *tsk, int sig);
 #else
 #define rt_sigmask(sig)    sigmask(sig)
 #endif
-#define siginmask(sig, mask) (rt_sigmask(sig) & (mask))
+#define siginmask(sig, mask) (((sig) < SIGRTMIN) && (rt_sigmask(sig) & (mask)))

 #define SIG_KERNEL_ONLY_MASK (\
     rt_sigmask(SIGKILL)   |  rt_sigmask(SIGSTOP))
@@ -406,14 +406,10 @@ int unhandled_signal(struct task_struct *tsk, int sig);
         rt_sigmask(SIGCONT)   |  rt_sigmask(SIGCHLD)   | \
     rt_sigmask(SIGWINCH)  |  rt_sigmask(SIGURG)    )

-#define sig_kernel_only(sig) \
-    (((sig) < SIGRTMIN) && siginmask(sig, SIG_KERNEL_ONLY_MASK))
-#define sig_kernel_coredump(sig) \
-    (((sig) < SIGRTMIN) && siginmask(sig, SIG_KERNEL_COREDUMP_MASK))
-#define sig_kernel_ignore(sig) \
-    (((sig) < SIGRTMIN) && siginmask(sig, SIG_KERNEL_IGNORE_MASK))
-#define sig_kernel_stop(sig) \
-    (((sig) < SIGRTMIN) && siginmask(sig, SIG_KERNEL_STOP_MASK))
+#define sig_kernel_only(sig)     siginmask(sig, SIG_KERNEL_ONLY_MASK)
+#define sig_kernel_coredump(sig) siginmask(sig, SIG_KERNEL_COREDUMP_MASK)
+#define sig_kernel_ignore(sig)   siginmask(sig, SIG_KERNEL_IGNORE_MASK)
+#define sig_kernel_stop(sig)     siginmask(sig, SIG_KERNEL_STOP_MASK)

 #define sig_user_defined(t, signr) \
     (((t)->sighand->action[(signr)-1].sa.sa_handler != SIG_DFL) &&    \