4.20-stable review patch. If anyone has any objections, please let me know.
------------------
[ Upstream commit ee17e5d6201c66492a0e8053190fca2ed2b8457d ]
Eric Biggers reported:
The following commit, which went into v4.20, introduced undefined behavior when sys_rt_sigqueueinfo() is called with sig=0:
commit 4ce5f9c9e7546915c559ffae594e6d73f918db00 Author: Eric W. Biederman ebiederm@xmission.com Date: Tue Sep 25 12:59:31 2018 +0200
signal: Use a smaller struct siginfo in the kernelIn sig_specific_sicodes(), used from known_siginfo_layout(), the expression '1ULL << ((sig)-1)' is undefined as it evaluates to 1ULL << 4294967295.
Reproducer:
#include <signal.h> #include <sys/syscall.h> #include <unistd.h>
int main(void) { siginfo_t si = { .si_code = 1 }; syscall(__NR_rt_sigqueueinfo, 0, 0, &si); }
UBSAN report for v5.0-rc1:
UBSAN: Undefined behaviour in kernel/signal.c:2946:7 shift exponent 4294967295 is too large for 64-bit type 'long unsigned int' CPU: 2 PID: 346 Comm: syz_signal Not tainted 5.0.0-rc1 #25 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1 04/01/2014 Call Trace: __dump_stack lib/dump_stack.c:77 [inline] dump_stack+0x70/0xa5 lib/dump_stack.c:113 ubsan_epilogue+0xd/0x40 lib/ubsan.c:159 __ubsan_handle_shift_out_of_bounds+0x12c/0x170 lib/ubsan.c:425 known_siginfo_layout+0xae/0xe0 kernel/signal.c:2946 post_copy_siginfo_from_user kernel/signal.c:3009 [inline] __copy_siginfo_from_user+0x35/0x60 kernel/signal.c:3035 __do_sys_rt_sigqueueinfo kernel/signal.c:3553 [inline] __se_sys_rt_sigqueueinfo kernel/signal.c:3549 [inline] __x64_sys_rt_sigqueueinfo+0x31/0x70 kernel/signal.c:3549 do_syscall_64+0x4c/0x1b0 arch/x86/entry/common.c:290 entry_SYSCALL_64_after_hwframe+0x49/0xbe RIP: 0033:0x433639 Code: c4 18 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 0f 83 7b 27 00 00 c3 66 2e 0f 1f 84 00 00 00 00 RSP: 002b:00007fffcb289fc8 EFLAGS: 00000246 ORIG_RAX: 0000000000000081 RAX: ffffffffffffffda RBX: 00000000004002e0 RCX: 0000000000433639 RDX: 00007fffcb289fd0 RSI: 0000000000000000 RDI: 0000000000000000 RBP: 00000000006b2018 R08: 000000000000004d R09: 0000000000000000 R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000401560 R13: 00000000004015f0 R14: 0000000000000000 R15: 0000000000000000
I have looked at the other callers of siginmask and they all appear to in locations where sig can not be zero.
I have looked at the code generation of adding an extra test against zero and gcc was able with a simple decrement instruction to combine the two tests together. So the at most adding this test cost a single cpu cycle. In practice that decrement instruction was already present as part of the mask comparison, so the only change was when the instruction was executed.
So given that it is cheap, and obviously correct to update siginmask to verify the signal is not zero. Fix this issue there to avoid any future problems.
Reported-by: Eric Biggers ebiggers@kernel.org Fixes: 4ce5f9c9e754 ("signal: Use a smaller struct siginfo in the kernel") Signed-off-by: "Eric W. Biederman" ebiederm@xmission.com Signed-off-by: Sasha Levin sashal@kernel.org --- include/linux/signal.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/signal.h b/include/linux/signal.h index f428e86f4800..b5d99482d3fe 100644 --- a/include/linux/signal.h +++ b/include/linux/signal.h @@ -388,7 +388,7 @@ extern bool unhandled_signal(struct task_struct *tsk, int sig); #endif
#define siginmask(sig, mask) \ - ((sig) < SIGRTMIN && (rt_sigmask(sig) & (mask))) + ((sig) > 0 && (sig) < SIGRTMIN && (rt_sigmask(sig) & (mask)))
#define SIG_KERNEL_ONLY_MASK (\ rt_sigmask(SIGKILL) | rt_sigmask(SIGSTOP))