Messages in this thread Patch in this message | | | Date | Fri, 29 Apr 2016 21:41:26 -0700 | Subject | [PATCH] Tolerate signal > 32 in sig_fatal (apparent bug detected by UBSAN) | From | Adam Richter <> |
| |
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@gmail.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) && \
| |