"H.J. Lu" <[email protected]> writes:
> Like this?
Yes, thanks! A few comments below.
> +AC_CACHE_CHECK([compiler support for -mshstk],
> + nettle_cv_x86_mshstk_cflags,
> +[AC_TRY_COMPILE([],[int foo],
> +nettle_cv_x86_mshstk_cflags=yes,
> +nettle_cv_x86_mshstk_cflags=no)])
> +if test "$nettle_cv_x86_mshstk_cflags" = yes; then
> + EXTRA_X86_CFLAGS=-mshstk
> +fi
> +AC_SUBST(EXTRA_X86_CFLAGS)
Maybe rename autoconf variable to TESTSUITE_CFLAGS.
This check may well be enough, but you could consider limiting it to x86
targets, and having the test program include x86intrin.h and use
_get_ssp.
> diff --git a/testsuite/.test-rules.make b/testsuite/.test-rules.make
> index 922a2c7f..257ce86e 100644
> --- a/testsuite/.test-rules.make
> +++ b/testsuite/.test-rules.make
> @@ -178,6 +178,9 @@ xts-test$(EXEEXT): xts-test.$(OBJEXT)
> pbkdf2-test$(EXEEXT): pbkdf2-test.$(OBJEXT)
> $(LINK) pbkdf2-test.$(OBJEXT) $(TEST_OBJS) -o pbkdf2-test$(EXEEXT)
>
> +ibt-test$(EXEEXT): ibt-test.$(OBJEXT)
> + $(LINK) ibt-test.$(OBJEXT) $(TEST_OBJS) -o ibt-test$(EXEEXT)
> +
Maybe rename x86-ibt-test, if you think this feature is going to be x86
only for the foreseeable future.
> diff --git a/testsuite/ibt-test.c b/testsuite/ibt-test.c
> new file mode 100644
> index 00000000..fdc97d8f
> --- /dev/null
> +++ b/testsuite/ibt-test.c
> @@ -0,0 +1,43 @@
> +#include "testutils.h"
> +#ifdef __CET__
It would make some sense to also check for __GNUC__,
__i386__/__x86_64__, and __posix__. Is __CET__ specific enough, or is
there a risk that some other compiler or target will use that name with
a different meaning? In particular, can it break windows builds?
If needed here, same should apply to the configure check from the
previous patch.
> +#include <signal.h>
> +#include <x86intrin.h>
> +
> +static void
> +segfault_handler(int signo)
> +{
> + exit(0);
> +}
> +
> +void
> +ibt_violation(void)
> +{
> +#ifdef __i386__
> + asm volatile("lea 1f, %eax\n\t"
> + "jmp *%eax\n"
> + "1:");
> +#else
> + asm volatile("lea 1f, %rax\n\t"
> + "jmp *%rax\n"
> + "1:");
> +#endif
> +}
Nice and easy. I think these need a clobber list to tell gcc that it
modifies eax/rax, though.
> +void
> +test_main(void)
> +{
> + /* NB: This test should trigger SIGSEGV on CET platforms. If SHSTK
> + is disabled, assuming IBT is also disabled. */
> + if (_get_ssp() == 0)
> + SKIP();
I would suggest
if (_get_ssp() == 0)
{
ibt_violation()
SKIP();
}
to have the inline asm exercised, even when cet is disabled.
Can you explain what _get_ssp does? It seems it is included via
x86intrin.h, and defined (on x86_64) as
extern __inline unsigned long long
__attribute__((__gnu_inline__, __always_inline__, __artificial__))
_get_ssp (void)
{
return __builtin_ia32_rdsspq ();
}
Would it be easier to define it using inline asm, eliminating the
configure check for -mshstk? Or is it a complicated thing involving a
cpuid check first?
> + signal(SIGSEGV, segfault_handler);
> + ibt_violation();
> + FAIL();
> +}
> +#else
> +void
> +test_main(void)
> +{
> +}
Should use SKIP()
Regards,
/Niels
--
Niels Möller. PGP-encrypted email is preferred. Keyid 368C6677.
Internet email is subject to wholesale government surveillance.
_______________________________________________
nettle-bugs mailing list
[email protected]
http://lists.lysator.liu.se/mailman/listinfo/nettle-bugs
