• Eric W. Biederman's avatar
    signal: Make siginmask safe when passed a signal of 0 · ee17e5d6
    Eric W. Biederman authored
    Eric Biggers reported:
    > The following commit, which went into v4.20, introduced undefined behavior when
    > sys_rt_sigqueueinfo() is called with sig=0:
    >
    > commit 4ce5f9c9
    > Author: Eric W. Biederman <ebiederm@xmission.com>
    > Date:   Tue Sep 25 12:59:31 2018 +0200
    >
    >     signal: Use a smaller struct siginfo in the kernel
    >
    > In 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: 's avatarEric Biggers <ebiggers@kernel.org>
    Fixes: 4ce5f9c9 ("signal: Use a smaller struct siginfo in the kernel")
    Signed-off-by: 's avatar"Eric W. Biederman" <ebiederm@xmission.com>
    ee17e5d6
Name
Last commit
Last update
Documentation Loading commit data...
LICENSES Loading commit data...
arch Loading commit data...
block Loading commit data...
certs Loading commit data...
crypto Loading commit data...
drivers Loading commit data...
firmware Loading commit data...
fs Loading commit data...
include Loading commit data...
init Loading commit data...
ipc Loading commit data...
kernel Loading commit data...
lib Loading commit data...
mm Loading commit data...
net Loading commit data...
samples Loading commit data...
scripts Loading commit data...
security Loading commit data...
sound Loading commit data...
tools Loading commit data...
usr Loading commit data...
virt Loading commit data...
.clang-format Loading commit data...
.cocciconfig Loading commit data...
.get_maintainer.ignore Loading commit data...
.gitattributes Loading commit data...
.gitignore Loading commit data...
.mailmap Loading commit data...
COPYING Loading commit data...
CREDITS Loading commit data...
Kbuild Loading commit data...
Kconfig Loading commit data...
MAINTAINERS Loading commit data...
Makefile Loading commit data...
README Loading commit data...