Hi,
grep'ed the tree for siglongjmp calls, and spotted possible offenders in
libssl's code. The code in question checks hardware capabilities for
ARM, S390x, and SPARCv9.
The code will call some routines that could trigger SIGILL (or SIGBUS),
which is caught with an own signal handler. This signal handler will
perform the previously mentioned siglongjmp and jumps out of the
capability testing.
Unfortunately, the signal handlers are registered and activated before
the jump environment is properly initialized. If the program receives
SIGILL (or SIGBUS in case of SPARCv9) from another source, the behavior
of the program will be undefined.
Fix for ARM and S390x is simple: Register the signal handlers after the
environment has been properly set up. Which means right after sigsetjmp.
The SPARCv9 code is more complicated: It registers a signal handler for
SIGILL and SIGBUS. These signal handlers don't block each other, which
means that SIGILL performing a siglongjmp could be interrupted by SIGBUS
which will perform a siglongjmp, too. I haven't seen these jump methods
as signal-safe in signal(3), so I assume that they are not. The first
part of the fix is to block SIGILL and SIGBUS while performing the
signal handler.
Also, SPARCv9 code overrides the environment between two independent
tests, still allowing these signals. It means that the setting of the
jump environment can be interrupted with a jump... That sounds like
asking for trouble. Therefore, I block these two signals during call of
sigsetjmp.
Last remaining bits are KNF and removing unused code.
It's difficult to write proper signal handlers, and even worse, I lack
machines of these types. "gcc -c" compiles the S390x and SPARCv9 files
for me, so there shouldn't be syntax errors. Yet I really need peer
reviews. If somebody dares to look at signals: Now is the time! :)
Tobias
Index: armcap.c
===
RCS file: /cvs/src/lib/libssl/src/crypto/armcap.c,v
retrieving revision 1.6
diff -u -p -r1.6 armcap.c
--- armcap.c20 Jun 2014 21:00:46 - 1.6
+++ armcap.c14 Mar 2015 17:52:50 -
@@ -14,7 +14,10 @@ unsigned int OPENSSL_armcap_P;
static sigset_t all_masked;
static sigjmp_buf ill_jmp;
- static void ill_handler (int sig) { siglongjmp(ill_jmp, sig);
+static void
+ill_handler(int sig)
+{
+ siglongjmp(ill_jmp, sig);
}
/*
@@ -31,9 +34,6 @@ void OPENSSL_cpuid_setup(void) __attribu
void
OPENSSL_cpuid_setup(void)
{
-#ifndef __OpenBSD__
- char *e;
-#endif
#if __ARM_ARCH__ >= 7
struct sigactionill_oact, ill_act;
sigset_toset;
@@ -59,14 +59,14 @@ OPENSSL_cpuid_setup(void)
ill_act.sa_mask = all_masked;
sigprocmask(SIG_SETMASK, &ill_act.sa_mask, &oset);
- sigaction(SIGILL, &ill_act, &ill_oact);
if (sigsetjmp(ill_jmp, 1) == 0) {
+ sigaction(SIGILL, &ill_act, &ill_oact);
_armv7_neon_probe();
OPENSSL_armcap_P |= ARMV7_NEON;
}
- sigaction (SIGILL, &ill_oact, NULL);
+ sigaction(SIGILL, &ill_oact, NULL);
sigprocmask(SIG_SETMASK, &oset, NULL);
#endif
}
Index: s390xcap.c
===
RCS file: /cvs/src/lib/libssl/src/crypto/s390xcap.c,v
retrieving revision 1.3
diff -u -p -r1.3 s390xcap.c
--- s390xcap.c 12 Jun 2014 15:49:27 - 1.3
+++ s390xcap.c 14 Mar 2015 17:52:50 -
@@ -8,7 +8,8 @@
extern unsigned long OPENSSL_s390xcap_P[];
static sigjmp_buf ill_jmp;
-static void ill_handler (int sig)
+static void
+ill_handler(int sig)
{
siglongjmp(ill_jmp, sig);
}
@@ -32,12 +33,13 @@ OPENSSL_cpuid_setup(void)
sigdelset(&ill_act.sa_mask, SIGILL);
sigdelset(&ill_act.sa_mask, SIGTRAP);
sigprocmask(SIG_SETMASK, &ill_act.sa_mask, &oset);
- sigaction (SIGILL, &ill_act, &oact);
/* protection against missing store-facility-list-extended */
- if (sigsetjmp(ill_jmp, 1) == 0)
+ if (sigsetjmp(ill_jmp, 1) == 0) {
+ sigaction(SIGILL, &ill_act, &oact);
OPENSSL_s390x_facilities();
+ }
- sigaction (SIGILL, &oact, NULL);
+ sigaction(SIGILL, &oact, NULL);
sigprocmask(SIG_SETMASK, &oset, NULL);
}
Index: sparcv9cap.c
===
RCS file: /cvs/src/lib/libssl/src/crypto/sparcv9cap.c,v
retrieving revision 1.7
diff -u -p -r1.7 sparcv9cap.c
--- sparcv9cap.c20 Jun 2014 21:00:46 - 1.7
+++ sparcv9cap.c14 Mar 2015 17:52:50 -
@@ -44,9 +44,8 @@ common_handler(int sig)
void
OPENSSL_cpuid_setup(void)
{
- char *e;
struct sigactioncommon_act, ill_oact, bus_oact;
- sigset_tall_masked, oset;
+ sigset_tothers_masked, all_masked, oset;
static int trigger = 0;
if (trigger)
@@ -56,25 +55,33 @@ OPENSSL_cpuid_setup(
