* [PATCH 00/20] exit cleanups @ 2021-10-20 17:32 Eric W. Biederman 2021-10-20 17:43 ` [PATCH 02/20] exit: Remove calls of do_exit after noreturn versions of die Eric W. Biederman ` (2 more replies) 0 siblings, 3 replies; 12+ messages in thread From: Eric W. Biederman @ 2021-10-20 17:32 UTC (permalink / raw) To: linux-kernel Cc: linux-arch, Linus Torvalds, Oleg Nesterov, Al Viro, Kees Cook, Andy Lutomirski, Jonas Bonn, Stefan Kristiansson, Stafford Horne, openrisc, Nick Hu, Greentime Hu, Vincent Chen, Heiko Carstens, Vasily Gorbik, Christian Borntraeger, linux-s390, Yoshinori Sato, Rich Felker, linux-sh, linux-xtensa, Chris Zankel, Max Filippov, David Miller, sparclinux, Thomas Bogendoerfer, Maciej Rozycki, linux-mips, Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras, linuxppc-dev, Thomas Gleixner, Ingo Molnar, Borislav Petkov, H Peter Anvin, Greg Kroah-Hartman While looking at some issues related to the exit path in the kernel I found several instances where the code is not using the existing abstractions properly. This set of changes introduces force_fatal_sig a way of sending a signal and not allowing it to be caught, and corrects the misuse of the existing abstractions that I found. A lot of the misuse of the existing abstractions are silly things such as doing something after calling a no return function, rolling BUG by hand, doing more work than necessary to terminate a kernel thread, or calling do_exit(SIGKILL) instead of calling force_sig(SIGKILL). It is my plan after sending all of these changes out for review to place them in a topic branch for sending Linus. Especially for the changes that depend upon the new helper force_fatal_sig this is important. Eric W. Biederman (20): exit/doublefault: Remove apparently bogus comment about rewind_stack_do_exit exit: Remove calls of do_exit after noreturn versions of die reboot: Remove the unreachable panic after do_exit in reboot(2) signal/sparc32: Remove unreachable do_exit in do_sparc_fault signal/mips: Update (_save|_restore)_fp_context to fail with -EFAULT signal/sh: Use force_sig(SIGKILL) instead of do_group_exit(SIGKILL) signal/powerpc: On swapcontext failure force SIGSEGV signal/sparc: In setup_tsb_params convert open coded BUG into BUG signal/vm86_32: Replace open coded BUG_ON with an actual BUG_ON signal/vm86_32: Properly send SIGSEGV when the vm86 state cannot be saved. signal/s390: Use force_sigsegv in default_trap_handler exit/kthread: Have kernel threads return instead of calling do_exit signal: Implement force_fatal_sig exit/syscall_user_dispatch: Send ordinary signals on failure signal/sparc32: Exit with a fatal signal when try_to_clear_window_buffer fails signal/sparc32: In setup_rt_frame and setup_fram use force_fatal_sig signal/x86: In emulate_vsyscall force a signal instead of calling do_exit exit/rtl8723bs: Replace the macro thread_exit with a simple return 0 exit/rtl8712: Replace the macro thread_exit with a simple return 0 exit/r8188eu: Replace the macro thread_exit with a simple return 0 arch/mips/kernel/r2300_fpu.S | 4 ++-- arch/mips/kernel/syscall.c | 9 -------- arch/nds32/kernel/traps.c | 2 +- arch/nds32/mm/fault.c | 6 +---- arch/openrisc/kernel/traps.c | 2 +- arch/openrisc/mm/fault.c | 4 +--- arch/powerpc/kernel/signal_32.c | 6 +++-- arch/powerpc/kernel/signal_64.c | 9 +++++--- arch/s390/include/asm/kdebug.h | 2 +- arch/s390/kernel/dumpstack.c | 2 +- arch/s390/kernel/traps.c | 2 +- arch/s390/mm/fault.c | 2 -- arch/sh/kernel/cpu/fpu.c | 10 +++++---- arch/sh/kernel/traps.c | 2 +- arch/sh/mm/fault.c | 2 -- arch/sparc/kernel/signal_32.c | 4 ++-- arch/sparc/kernel/windows.c | 6 +++-- arch/sparc/mm/fault_32.c | 1 - arch/sparc/mm/tsb.c | 2 +- arch/x86/entry/vsyscall/vsyscall_64.c | 3 ++- arch/x86/kernel/doublefault_32.c | 3 --- arch/x86/kernel/signal.c | 6 ++++- arch/x86/kernel/vm86_32.c | 8 +++---- arch/xtensa/kernel/traps.c | 2 +- arch/xtensa/mm/fault.c | 3 +-- drivers/firmware/stratix10-svc.c | 4 ++-- drivers/soc/ti/wkup_m3_ipc.c | 2 +- drivers/staging/r8188eu/core/rtw_cmd.c | 2 +- drivers/staging/r8188eu/core/rtw_mp.c | 2 +- drivers/staging/r8188eu/include/osdep_service.h | 2 -- drivers/staging/rtl8712/osdep_service.h | 1 - drivers/staging/rtl8712/rtl8712_cmd.c | 2 +- drivers/staging/rtl8723bs/core/rtw_cmd.c | 2 +- drivers/staging/rtl8723bs/core/rtw_xmit.c | 2 +- drivers/staging/rtl8723bs/hal/rtl8723bs_xmit.c | 2 +- .../rtl8723bs/include/osdep_service_linux.h | 2 -- fs/ocfs2/journal.c | 5 +---- include/linux/sched/signal.h | 1 + kernel/entry/syscall_user_dispatch.c | 12 ++++++---- kernel/kthread.c | 2 +- kernel/reboot.c | 1 - kernel/signal.c | 26 ++++++++++++++-------- net/batman-adv/tp_meter.c | 2 +- 43 files changed, 83 insertions(+), 91 deletions(-) Eric ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 02/20] exit: Remove calls of do_exit after noreturn versions of die 2021-10-20 17:32 [PATCH 00/20] exit cleanups Eric W. Biederman @ 2021-10-20 17:43 ` Eric W. Biederman 2021-10-21 16:02 ` Kees Cook 2021-10-20 17:43 ` [PATCH 06/20] signal/sh: Use force_sig(SIGKILL) instead of do_group_exit(SIGKILL) Eric W. Biederman 2021-10-20 21:51 ` [PATCH 21/20] signal: Replace force_sigsegv(SIGSEGV) with force_fatal_sig(SIGSEGV) Eric W. Biederman 2 siblings, 1 reply; 12+ messages in thread From: Eric W. Biederman @ 2021-10-20 17:43 UTC (permalink / raw) To: linux-kernel Cc: linux-arch, Linus Torvalds, Oleg Nesterov, Al Viro, Kees Cook, Eric W. Biederman, Jonas Bonn, Stefan Kristiansson, Stafford Horne, openrisc, Nick Hu, Greentime Hu, Vincent Chen, Heiko Carstens, Vasily Gorbik, Christian Borntraeger, linux-s390, Yoshinori Sato, Rich Felker, linux-sh, linux-xtensa, Chris Zankel, Max Filippov On nds32, openrisc, s390, sh, and xtensa the function die never returns. Mark die __noreturn so that no one expects die to return. Remove the do_exit calls after die as they will never be reached. Cc: Jonas Bonn <jonas@southpole.se> Cc: Stefan Kristiansson <stefan.kristiansson@saunalahti.fi> Cc: Stafford Horne <shorne@gmail.com> Cc: openrisc@lists.librecores.org Cc: Nick Hu <nickhu@andestech.com> Cc: Greentime Hu <green.hu@gmail.com> Cc: Vincent Chen <deanbo422@gmail.com> Cc: Heiko Carstens <hca@linux.ibm.com> Cc: Vasily Gorbik <gor@linux.ibm.com> Cc: Christian Borntraeger <borntraeger@de.ibm.com> Cc: linux-s390@vger.kernel.org Cc: Yoshinori Sato <ysato@users.sourceforge.jp> Cc: Rich Felker <dalias@libc.org> Cc: linux-sh@vger.kernel.org Cc: linux-xtensa@linux-xtensa.org Cc: Chris Zankel <chris@zankel.net> Cc: Max Filippov <jcmvbkbc@gmail.com> Fixes: 2.3.16 Fixes: 2.3.99-pre8 Fixes: 3f65ce4d141e ("[PATCH] xtensa: Architecture support for Tensilica Xtensa Part 5") Fixes: 664eec400bf8 ("nds32: MMU fault handling and page table management") Fixes: 61e85e367535 ("OpenRISC: Memory management") Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com> --- arch/nds32/kernel/traps.c | 2 +- arch/nds32/mm/fault.c | 6 +----- arch/openrisc/kernel/traps.c | 2 +- arch/openrisc/mm/fault.c | 4 +--- arch/s390/include/asm/kdebug.h | 2 +- arch/s390/kernel/dumpstack.c | 2 +- arch/s390/mm/fault.c | 2 -- arch/sh/kernel/traps.c | 2 +- arch/sh/mm/fault.c | 2 -- arch/xtensa/kernel/traps.c | 2 +- arch/xtensa/mm/fault.c | 3 +-- 11 files changed, 9 insertions(+), 20 deletions(-) diff --git a/arch/nds32/kernel/traps.c b/arch/nds32/kernel/traps.c index f06421c645af..ca75d475eda4 100644 --- a/arch/nds32/kernel/traps.c +++ b/arch/nds32/kernel/traps.c @@ -118,7 +118,7 @@ DEFINE_SPINLOCK(die_lock); /* * This function is protected against re-entrancy. */ -void die(const char *str, struct pt_regs *regs, int err) +void __noreturn die(const char *str, struct pt_regs *regs, int err) { struct task_struct *tsk = current; static int die_counter; diff --git a/arch/nds32/mm/fault.c b/arch/nds32/mm/fault.c index f02524eb6d56..1d139b117168 100644 --- a/arch/nds32/mm/fault.c +++ b/arch/nds32/mm/fault.c @@ -13,7 +13,7 @@ #include <asm/tlbflush.h> -extern void die(const char *str, struct pt_regs *regs, long err); +extern void __noreturn die(const char *str, struct pt_regs *regs, long err); /* * This is useful to dump out the page tables associated with @@ -299,10 +299,6 @@ void do_page_fault(unsigned long entry, unsigned long addr, show_pte(mm, addr); die("Oops", regs, error_code); - bust_spinlocks(0); - do_exit(SIGKILL); - - return; /* * We ran out of memory, or some other thing happened to us that made diff --git a/arch/openrisc/kernel/traps.c b/arch/openrisc/kernel/traps.c index aa1e709405ac..0898cb159fac 100644 --- a/arch/openrisc/kernel/traps.c +++ b/arch/openrisc/kernel/traps.c @@ -197,7 +197,7 @@ void nommu_dump_state(struct pt_regs *regs, } /* This is normally the 'Oops' routine */ -void die(const char *str, struct pt_regs *regs, long err) +void __noreturn die(const char *str, struct pt_regs *regs, long err) { console_verbose(); diff --git a/arch/openrisc/mm/fault.c b/arch/openrisc/mm/fault.c index c730d1a51686..f0fa6394a58e 100644 --- a/arch/openrisc/mm/fault.c +++ b/arch/openrisc/mm/fault.c @@ -32,7 +32,7 @@ unsigned long pte_errors; /* updated by do_page_fault() */ */ volatile pgd_t *current_pgd[NR_CPUS]; -extern void die(char *, struct pt_regs *, long); +extern void __noreturn die(char *, struct pt_regs *, long); /* * This routine handles page faults. It determines the address, @@ -248,8 +248,6 @@ asmlinkage void do_page_fault(struct pt_regs *regs, unsigned long address, die("Oops", regs, write_acc); - do_exit(SIGKILL); - /* * We ran out of memory, or some other thing happened to us that made * us unable to handle the page fault gracefully. diff --git a/arch/s390/include/asm/kdebug.h b/arch/s390/include/asm/kdebug.h index d5327f064799..4377238e4752 100644 --- a/arch/s390/include/asm/kdebug.h +++ b/arch/s390/include/asm/kdebug.h @@ -23,6 +23,6 @@ enum die_val { DIE_NMI_IPI, }; -extern void die(struct pt_regs *, const char *); +extern void __noreturn die(struct pt_regs *, const char *); #endif diff --git a/arch/s390/kernel/dumpstack.c b/arch/s390/kernel/dumpstack.c index db1bc00229ca..f45e66b8bed6 100644 --- a/arch/s390/kernel/dumpstack.c +++ b/arch/s390/kernel/dumpstack.c @@ -192,7 +192,7 @@ void show_regs(struct pt_regs *regs) static DEFINE_SPINLOCK(die_lock); -void die(struct pt_regs *regs, const char *str) +void __noreturn die(struct pt_regs *regs, const char *str) { static int die_counter; diff --git a/arch/s390/mm/fault.c b/arch/s390/mm/fault.c index 212632d57db9..d30f5986fa85 100644 --- a/arch/s390/mm/fault.c +++ b/arch/s390/mm/fault.c @@ -260,7 +260,6 @@ static noinline void do_no_context(struct pt_regs *regs) " in virtual user address space\n"); dump_fault_info(regs); die(regs, "Oops"); - do_exit(SIGKILL); } static noinline void do_low_address(struct pt_regs *regs) @@ -270,7 +269,6 @@ static noinline void do_low_address(struct pt_regs *regs) if (regs->psw.mask & PSW_MASK_PSTATE) { /* Low-address protection hit in user mode 'cannot happen'. */ die (regs, "Low-address protection"); - do_exit(SIGKILL); } do_no_context(regs); diff --git a/arch/sh/kernel/traps.c b/arch/sh/kernel/traps.c index e76b22157099..cbe3201d4f21 100644 --- a/arch/sh/kernel/traps.c +++ b/arch/sh/kernel/traps.c @@ -20,7 +20,7 @@ static DEFINE_SPINLOCK(die_lock); -void die(const char *str, struct pt_regs *regs, long err) +void __noreturn die(const char *str, struct pt_regs *regs, long err) { static int die_counter; diff --git a/arch/sh/mm/fault.c b/arch/sh/mm/fault.c index 88a1f453d73e..1e1aa75df3ca 100644 --- a/arch/sh/mm/fault.c +++ b/arch/sh/mm/fault.c @@ -238,8 +238,6 @@ no_context(struct pt_regs *regs, unsigned long error_code, show_fault_oops(regs, address); die("Oops", regs, error_code); - bust_spinlocks(0); - do_exit(SIGKILL); } static void diff --git a/arch/xtensa/kernel/traps.c b/arch/xtensa/kernel/traps.c index 874b6efc6fb3..fb056a191339 100644 --- a/arch/xtensa/kernel/traps.c +++ b/arch/xtensa/kernel/traps.c @@ -527,7 +527,7 @@ void show_stack(struct task_struct *task, unsigned long *sp, const char *loglvl) DEFINE_SPINLOCK(die_lock); -void die(const char * str, struct pt_regs * regs, long err) +void __noreturn die(const char * str, struct pt_regs * regs, long err) { static int die_counter; const char *pr = ""; diff --git a/arch/xtensa/mm/fault.c b/arch/xtensa/mm/fault.c index 95a74890c7e9..fd6a70635962 100644 --- a/arch/xtensa/mm/fault.c +++ b/arch/xtensa/mm/fault.c @@ -238,7 +238,7 @@ void do_page_fault(struct pt_regs *regs) void bad_page_fault(struct pt_regs *regs, unsigned long address, int sig) { - extern void die(const char*, struct pt_regs*, long); + extern void __noreturn die(const char*, struct pt_regs*, long); const struct exception_table_entry *entry; /* Are we prepared to handle this kernel fault? */ @@ -257,5 +257,4 @@ bad_page_fault(struct pt_regs *regs, unsigned long address, int sig) "address %08lx\n pc = %08lx, ra = %08lx\n", address, regs->pc, regs->areg[0]); die("Oops", regs, sig); - do_exit(sig); } -- 2.20.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 02/20] exit: Remove calls of do_exit after noreturn versions of die 2021-10-20 17:43 ` [PATCH 02/20] exit: Remove calls of do_exit after noreturn versions of die Eric W. Biederman @ 2021-10-21 16:02 ` Kees Cook 2021-10-21 16:25 ` Eric W. Biederman 0 siblings, 1 reply; 12+ messages in thread From: Kees Cook @ 2021-10-21 16:02 UTC (permalink / raw) To: Eric W. Biederman Cc: linux-kernel, linux-arch, Linus Torvalds, Oleg Nesterov, Al Viro, Jonas Bonn, Stefan Kristiansson, Stafford Horne, openrisc, Nick Hu, Greentime Hu, Vincent Chen, Heiko Carstens, Vasily Gorbik, Christian Borntraeger, linux-s390, Yoshinori Sato, Rich Felker, linux-sh, linux-xtensa, Chris Zankel, Max Filippov On Wed, Oct 20, 2021 at 12:43:48PM -0500, Eric W. Biederman wrote: > On nds32, openrisc, s390, sh, and xtensa the function die never > returns. Mark die __noreturn so that no one expects die to return. > Remove the do_exit calls after die as they will never be reached. Maybe note that the "bust_spinlocks" calls are also redundant, since they're in die(). I note that is a "mismatch" between the do_kill() in die() (SIGSEGV) and after die() (SIGKILL). This patch makes no behavioral change (the first caller would "win"), but I thought I'd note it in case some architecture would prefer a different signal. Reviewed-by: Kees Cook <keescook@chromium.org> -Kees -- Kees Cook ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 02/20] exit: Remove calls of do_exit after noreturn versions of die 2021-10-21 16:02 ` Kees Cook @ 2021-10-21 16:25 ` Eric W. Biederman 0 siblings, 0 replies; 12+ messages in thread From: Eric W. Biederman @ 2021-10-21 16:25 UTC (permalink / raw) To: Kees Cook Cc: linux-kernel, linux-arch, Linus Torvalds, Oleg Nesterov, Al Viro, Jonas Bonn, Stefan Kristiansson, Stafford Horne, openrisc, Nick Hu, Greentime Hu, Vincent Chen, Heiko Carstens, Vasily Gorbik, Christian Borntraeger, linux-s390, Yoshinori Sato, Rich Felker, linux-sh, linux-xtensa, Chris Zankel, Max Filippov Kees Cook <keescook@chromium.org> writes: > On Wed, Oct 20, 2021 at 12:43:48PM -0500, Eric W. Biederman wrote: >> On nds32, openrisc, s390, sh, and xtensa the function die never >> returns. Mark die __noreturn so that no one expects die to return. >> Remove the do_exit calls after die as they will never be reached. > > Maybe note that the "bust_spinlocks" calls are also redundant, since > they're in die(). I note that is a "mismatch" between the do_kill() > in die() (SIGSEGV) and after die() (SIGKILL). This patch makes no > behavioral change (the first caller would "win"), but I thought I'd note > it in case some architecture would prefer a different signal. If someone has some strong preferences in the matter of which signal a wait on a processes that has oopsed should return please let me know. My next step in cleaning up the uses of do_exit looks like it is going to be getting all of the architectures to use the same signal for oopses (aka die), and then introducing a helper (called something like "make_task_dead" or "oops_task_exit" ) that will replace do_exit on the oops path and not take a signal number at all. That helper I can then remove the ptrace break point from and possibly some of the coredump logic as well. Ultimately it will be something we can optimize for the case when we know there is a kernel bug and we just want the task to exit so the rest of the system can limp along as best as it can. Eric ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 06/20] signal/sh: Use force_sig(SIGKILL) instead of do_group_exit(SIGKILL) 2021-10-20 17:32 [PATCH 00/20] exit cleanups Eric W. Biederman 2021-10-20 17:43 ` [PATCH 02/20] exit: Remove calls of do_exit after noreturn versions of die Eric W. Biederman @ 2021-10-20 17:43 ` Eric W. Biederman 2021-10-20 19:57 ` Linus Torvalds 2021-10-21 16:08 ` Kees Cook 2021-10-20 21:51 ` [PATCH 21/20] signal: Replace force_sigsegv(SIGSEGV) with force_fatal_sig(SIGSEGV) Eric W. Biederman 2 siblings, 2 replies; 12+ messages in thread From: Eric W. Biederman @ 2021-10-20 17:43 UTC (permalink / raw) To: linux-kernel Cc: linux-arch, Linus Torvalds, Oleg Nesterov, Al Viro, Kees Cook, Eric W. Biederman, Yoshinori Sato, Rich Felker, linux-sh Today the sh code allocates memory the first time a process uses the fpu. If that memory allocation fails, kill the affected task with force_sig(SIGKILL) rather than do_group_exit(SIGKILL). Calling do_group_exit from an exception handler can potentially lead to dead locks as do_group_exit is not designed to be called from interrupt context. Instead use force_sig(SIGKILL) to kill the userspace process. Sending signals in general and force_sig in particular has been tested from interrupt context so there should be no problems. Cc: Yoshinori Sato <ysato@users.sourceforge.jp> Cc: Rich Felker <dalias@libc.org> Cc: linux-sh@vger.kernel.org Fixes: 0ea820cf9bf5 ("sh: Move over to dynamically allocated FPU context.") Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com> --- arch/sh/kernel/cpu/fpu.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/arch/sh/kernel/cpu/fpu.c b/arch/sh/kernel/cpu/fpu.c index ae354a2931e7..fd6db0ab1928 100644 --- a/arch/sh/kernel/cpu/fpu.c +++ b/arch/sh/kernel/cpu/fpu.c @@ -62,18 +62,20 @@ void fpu_state_restore(struct pt_regs *regs) } if (!tsk_used_math(tsk)) { - local_irq_enable(); + int ret; /* * does a slab alloc which can sleep */ - if (init_fpu(tsk)) { + local_irq_enable(); + ret = init_fpu(tsk); + local_irq_disable(); + if (ret) { /* * ran out of memory! */ - do_group_exit(SIGKILL); + force_sig(SIGKILL); return; } - local_irq_disable(); } grab_fpu(regs); -- 2.20.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 06/20] signal/sh: Use force_sig(SIGKILL) instead of do_group_exit(SIGKILL) 2021-10-20 17:43 ` [PATCH 06/20] signal/sh: Use force_sig(SIGKILL) instead of do_group_exit(SIGKILL) Eric W. Biederman @ 2021-10-20 19:57 ` Linus Torvalds 2021-10-27 14:24 ` Rich Felker 2021-10-21 16:08 ` Kees Cook 1 sibling, 1 reply; 12+ messages in thread From: Linus Torvalds @ 2021-10-20 19:57 UTC (permalink / raw) To: Eric W. Biederman Cc: Linux Kernel Mailing List, linux-arch, Oleg Nesterov, Al Viro, Kees Cook, Yoshinori Sato, Rich Felker, Linux-sh list On Wed, Oct 20, 2021 at 7:44 AM Eric W. Biederman <ebiederm@xmission.com> wrote: > > + force_sig(SIGKILL); I wonder if SIGFPE would be a more intuitive thing. Doesn't really matter, this is a "doesn't happen" event anyway, but that was just my reaction to reading the patch. Linus ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 06/20] signal/sh: Use force_sig(SIGKILL) instead of do_group_exit(SIGKILL) 2021-10-20 19:57 ` Linus Torvalds @ 2021-10-27 14:24 ` Rich Felker 0 siblings, 0 replies; 12+ messages in thread From: Rich Felker @ 2021-10-27 14:24 UTC (permalink / raw) To: Linus Torvalds Cc: Eric W. Biederman, Linux Kernel Mailing List, linux-arch, Oleg Nesterov, Al Viro, Kees Cook, Yoshinori Sato, Linux-sh list On Wed, Oct 20, 2021 at 09:57:58AM -1000, Linus Torvalds wrote: > On Wed, Oct 20, 2021 at 7:44 AM Eric W. Biederman <ebiederm@xmission.com> wrote: > > > > + force_sig(SIGKILL); > > I wonder if SIGFPE would be a more intuitive thing. > > Doesn't really matter, this is a "doesn't happen" event anyway, but > that was just my reaction to reading the patch. I think SIGKILL makes more sense unless there's a way the process could handle the resulting SIGFPE and recover. I'd actually like to see the lazy allocation of FPU state just removed (the amount of space saved is tiny relative to the complexity cost and the negative aspects of unrecoverable late failure) but for now let's just go with this. Rich ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 06/20] signal/sh: Use force_sig(SIGKILL) instead of do_group_exit(SIGKILL) 2021-10-20 17:43 ` [PATCH 06/20] signal/sh: Use force_sig(SIGKILL) instead of do_group_exit(SIGKILL) Eric W. Biederman 2021-10-20 19:57 ` Linus Torvalds @ 2021-10-21 16:08 ` Kees Cook 1 sibling, 0 replies; 12+ messages in thread From: Kees Cook @ 2021-10-21 16:08 UTC (permalink / raw) To: Eric W. Biederman Cc: linux-kernel, linux-arch, Linus Torvalds, Oleg Nesterov, Al Viro, Yoshinori Sato, Rich Felker, linux-sh On Wed, Oct 20, 2021 at 12:43:52PM -0500, Eric W. Biederman wrote: > Today the sh code allocates memory the first time a process uses > the fpu. If that memory allocation fails, kill the affected task > with force_sig(SIGKILL) rather than do_group_exit(SIGKILL). > > Calling do_group_exit from an exception handler can potentially lead > to dead locks as do_group_exit is not designed to be called from > interrupt context. Instead use force_sig(SIGKILL) to kill the > userspace process. Sending signals in general and force_sig in > particular has been tested from interrupt context so there should be > no problems. > > Cc: Yoshinori Sato <ysato@users.sourceforge.jp> > Cc: Rich Felker <dalias@libc.org> > Cc: linux-sh@vger.kernel.org > Fixes: 0ea820cf9bf5 ("sh: Move over to dynamically allocated FPU context.") > Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com> Looks sane; there should be no observable changes. Reviewed-by: Kees Cook <keescook@chromium.org> -- Kees Cook ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 21/20] signal: Replace force_sigsegv(SIGSEGV) with force_fatal_sig(SIGSEGV) 2021-10-20 17:32 [PATCH 00/20] exit cleanups Eric W. Biederman 2021-10-20 17:43 ` [PATCH 02/20] exit: Remove calls of do_exit after noreturn versions of die Eric W. Biederman 2021-10-20 17:43 ` [PATCH 06/20] signal/sh: Use force_sig(SIGKILL) instead of do_group_exit(SIGKILL) Eric W. Biederman @ 2021-10-20 21:51 ` Eric W. Biederman 2021-10-21 8:09 ` Geert Uytterhoeven 2021-10-21 8:32 ` Philippe Mathieu-Daudé 2 siblings, 2 replies; 12+ messages in thread From: Eric W. Biederman @ 2021-10-20 21:51 UTC (permalink / raw) To: linux-kernel Cc: linux-arch, Linus Torvalds, Oleg Nesterov, Al Viro, Kees Cook, Andy Lutomirski, Jonas Bonn, Stefan Kristiansson, Stafford Horne, openrisc, Nick Hu, Greentime Hu, Vincent Chen, Heiko Carstens, Vasily Gorbik, Christian Borntraeger, linux-s390, Yoshinori Sato, Rich Felker, linux-sh, linux-xtensa, Chris Zankel, Max Filippov, David Miller, sparclinux, Thomas Bogendoerfer, Maciej Rozycki, linux-mips, Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras, linuxppc-dev, Thomas Gleixner, Ingo Molnar, Borislav Petkov, H Peter Anvin, Greg Kroah-Hartman Now that force_fatal_sig exists it is unnecessary and a bit confusing to use force_sigsegv in cases where the simpler force_fatal_sig is wanted. So change every instance we can to make the code clearer. Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com> --- arch/arc/kernel/process.c | 2 +- arch/m68k/kernel/traps.c | 2 +- arch/powerpc/kernel/signal_32.c | 2 +- arch/powerpc/kernel/signal_64.c | 4 ++-- arch/s390/kernel/traps.c | 2 +- arch/um/kernel/trap.c | 2 +- arch/x86/kernel/vm86_32.c | 2 +- fs/exec.c | 2 +- 8 files changed, 9 insertions(+), 9 deletions(-) diff --git a/arch/arc/kernel/process.c b/arch/arc/kernel/process.c index 3793876f42d9..8e90052f6f05 100644 --- a/arch/arc/kernel/process.c +++ b/arch/arc/kernel/process.c @@ -294,7 +294,7 @@ int elf_check_arch(const struct elf32_hdr *x) eflags = x->e_flags; if ((eflags & EF_ARC_OSABI_MSK) != EF_ARC_OSABI_CURRENT) { pr_err("ABI mismatch - you need newer toolchain\n"); - force_sigsegv(SIGSEGV); + force_fatal_sig(SIGSEGV); return 0; } diff --git a/arch/m68k/kernel/traps.c b/arch/m68k/kernel/traps.c index 5b19fcdcd69e..74045d164ddb 100644 --- a/arch/m68k/kernel/traps.c +++ b/arch/m68k/kernel/traps.c @@ -1150,7 +1150,7 @@ asmlinkage void set_esp0(unsigned long ssp) */ asmlinkage void fpsp040_die(void) { - force_sigsegv(SIGSEGV); + force_fatal_sig(SIGSEGV); } #ifdef CONFIG_M68KFPU_EMU diff --git a/arch/powerpc/kernel/signal_32.c b/arch/powerpc/kernel/signal_32.c index 666f3da41232..933ab95805a6 100644 --- a/arch/powerpc/kernel/signal_32.c +++ b/arch/powerpc/kernel/signal_32.c @@ -1063,7 +1063,7 @@ SYSCALL_DEFINE3(swapcontext, struct ucontext __user *, old_ctx, * We kill the task with a SIGSEGV in this situation. */ if (do_setcontext(new_ctx, regs, 0)) { - force_sigsegv(SIGSEGV); + force_fatal_sig(SIGSEGV); return -EFAULT; } diff --git a/arch/powerpc/kernel/signal_64.c b/arch/powerpc/kernel/signal_64.c index d8de622c9e4a..8ead9b3f47c6 100644 --- a/arch/powerpc/kernel/signal_64.c +++ b/arch/powerpc/kernel/signal_64.c @@ -704,7 +704,7 @@ SYSCALL_DEFINE3(swapcontext, struct ucontext __user *, old_ctx, */ if (__get_user_sigset(&set, &new_ctx->uc_sigmask)) { - force_sigsegv(SIGSEGV); + force_fatal_sig(SIGSEGV); return -EFAULT; } set_current_blocked(&set); @@ -713,7 +713,7 @@ SYSCALL_DEFINE3(swapcontext, struct ucontext __user *, old_ctx, return -EFAULT; if (__unsafe_restore_sigcontext(current, NULL, 0, &new_ctx->uc_mcontext)) { user_read_access_end(); - force_sigsegv(SIGSEGV); + force_fatal_sig(SIGSEGV); return -EFAULT; } user_read_access_end(); diff --git a/arch/s390/kernel/traps.c b/arch/s390/kernel/traps.c index 51729ea2cf8e..01a7c68dcfb6 100644 --- a/arch/s390/kernel/traps.c +++ b/arch/s390/kernel/traps.c @@ -84,7 +84,7 @@ static void default_trap_handler(struct pt_regs *regs) { if (user_mode(regs)) { report_user_fault(regs, SIGSEGV, 0); - force_sigsegv(SIGSEGV); + force_fatal_sig(SIGSEGV); } else die(regs, "Unknown program exception"); } diff --git a/arch/um/kernel/trap.c b/arch/um/kernel/trap.c index 3198c4767387..c32efb09db21 100644 --- a/arch/um/kernel/trap.c +++ b/arch/um/kernel/trap.c @@ -158,7 +158,7 @@ static void bad_segv(struct faultinfo fi, unsigned long ip) void fatal_sigsegv(void) { - force_sigsegv(SIGSEGV); + force_fatal_sig(SIGSEGV); do_signal(¤t->thread.regs); /* * This is to tell gcc that we're not returning - do_signal diff --git a/arch/x86/kernel/vm86_32.c b/arch/x86/kernel/vm86_32.c index 040fd01be8b3..7ff0f622abd4 100644 --- a/arch/x86/kernel/vm86_32.c +++ b/arch/x86/kernel/vm86_32.c @@ -159,7 +159,7 @@ void save_v86_state(struct kernel_vm86_regs *regs, int retval) user_access_end(); Efault: pr_alert("could not access userspace vm86 info\n"); - force_sigsegv(SIGSEGV); + force_fatal_sig(SIGSEGV); } static int do_vm86_irq_handling(int subfunction, int irqnumber); diff --git a/fs/exec.c b/fs/exec.c index a098c133d8d7..ac7b51b51f38 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -1852,7 +1852,7 @@ static int bprm_execve(struct linux_binprm *bprm, * SIGSEGV. */ if (bprm->point_of_no_return && !fatal_signal_pending(current)) - force_sigsegv(SIGSEGV); + force_fatal_sig(SIGSEGV); out_unmark: current->fs->in_exec = 0; -- 2.20.1 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH 21/20] signal: Replace force_sigsegv(SIGSEGV) with force_fatal_sig(SIGSEGV) 2021-10-20 21:51 ` [PATCH 21/20] signal: Replace force_sigsegv(SIGSEGV) with force_fatal_sig(SIGSEGV) Eric W. Biederman @ 2021-10-21 8:09 ` Geert Uytterhoeven 2021-10-21 13:33 ` Eric W. Biederman 2021-10-21 8:32 ` Philippe Mathieu-Daudé 1 sibling, 1 reply; 12+ messages in thread From: Geert Uytterhoeven @ 2021-10-21 8:09 UTC (permalink / raw) To: Eric W. Biederman Cc: Linux Kernel Mailing List, Rich Felker, open list:TENSILICA XTENSA PORT (xtensa), Benjamin Herrenschmidt, open list:BROADCOM NVRAM DRIVER, Max Filippov, Paul Mackerras, H Peter Anvin, sparclinux, Vincent Chen, Thomas Gleixner, Linux-Arch, linux-s390, Yoshinori Sato, Michael Ellerman, Linux-sh list, Christian Borntraeger, Ingo Molnar, Jonas Bonn, Kees Cook, Vasily Gorbik, Heiko Carstens, Openrisc, Borislav Petkov, Al Viro, Andy Lutomirski, Chris Zankel, Thomas Bogendoerfer, Nick Hu, linuxppc-dev, Oleg Nesterov, Greg Kroah-Hartman, Maciej Rozycki, Linus Torvalds, David Miller, Greentime Hu Hi Eric, Patch 21/20? On Wed, Oct 20, 2021 at 11:52 PM Eric W. Biederman <ebiederm@xmission.com> wrote: > Now that force_fatal_sig exists it is unnecessary and a bit confusing > to use force_sigsegv in cases where the simpler force_fatal_sig is > wanted. So change every instance we can to make the code clearer. > > Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com> > arch/m68k/kernel/traps.c | 2 +- Acked-by: Geert Uytterhoeven <geert@linux-m68k.org> Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 21/20] signal: Replace force_sigsegv(SIGSEGV) with force_fatal_sig(SIGSEGV) 2021-10-21 8:09 ` Geert Uytterhoeven @ 2021-10-21 13:33 ` Eric W. Biederman 0 siblings, 0 replies; 12+ messages in thread From: Eric W. Biederman @ 2021-10-21 13:33 UTC (permalink / raw) To: Geert Uytterhoeven Cc: Linux Kernel Mailing List, Rich Felker, open list:TENSILICA XTENSA PORT (xtensa), Benjamin Herrenschmidt, open list:BROADCOM NVRAM DRIVER, Max Filippov, Paul Mackerras, H Peter Anvin, sparclinux, Vincent Chen, Thomas Gleixner, Linux-Arch, linux-s390, Yoshinori Sato, Michael Ellerman, Linux-sh list, Christian Borntraeger, Ingo Molnar, Jonas Bonn, Kees Cook, Vasily Gorbik, Heiko Carstens, Openrisc, Borislav Petkov, Al Viro, Andy Lutomirski, Chris Zankel, Thomas Bogendoerfer, Nick Hu, linuxppc-dev, Oleg Nesterov, Greg Kroah-Hartman, Maciej Rozycki, Linus Torvalds, David Miller, Greentime Hu Geert Uytterhoeven <geert@linux-m68k.org> writes: > Hi Eric, > > Patch 21/20? In reviewing another part of the patchset Linus asked if force_sigsegv could go away. It can't completely but I can get this far. Given that it is just a cleanup it makes most sense to me as an additional patch on top of what is already here. > On Wed, Oct 20, 2021 at 11:52 PM Eric W. Biederman > <ebiederm@xmission.com> wrote: >> Now that force_fatal_sig exists it is unnecessary and a bit confusing >> to use force_sigsegv in cases where the simpler force_fatal_sig is >> wanted. So change every instance we can to make the code clearer. >> >> Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com> > >> arch/m68k/kernel/traps.c | 2 +- > > Acked-by: Geert Uytterhoeven <geert@linux-m68k.org> Thank you. Eric ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 21/20] signal: Replace force_sigsegv(SIGSEGV) with force_fatal_sig(SIGSEGV) 2021-10-20 21:51 ` [PATCH 21/20] signal: Replace force_sigsegv(SIGSEGV) with force_fatal_sig(SIGSEGV) Eric W. Biederman 2021-10-21 8:09 ` Geert Uytterhoeven @ 2021-10-21 8:32 ` Philippe Mathieu-Daudé 1 sibling, 0 replies; 12+ messages in thread From: Philippe Mathieu-Daudé @ 2021-10-21 8:32 UTC (permalink / raw) To: Eric W. Biederman Cc: open list, linux-arch, Linus Torvalds, Oleg Nesterov, Al Viro, Kees Cook, Andy Lutomirski, Jonas Bonn, Stefan Kristiansson, Stafford Horne, openrisc, Nick Hu, Greentime Hu, Vincent Chen, Heiko Carstens, Vasily Gorbik, Christian Borntraeger, linux-s390, Yoshinori Sato, Rich Felker, linux-sh, linux-xtensa, Chris Zankel, Max Filippov, David Miller, sparclinux, Thomas Bogendoerfer, Maciej Rozycki, open list:BROADCOM NVRAM DRIVER, Michael Ellerman, Benjamin Herrenschmidt, Paul Mackerras, linuxppc-dev, Thomas Gleixner, Ingo Molnar, Borislav Petkov, H Peter Anvin, Greg Kroah-Hartman On Wed, Oct 20, 2021 at 11:52 PM Eric W. Biederman <ebiederm@xmission.com> wrote: > > > Now that force_fatal_sig exists it is unnecessary and a bit confusing > to use force_sigsegv in cases where the simpler force_fatal_sig is > wanted. So change every instance we can to make the code clearer. > > Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com> > --- > arch/arc/kernel/process.c | 2 +- > arch/m68k/kernel/traps.c | 2 +- > arch/powerpc/kernel/signal_32.c | 2 +- > arch/powerpc/kernel/signal_64.c | 4 ++-- > arch/s390/kernel/traps.c | 2 +- > arch/um/kernel/trap.c | 2 +- > arch/x86/kernel/vm86_32.c | 2 +- > fs/exec.c | 2 +- > 8 files changed, 9 insertions(+), 9 deletions(-) Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2021-10-27 14:24 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-10-20 17:32 [PATCH 00/20] exit cleanups Eric W. Biederman 2021-10-20 17:43 ` [PATCH 02/20] exit: Remove calls of do_exit after noreturn versions of die Eric W. Biederman 2021-10-21 16:02 ` Kees Cook 2021-10-21 16:25 ` Eric W. Biederman 2021-10-20 17:43 ` [PATCH 06/20] signal/sh: Use force_sig(SIGKILL) instead of do_group_exit(SIGKILL) Eric W. Biederman 2021-10-20 19:57 ` Linus Torvalds 2021-10-27 14:24 ` Rich Felker 2021-10-21 16:08 ` Kees Cook 2021-10-20 21:51 ` [PATCH 21/20] signal: Replace force_sigsegv(SIGSEGV) with force_fatal_sig(SIGSEGV) Eric W. Biederman 2021-10-21 8:09 ` Geert Uytterhoeven 2021-10-21 13:33 ` Eric W. Biederman 2021-10-21 8:32 ` Philippe Mathieu-Daudé
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).