Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1345494 > unrolled thread

[PATCH 01/10] selftests/x86: In syscall_nt, test NT|TF as well

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 01/10] selftests/x86: In syscall_nt, test NT|TF as well Andy Lutomirski <luto@kernel.org> - 2016-02-29 06:30 +0100
    Re: [PATCH 01/10] selftests/x86: In syscall_nt, test NT|TF as well Borislav Petkov <bp@alien8.de> - 2016-03-02 14:10 +0100
      Re: [PATCH 01/10] selftests/x86: In syscall_nt, test NT|TF as well One Thousand Gnomes <gnomes@lxorguk.ukuu.org.uk> - 2016-03-02 15:10 +0100
        Re: [PATCH 01/10] selftests/x86: In syscall_nt, test NT|TF as well Borislav Petkov <bp@alien8.de> - 2016-03-02 15:30 +0100
          Re: [PATCH 01/10] selftests/x86: In syscall_nt, test NT|TF as well Andy Lutomirski <luto@amacapital.net> - 2016-03-02 20:10 +0100
            Re: [PATCH 01/10] selftests/x86: In syscall_nt, test NT|TF as well Borislav Petkov <bp@alien8.de> - 2016-03-02 20:40 +0100

#1345494 — [PATCH 01/10] selftests/x86: In syscall_nt, test NT|TF as well

Setting TF prevents fastpath returns in most cases, which causes the
test to fail on 32-bit kernels because 32-bit kernels do not, in
fact, handle NT correctly on SYSENTER entries.

The next patch will fix 32-bit kernels.

Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 tools/testing/selftests/x86/syscall_nt.c | 57 +++++++++++++++++++++++++++-----
 1 file changed, 49 insertions(+), 8 deletions(-)

diff --git a/tools/testing/selftests/x86/syscall_nt.c b/tools/testing/selftests/x86/syscall_nt.c
index 60c06af4646a..a6ceff86c199 100644
--- a/tools/testing/selftests/x86/syscall_nt.c
+++ b/tools/testing/selftests/x86/syscall_nt.c
@@ -17,6 +17,9 @@
 
 #include <stdio.h>
 #include <unistd.h>
+#include <string.h>
+#include <signal.h>
+#include <err.h>
 #include <sys/syscall.h>
 #include <asm/processor-flags.h>
 
@@ -26,6 +29,8 @@
 # define WIDTH "l"
 #endif
 
+static unsigned int nerrs;
+
 static unsigned long get_eflags(void)
 {
 	unsigned long eflags;
@@ -39,16 +44,52 @@ static void set_eflags(unsigned long eflags)
 		      : : "rm" (eflags) : "flags");
 }
 
-int main()
+static void sethandler(int sig, void (*handler)(int, siginfo_t *, void *),
+		       int flags)
 {
-	printf("[RUN]\tSet NT and issue a syscall\n");
-	set_eflags(get_eflags() | X86_EFLAGS_NT);
+	struct sigaction sa;
+	memset(&sa, 0, sizeof(sa));
+	sa.sa_sigaction = handler;
+	sa.sa_flags = SA_SIGINFO | flags;
+	sigemptyset(&sa.sa_mask);
+	if (sigaction(sig, &sa, 0))
+		err(1, "sigaction");
+}
+
+static void sigtrap(int sig, siginfo_t *si, void *ctx_void)
+{
+}
+
+static void do_it(unsigned long extraflags)
+{
+	unsigned long flags;
+
+	set_eflags(get_eflags() | extraflags);
 	syscall(SYS_getpid);
-	if (get_eflags() & X86_EFLAGS_NT) {
-		printf("[OK]\tThe syscall worked and NT is still set\n");
-		return 0;
+	flags = get_eflags();
+	if ((flags & extraflags) == extraflags) {
+		printf("[OK]\tThe syscall worked and flags are still set\n");
 	} else {
-		printf("[FAIL]\tThe syscall worked but NT was cleared\n");
-		return 1;
+		printf("[FAIL]\tThe syscall worked but flags were cleared (flags = 0x%lx but expected 0x%lx set)\n",
+		       flags, extraflags);
+		nerrs++;
 	}
 }
+
+int main()
+{
+	printf("[RUN]\tSet NT and issue a syscall\n");
+	do_it(X86_EFLAGS_NT);
+
+	/*
+	 * Now try it again with TF set -- TF forces returns via IRET in all
+	 * cases except non-ptregs-using 64-bit full fast path syscalls.
+	 */
+
+	sethandler(SIGTRAP, sigtrap, 0);
+
+	printf("[RUN]\tSet NT|TF and issue a syscall\n");
+	do_it(X86_EFLAGS_NT | X86_EFLAGS_TF);
+
+	return nerrs == 0 ? 0 : 1;
+}
-- 
2.5.0

[toc] | [next] | [standalone]

#1348021

On Sun, Feb 28, 2016 at 09:28:46PM -0800, Andy Lutomirski wrote:
> Setting TF prevents fastpath returns in most cases, which causes the
> test to fail on 32-bit kernels because 32-bit kernels do not, in
> fact, handle NT correctly on SYSENTER entries.
> 
> The next patch will fix 32-bit kernels.
> 
> Signed-off-by: Andy Lutomirski <luto@kernel.org>
> ---
>  tools/testing/selftests/x86/syscall_nt.c | 57 +++++++++++++++++++++++++++-----
>  1 file changed, 49 insertions(+), 8 deletions(-)
> 
> diff --git a/tools/testing/selftests/x86/syscall_nt.c b/tools/testing/selftests/x86/syscall_nt.c
> index 60c06af4646a..a6ceff86c199 100644
> --- a/tools/testing/selftests/x86/syscall_nt.c
> +++ b/tools/testing/selftests/x86/syscall_nt.c

...

> +static void do_it(unsigned long extraflags)
> +{
> +	unsigned long flags;
> +
> +	set_eflags(get_eflags() | extraflags);
>  	syscall(SYS_getpid);
> -	if (get_eflags() & X86_EFLAGS_NT) {
> -		printf("[OK]\tThe syscall worked and NT is still set\n");
> -		return 0;
> +	flags = get_eflags();
> +	if ((flags & extraflags) == extraflags) {
> +		printf("[OK]\tThe syscall worked and flags are still set\n");
>  	} else {
> -		printf("[FAIL]\tThe syscall worked but NT was cleared\n");
> -		return 1;
> +		printf("[FAIL]\tThe syscall worked but flags were cleared (flags = 0x%lx but expected 0x%lx set)\n",
> +		       flags, extraflags);
> +		nerrs++;
>  	}
>  }
> +
> +int main()

ERROR: Bad function definition - int main() should probably be int main(void)

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

[toc] | [prev] | [next] | [standalone]

#1348057

On Wed, 2 Mar 2016 13:59:52 +0100
Borislav Petkov <bp@alien8.de> wrote:

> On Sun, Feb 28, 2016 at 09:28:46PM -0800, Andy Lutomirski wrote:
> > Setting TF prevents fastpath returns in most cases, which causes the
> > test to fail on 32-bit kernels because 32-bit kernels do not, in
> > fact, handle NT correctly on SYSENTER entries.
> > 
> > The next patch will fix 32-bit kernels.
> > 
> > Signed-off-by: Andy Lutomirski <luto@kernel.org>
> > ---
> >  tools/testing/selftests/x86/syscall_nt.c | 57 +++++++++++++++++++++++++++-----
> >  1 file changed, 49 insertions(+), 8 deletions(-)
> > 
> > diff --git a/tools/testing/selftests/x86/syscall_nt.c b/tools/testing/selftests/x86/syscall_nt.c
> > index 60c06af4646a..a6ceff86c199 100644
> > --- a/tools/testing/selftests/x86/syscall_nt.c
> > +++ b/tools/testing/selftests/x86/syscall_nt.c  
> 
> ...
> 
> > +static void do_it(unsigned long extraflags)
> > +{
> > +	unsigned long flags;
> > +
> > +	set_eflags(get_eflags() | extraflags);
> >  	syscall(SYS_getpid);
> > -	if (get_eflags() & X86_EFLAGS_NT) {
> > -		printf("[OK]\tThe syscall worked and NT is still set\n");
> > -		return 0;
> > +	flags = get_eflags();
> > +	if ((flags & extraflags) == extraflags) {
> > +		printf("[OK]\tThe syscall worked and flags are still set\n");
> >  	} else {
> > -		printf("[FAIL]\tThe syscall worked but NT was cleared\n");
> > -		return 1;
> > +		printf("[FAIL]\tThe syscall worked but flags were cleared (flags = 0x%lx but expected 0x%lx set)\n",
> > +		       flags, extraflags);
> > +		nerrs++;
> >  	}
> >  }
> > +
> > +int main()  
> 
> ERROR: Bad function definition - int main() should probably be int main(void)

int main(void) is wrong as there are passed arguments

int main() is ok (in C89 at least) because it means "there are unknown
arguments"

int main(int argc, char *argv[]) is allowed

int main(void) is not safe on all platforms because some compilers
choose to do the argument cleanup in the return path of the called
function. Having the wrong number of arguments doesn't end well in such
cases. I doubt any Linux platforms do this but we shouldn't be
encouraging bad programming techniques 8)

Alan

[toc] | [prev] | [next] | [standalone]

#1348071

On Wed, Mar 02, 2016 at 02:01:15PM +0000, One Thousand Gnomes wrote:
> int main(void) is wrong as there are passed arguments

Not in this particular case - test doesn't take args.

> int main() is ok (in C89 at least) because it means "there are unknown
> arguments"
> 
> int main(int argc, char *argv[]) is allowed
> 
> int main(void) is not safe on all platforms because some compilers
> choose to do the argument cleanup in the return path of the called
> function. Having the wrong number of arguments doesn't end well in such
> cases. I doubt any Linux platforms do this but we shouldn't be
> encouraging bad programming techniques 8)

There's also the variadic thing. Here's hpa's sermon from a couple of
years ago:

http://thread.gmane.org/gmane.linux.kernel/1268751/focus=1268792

:-))))

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

[toc] | [prev] | [next] | [standalone]

#1348412

On Mar 2, 2016 6:29 AM, "Borislav Petkov" <bp@alien8.de> wrote:
>
> On Wed, Mar 02, 2016 at 02:01:15PM +0000, One Thousand Gnomes wrote:
> > int main(void) is wrong as there are passed arguments
>
> Not in this particular case - test doesn't take args.

IIRC the C standard says otherwise. main is special.

Arguably checkpatch should learn about that -- it doesn't matter for
*kernel* code, but this is a user program.  (Also, it's shamelessly
written in C99.  I figure 15 years is long enough...)

[toc] | [prev] | [next] | [standalone]

#1348423

On Wed, Mar 02, 2016 at 11:03:44AM -0800, Andy Lutomirski wrote:
> IIRC the C standard says otherwise. main is special.

My C99 draft pdf says either "int main(void)" or "int main(int argc,
char *argv[])".

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web