Messages in this thread |  | | From | (Eric W. Biederman) | Date | Thu, 28 Oct 2021 10:56:24 -0500 | Subject | Re: [PATCH 11/20] signal/s390: Use force_sigsegv in default_trap_handler |
| |
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.
In principle it isn't too hard to close that race, and I was hoping to be able to tell you that I had sorted by the time I replied. Unfortunately it looks like it will take another week or two so will probably not be ready by the merge window.
I am definitely going to close that race.
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"); >> } >>
|  |