From c999b933faa5e281e3af2e110eccaf91698b0a81 Mon Sep 17 00:00:00 2001 From: "Eric W. Biederman" Date: Sat, 14 Apr 2018 13:03:25 -0500 Subject: signal: Reduce copy_siginfo_to_user to just copy_to_user Now that every instance of struct siginfo is now initialized it is no longer necessary to copy struct siginfo piece by piece to userspace but instead the entire structure can be copied. As well as making the code simpler and more efficient this means that copy_sinfo_to_user no longer cares which union member of struct siginfo is in use. In practice this means that all 32bit architectures that define FPE_FIXME will handle properly send SI_USER when kill(SIGFPE) is sent. While still performing their historic architectural brokenness when 0 is used a floating pointer signal. This matches the current behavior of 64bit architectures that define FPE_FIXME who get lucky and an overloaded SI_USER has continuted to work through copy_siginfo_to_user because the 8 byte si_addr occupies the same bytes in struct siginfo as the 4 byte si_pid and the 4 byte si_uid. Problematic architectures still need to fix their ABI so that signalfd and 32bit compat code will work properly. Signed-off-by: "Eric W. Biederman" --- kernel/signal.c | 84 ++------------------------------------------------------- 1 file changed, 2 insertions(+), 82 deletions(-) (limited to 'kernel') diff --git a/kernel/signal.c b/kernel/signal.c index d4ccea599692..d56f4d496c89 100644 --- a/kernel/signal.c +++ b/kernel/signal.c @@ -2850,89 +2850,9 @@ enum siginfo_layout siginfo_layout(int sig, int si_code) int copy_siginfo_to_user(siginfo_t __user *to, const siginfo_t *from) { - int err; - - if (!access_ok (VERIFY_WRITE, to, sizeof(siginfo_t))) + if (copy_to_user(to, from , sizeof(struct siginfo))) return -EFAULT; - if (from->si_code < 0) - return __copy_to_user(to, from, sizeof(siginfo_t)) - ? -EFAULT : 0; - /* - * If you change siginfo_t structure, please be sure - * this code is fixed accordingly. - * Please remember to update the signalfd_copyinfo() function - * inside fs/signalfd.c too, in case siginfo_t changes. - * It should never copy any pad contained in the structure - * to avoid security leaks, but must copy the generic - * 3 ints plus the relevant union member. - */ - err = __put_user(from->si_signo, &to->si_signo); - err |= __put_user(from->si_errno, &to->si_errno); - err |= __put_user(from->si_code, &to->si_code); - switch (siginfo_layout(from->si_signo, from->si_code)) { - case SIL_KILL: - err |= __put_user(from->si_pid, &to->si_pid); - err |= __put_user(from->si_uid, &to->si_uid); - break; - case SIL_TIMER: - /* Unreached SI_TIMER is negative */ - break; - case SIL_POLL: - err |= __put_user(from->si_band, &to->si_band); - err |= __put_user(from->si_fd, &to->si_fd); - break; - case SIL_FAULT: - err |= __put_user(from->si_addr, &to->si_addr); -#ifdef __ARCH_SI_TRAPNO - err |= __put_user(from->si_trapno, &to->si_trapno); -#endif -#ifdef __ia64__ - err |= __put_user(from->si_imm, &to->si_imm); - err |= __put_user(from->si_flags, &to->si_flags); - err |= __put_user(from->si_isr, &to->si_isr); -#endif - /* - * Other callers might not initialize the si_lsb field, - * so check explicitly for the right codes here. - */ -#ifdef BUS_MCEERR_AR - if (from->si_signo == SIGBUS && from->si_code == BUS_MCEERR_AR) - err |= __put_user(from->si_addr_lsb, &to->si_addr_lsb); -#endif -#ifdef BUS_MCEERR_AO - if (from->si_signo == SIGBUS && from->si_code == BUS_MCEERR_AO) - err |= __put_user(from->si_addr_lsb, &to->si_addr_lsb); -#endif -#ifdef SEGV_BNDERR - if (from->si_signo == SIGSEGV && from->si_code == SEGV_BNDERR) { - err |= __put_user(from->si_lower, &to->si_lower); - err |= __put_user(from->si_upper, &to->si_upper); - } -#endif -#ifdef SEGV_PKUERR - if (from->si_signo == SIGSEGV && from->si_code == SEGV_PKUERR) - err |= __put_user(from->si_pkey, &to->si_pkey); -#endif - break; - case SIL_CHLD: - err |= __put_user(from->si_pid, &to->si_pid); - err |= __put_user(from->si_uid, &to->si_uid); - err |= __put_user(from->si_status, &to->si_status); - err |= __put_user(from->si_utime, &to->si_utime); - err |= __put_user(from->si_stime, &to->si_stime); - break; - case SIL_RT: - err |= __put_user(from->si_pid, &to->si_pid); - err |= __put_user(from->si_uid, &to->si_uid); - err |= __put_user(from->si_ptr, &to->si_ptr); - break; - case SIL_SYS: - err |= __put_user(from->si_call_addr, &to->si_call_addr); - err |= __put_user(from->si_syscall, &to->si_syscall); - err |= __put_user(from->si_arch, &to->si_arch); - break; - } - return err; + return 0; } #ifdef CONFIG_COMPAT -- cgit v1.2.3-59-g8ed1b From 0c362f96e1c6bb76ab9b0b828985655fd2516bfa Mon Sep 17 00:00:00 2001 From: "Eric W. Biederman" Date: Sat, 14 Apr 2018 14:20:30 -0500 Subject: signal: Stop special casing TRAP_FIXME and FPE_FIXME in siginfo_layout After more experience with the cases where no one the si_code of 0 is used both as a signal specific si_code, and as SI_USER it appears that no one cares about the signal specific si_code case and the good solution is to just fix the architectures by using a different si_code. In none of the conversations has anyone even suggested that anything depends on the signal specific redefinition of SI_USER. There are at least test cases that care when si_code as 0 does not work as si_user. So make things simple and keep the generic code from introducing problems by removing the special casing of TRAP_FIXME and FPE_FIXME. This will ensure the generic case of sending a signal with kill will always set SI_USER and work. The architecture specific, and signal specific overloads that set si_code to 0 will now have problems with signalfd and the 32bit compat versions of siginfo copying. At least until they are fixed. Signed-off-by: "Eric W. Biederman" --- kernel/signal.c | 9 --------- 1 file changed, 9 deletions(-) (limited to 'kernel') diff --git a/kernel/signal.c b/kernel/signal.c index d56f4d496c89..fc82d2c0918f 100644 --- a/kernel/signal.c +++ b/kernel/signal.c @@ -2835,15 +2835,6 @@ enum siginfo_layout siginfo_layout(int sig, int si_code) layout = SIL_POLL; else if (si_code < 0) layout = SIL_RT; - /* Tests to support buggy kernel ABIs */ -#ifdef TRAP_FIXME - if ((sig == SIGTRAP) && (si_code == TRAP_FIXME)) - layout = SIL_FAULT; -#endif -#ifdef FPE_FIXME - if ((sig == SIGFPE) && (si_code == FPE_FIXME)) - layout = SIL_FAULT; -#endif } return layout; } -- cgit v1.2.3-59-g8ed1b From 3a11ab148ac21747f00449d2b8b389c24475e10a Mon Sep 17 00:00:00 2001 From: "Eric W. Biederman" Date: Sun, 15 Apr 2018 19:36:16 -0500 Subject: signal: Remove SEGV_BNDERR ifdefs After the last round of cleanups to siginfo.h SEGV_BNDERR is defined on all architectures so testing to see if it is defined is unnecessary. Signed-off-by: "Eric W. Biederman" --- kernel/signal.c | 6 ------ 1 file changed, 6 deletions(-) (limited to 'kernel') diff --git a/kernel/signal.c b/kernel/signal.c index fc82d2c0918f..a6d55a6e9915 100644 --- a/kernel/signal.c +++ b/kernel/signal.c @@ -1570,7 +1570,6 @@ int send_sig_mceerr(int code, void __user *addr, short lsb, struct task_struct * EXPORT_SYMBOL(send_sig_mceerr); #endif -#ifdef SEGV_BNDERR int force_sig_bnderr(void __user *addr, void __user *lower, void __user *upper) { struct siginfo info; @@ -1584,7 +1583,6 @@ int force_sig_bnderr(void __user *addr, void __user *lower, void __user *upper) info.si_upper = upper; return force_sig_info(info.si_signo, &info, current); } -#endif #ifdef SEGV_PKUERR int force_sig_pkuerr(void __user *addr, u32 pkey) @@ -2890,13 +2888,11 @@ int __copy_siginfo_to_user32(struct compat_siginfo __user *to, if ((from->si_signo == SIGBUS) && (from->si_code == BUS_MCEERR_AO)) new.si_addr_lsb = from->si_addr_lsb; #endif -#ifdef SEGV_BNDERR if ((from->si_signo == SIGSEGV) && (from->si_code == SEGV_BNDERR)) { new.si_lower = ptr_to_compat(from->si_lower); new.si_upper = ptr_to_compat(from->si_upper); } -#endif #ifdef SEGV_PKUERR if ((from->si_signo == SIGSEGV) && (from->si_code == SEGV_PKUERR)) @@ -2976,12 +2972,10 @@ int copy_siginfo_from_user32(struct siginfo *to, if ((from.si_signo == SIGBUS) && (from.si_code == BUS_MCEERR_AO)) to->si_addr_lsb = from.si_addr_lsb; #endif -#ifdef SEGV_BNDERR if ((from.si_signo == SIGSEGV) && (from.si_code == SEGV_BNDERR)) { to->si_lower = compat_ptr(from.si_lower); to->si_upper = compat_ptr(from.si_upper); } -#endif #ifdef SEGV_PKUERR if ((from.si_signo == SIGSEGV) && (from.si_code == SEGV_PKUERR)) to->si_pkey = from.si_pkey; -- cgit v1.2.3-59-g8ed1b From 4181d22596f61d060139bb114724f89b3ad28c8d Mon Sep 17 00:00:00 2001 From: "Eric W. Biederman" Date: Sun, 15 Apr 2018 19:40:02 -0500 Subject: signal: Remove ifdefs for BUS_MCEERR_AR and BUS_MCEERR_AO With the recent architecture cleanups these si_codes are always defined so there is no need to test for them. Signed-off-by: "Eric W. Biederman" --- fs/signalfd.c | 15 ++------------- kernel/signal.c | 24 ++++++++---------------- 2 files changed, 10 insertions(+), 29 deletions(-) (limited to 'kernel') diff --git a/fs/signalfd.c b/fs/signalfd.c index d2187a813376..ff302bf50be4 100644 --- a/fs/signalfd.c +++ b/fs/signalfd.c @@ -117,26 +117,15 @@ static int signalfd_copyinfo(struct signalfd_siginfo __user *uinfo, #ifdef __ARCH_SI_TRAPNO err |= __put_user(kinfo->si_trapno, &uinfo->ssi_trapno); #endif -#ifdef BUS_MCEERR_AO /* * Other callers might not initialize the si_lsb field, * so check explicitly for the right codes here. */ if (kinfo->si_signo == SIGBUS && - kinfo->si_code == BUS_MCEERR_AO) + ((kinfo->si_code == BUS_MCEERR_AR) || + (kinfo->si_code == BUS_MCEERR_AO))) err |= __put_user((short) kinfo->si_addr_lsb, &uinfo->ssi_addr_lsb); -#endif -#ifdef BUS_MCEERR_AR - /* - * Other callers might not initialize the si_lsb field, - * so check explicitly for the right codes here. - */ - if (kinfo->si_signo == SIGBUS && - kinfo->si_code == BUS_MCEERR_AR) - err |= __put_user((short) kinfo->si_addr_lsb, - &uinfo->ssi_addr_lsb); -#endif break; case SIL_CHLD: err |= __put_user(kinfo->si_pid, &uinfo->ssi_pid); diff --git a/kernel/signal.c b/kernel/signal.c index a6d55a6e9915..b87a9c21f698 100644 --- a/kernel/signal.c +++ b/kernel/signal.c @@ -1539,7 +1539,6 @@ int send_sig_fault(int sig, int code, void __user *addr return send_sig_info(info.si_signo, &info, t); } -#if defined(BUS_MCEERR_AO) && defined(BUS_MCEERR_AR) int force_sig_mceerr(int code, void __user *addr, short lsb, struct task_struct *t) { struct siginfo info; @@ -1568,7 +1567,6 @@ int send_sig_mceerr(int code, void __user *addr, short lsb, struct task_struct * return send_sig_info(info.si_signo, &info, t); } EXPORT_SYMBOL(send_sig_mceerr); -#endif int force_sig_bnderr(void __user *addr, void __user *lower, void __user *upper) { @@ -2880,14 +2878,11 @@ int __copy_siginfo_to_user32(struct compat_siginfo __user *to, #ifdef __ARCH_SI_TRAPNO new.si_trapno = from->si_trapno; #endif -#ifdef BUS_MCEERR_AR - if ((from->si_signo == SIGBUS) && (from->si_code == BUS_MCEERR_AR)) - new.si_addr_lsb = from->si_addr_lsb; -#endif -#ifdef BUS_MCEERR_AO - if ((from->si_signo == SIGBUS) && (from->si_code == BUS_MCEERR_AO)) + if ((from->si_signo == SIGBUS) && + ((from->si_code == BUS_MCEERR_AR) || + (from->si_code == BUS_MCEERR_AO))) new.si_addr_lsb = from->si_addr_lsb; -#endif + if ((from->si_signo == SIGSEGV) && (from->si_code == SEGV_BNDERR)) { new.si_lower = ptr_to_compat(from->si_lower); @@ -2964,14 +2959,11 @@ int copy_siginfo_from_user32(struct siginfo *to, #ifdef __ARCH_SI_TRAPNO to->si_trapno = from.si_trapno; #endif -#ifdef BUS_MCEERR_AR - if ((from.si_signo == SIGBUS) && (from.si_code == BUS_MCEERR_AR)) + if ((from.si_signo == SIGBUS) && + ((from.si_code == BUS_MCEERR_AR) || + (from.si_code == BUS_MCEERR_AO))) to->si_addr_lsb = from.si_addr_lsb; -#endif -#ifdef BUS_MCEER_AO - if ((from.si_signo == SIGBUS) && (from.si_code == BUS_MCEERR_AO)) - to->si_addr_lsb = from.si_addr_lsb; -#endif + if ((from.si_signo == SIGSEGV) && (from.si_code == SEGV_BNDERR)) { to->si_lower = compat_ptr(from.si_lower); to->si_upper = compat_ptr(from.si_upper); -- cgit v1.2.3-59-g8ed1b From 36a4ca3d9b5205819e4c47686cafb4e9b7ae76d3 Mon Sep 17 00:00:00 2001 From: "Eric W. Biederman" Date: Tue, 24 Apr 2018 21:06:43 -0500 Subject: signal: Remove unncessary #ifdef SEGV_PKUERR in 32bit compat code The only architecture that does not support SEGV_PKUERR is ia64 and ia64 has not had 32bit support since some time in 2008. Therefore copy_siginfo_to_user32 and copy_siginfo_from_user32 do not need to include support for a missing SEGV_PKUERR. Compile test on ia64. Signed-off-by: "Eric W. Biederman" --- kernel/signal.c | 5 ----- 1 file changed, 5 deletions(-) (limited to 'kernel') diff --git a/kernel/signal.c b/kernel/signal.c index b87a9c21f698..376b42f26e6d 100644 --- a/kernel/signal.c +++ b/kernel/signal.c @@ -2888,12 +2888,9 @@ int __copy_siginfo_to_user32(struct compat_siginfo __user *to, new.si_lower = ptr_to_compat(from->si_lower); new.si_upper = ptr_to_compat(from->si_upper); } -#ifdef SEGV_PKUERR if ((from->si_signo == SIGSEGV) && (from->si_code == SEGV_PKUERR)) new.si_pkey = from->si_pkey; -#endif - break; case SIL_CHLD: new.si_pid = from->si_pid; @@ -2968,10 +2965,8 @@ int copy_siginfo_from_user32(struct siginfo *to, to->si_lower = compat_ptr(from.si_lower); to->si_upper = compat_ptr(from.si_upper); } -#ifdef SEGV_PKUERR if ((from.si_signo == SIGSEGV) && (from.si_code == SEGV_PKUERR)) to->si_pkey = from.si_pkey; -#endif break; case SIL_CHLD: to->si_pid = from.si_pid; -- cgit v1.2.3-59-g8ed1b From 31931c93dfe05a76385a443ed28244a50e915a46 Mon Sep 17 00:00:00 2001 From: "Eric W. Biederman" Date: Tue, 24 Apr 2018 20:59:47 -0500 Subject: signal: Extend siginfo_layout with SIL_FAULT_{MCEERR|BNDERR|PKUERR} Update the siginfo_layout function and enum siginfo_layout to represent all of the possible field layouts of struct siginfo. This allows the uses of siginfo_layout in um and arm64 where they are testing for SIL_FAULT to be more accurate as this rules out the other cases. Further this allows the switch statements on siginfo_layout to be simpler if perhaps a little more wordy. Making it easier to understand what is actually going on. As SIL_FAULT_BNDERR and SIL_FAULT_PKUERR are never expected to appear in signalfd just treat them as SIL_FAULT. To include them would take 20 extra bytes an pretty much fill up what is left of signalfd_siginfo. Signed-off-by: "Eric W. Biederman" --- fs/signalfd.c | 24 ++++++++++----- include/linux/signal.h | 3 ++ kernel/signal.c | 81 ++++++++++++++++++++++++++++++++++---------------- 3 files changed, 75 insertions(+), 33 deletions(-) (limited to 'kernel') diff --git a/fs/signalfd.c b/fs/signalfd.c index f652249f59f9..cbb42f77a2bd 100644 --- a/fs/signalfd.c +++ b/fs/signalfd.c @@ -112,19 +112,27 @@ static int signalfd_copyinfo(struct signalfd_siginfo __user *uinfo, new.ssi_band = kinfo->si_band; new.ssi_fd = kinfo->si_fd; break; + case SIL_FAULT_BNDERR: + case SIL_FAULT_PKUERR: + /* + * Fall through to the SIL_FAULT case. Both SIL_FAULT_BNDERR + * and SIL_FAULT_PKUERR are only generated by faults that + * deliver them synchronously to userspace. In case someone + * injects one of these signals and signalfd catches it treat + * it as SIL_FAULT. + */ case SIL_FAULT: new.ssi_addr = (long) kinfo->si_addr; #ifdef __ARCH_SI_TRAPNO new.ssi_trapno = kinfo->si_trapno; #endif - /* - * Other callers might not initialize the si_lsb field, - * so check explicitly for the right codes here. - */ - if (kinfo->si_signo == SIGBUS && - ((kinfo->si_code == BUS_MCEERR_AR) || - (kinfo->si_code == BUS_MCEERR_AO))) - new.ssi_addr_lsb = (short) kinfo->si_addr_lsb; + break; + case SIL_FAULT_MCEERR: + new.ssi_addr = (long) kinfo->si_addr; +#ifdef __ARCH_SI_TRAPNO + new.ssi_trapno = kinfo->si_trapno; +#endif + new.ssi_addr_lsb = (short) kinfo->si_addr_lsb; break; case SIL_CHLD: new.ssi_pid = kinfo->si_pid; diff --git a/include/linux/signal.h b/include/linux/signal.h index a9bc7e1b077e..3c5200137b24 100644 --- a/include/linux/signal.h +++ b/include/linux/signal.h @@ -28,6 +28,9 @@ enum siginfo_layout { SIL_TIMER, SIL_POLL, SIL_FAULT, + SIL_FAULT_MCEERR, + SIL_FAULT_BNDERR, + SIL_FAULT_PKUERR, SIL_CHLD, SIL_RT, SIL_SYS, diff --git a/kernel/signal.c b/kernel/signal.c index 376b42f26e6d..8a85da8aaa7c 100644 --- a/kernel/signal.c +++ b/kernel/signal.c @@ -2820,8 +2820,19 @@ enum siginfo_layout siginfo_layout(int sig, int si_code) [SIGPOLL] = { NSIGPOLL, SIL_POLL }, [SIGSYS] = { NSIGSYS, SIL_SYS }, }; - if ((sig < ARRAY_SIZE(filter)) && (si_code <= filter[sig].limit)) + if ((sig < ARRAY_SIZE(filter)) && (si_code <= filter[sig].limit)) { layout = filter[sig].layout; + /* Handle the exceptions */ + if ((sig == SIGBUS) && + (si_code >= BUS_MCEERR_AR) && (si_code <= BUS_MCEERR_AO)) + layout = SIL_FAULT_MCEERR; + else if ((sig == SIGSEGV) && (si_code == SEGV_BNDERR)) + layout = SIL_FAULT_BNDERR; +#ifdef SEGV_PKUERR + else if ((sig == SIGSEGV) && (si_code == SEGV_PKUERR)) + layout = SIL_FAULT_PKUERR; +#endif + } else if (si_code <= NSIGPOLL) layout = SIL_POLL; } else { @@ -2878,19 +2889,28 @@ int __copy_siginfo_to_user32(struct compat_siginfo __user *to, #ifdef __ARCH_SI_TRAPNO new.si_trapno = from->si_trapno; #endif - if ((from->si_signo == SIGBUS) && - ((from->si_code == BUS_MCEERR_AR) || - (from->si_code == BUS_MCEERR_AO))) - new.si_addr_lsb = from->si_addr_lsb; - - if ((from->si_signo == SIGSEGV) && - (from->si_code == SEGV_BNDERR)) { - new.si_lower = ptr_to_compat(from->si_lower); - new.si_upper = ptr_to_compat(from->si_upper); - } - if ((from->si_signo == SIGSEGV) && - (from->si_code == SEGV_PKUERR)) - new.si_pkey = from->si_pkey; + break; + case SIL_FAULT_MCEERR: + new.si_addr = ptr_to_compat(from->si_addr); +#ifdef __ARCH_SI_TRAPNO + new.si_trapno = from->si_trapno; +#endif + new.si_addr_lsb = from->si_addr_lsb; + break; + case SIL_FAULT_BNDERR: + new.si_addr = ptr_to_compat(from->si_addr); +#ifdef __ARCH_SI_TRAPNO + new.si_trapno = from->si_trapno; +#endif + new.si_lower = ptr_to_compat(from->si_lower); + new.si_upper = ptr_to_compat(from->si_upper); + break; + case SIL_FAULT_PKUERR: + new.si_addr = ptr_to_compat(from->si_addr); +#ifdef __ARCH_SI_TRAPNO + new.si_trapno = from->si_trapno; +#endif + new.si_pkey = from->si_pkey; break; case SIL_CHLD: new.si_pid = from->si_pid; @@ -2956,17 +2976,28 @@ int copy_siginfo_from_user32(struct siginfo *to, #ifdef __ARCH_SI_TRAPNO to->si_trapno = from.si_trapno; #endif - if ((from.si_signo == SIGBUS) && - ((from.si_code == BUS_MCEERR_AR) || - (from.si_code == BUS_MCEERR_AO))) - to->si_addr_lsb = from.si_addr_lsb; - - if ((from.si_signo == SIGSEGV) && (from.si_code == SEGV_BNDERR)) { - to->si_lower = compat_ptr(from.si_lower); - to->si_upper = compat_ptr(from.si_upper); - } - if ((from.si_signo == SIGSEGV) && (from.si_code == SEGV_PKUERR)) - to->si_pkey = from.si_pkey; + break; + case SIL_FAULT_MCEERR: + to->si_addr = compat_ptr(from.si_addr); +#ifdef __ARCH_SI_TRAPNO + to->si_trapno = from.si_trapno; +#endif + to->si_addr_lsb = from.si_addr_lsb; + break; + case SIL_FAULT_BNDERR: + to->si_addr = compat_ptr(from.si_addr); +#ifdef __ARCH_SI_TRAPNO + to->si_trapno = from.si_trapno; +#endif + to->si_lower = compat_ptr(from.si_lower); + to->si_upper = compat_ptr(from.si_upper); + break; + case SIL_FAULT_PKUERR: + to->si_addr = compat_ptr(from.si_addr); +#ifdef __ARCH_SI_TRAPNO + to->si_trapno = from.si_trapno; +#endif + to->si_pkey = from.si_pkey; break; case SIL_CHLD: to->si_pid = from.si_pid; -- cgit v1.2.3-59-g8ed1b