Messages in this thread | | | From | (Eric W. Biederman) | Date | Fri, 29 Oct 2021 14:32:36 -0500 | Subject | Re: [PATCH 11/20] signal/s390: Use force_sigsegv in default_trap_handler |
| |
ebiederm@xmission.com (Eric W. Biederman) writes:
> Christian Borntraeger <borntraeger@de.ibm.com> writes: > >> Am 20.10.21 um 19:43 schrieb Eric W. Biederman: >>> Reading the history it is unclear why default_trap_handler calls >>> do_exit. It is not even menthioned in the commit where the change >>> happened. My best guess is that because it is unknown why the >>> exception happened it was desired to guarantee the process never >>> returned to userspace. >>> >>> Using do_exit(SIGSEGV) has the problem that it will only terminate one >>> thread of a process, leaving the process in an undefined state. >>> >>> Use force_sigsegv(SIGSEGV) instead which effectively has the same >>> behavior except that is uses the ordinary signal mechanism and >>> terminates all threads of a process and is generally well defined. >> >> Do I get that right, that programs can not block SIGSEGV from force_sigsegv >> with a signal handler? Thats how I read the code. If this is true >> then >> >> Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com> > > 99% true, and it is what force_sigsegv(SIGSEGV) intends to do. > > Andy Lutormorski pointed at a race where a thread can call sigaction > and change the signal handler after force_sigsegv has run but before > the process dequeues the SIGSEGV.
I now have a simple patch that closes the sigaction vs force_sig race, that I am adding to this set of changes. So now I can say programs can not block force_sigsegv(SIGSEGV) with a signal handler or any other method.
Eric
>>> 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 >>> Fixes: ca2ab03237ec ("[PATCH] s390: core changes") >>> History Tree: https://git.kernel.org/pub/scm/linux/kernel/git/tglx/history.git >>> Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com> >>> --- >>> arch/s390/kernel/traps.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/arch/s390/kernel/traps.c b/arch/s390/kernel/traps.c >>> index bcefc2173de4..51729ea2cf8e 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); >>> - do_exit(SIGSEGV); >>> + force_sigsegv(SIGSEGV); >>> } else >>> die(regs, "Unknown program exception"); >>> } >>>
| |