* [PATCH 0/2] A fix and a cleanup to the x86 sigreturn selftests @ 2018-06-27 5:17 Andy Lutomirski 2018-06-27 5:17 ` [PATCH 1/2] selftests/x86/sigreturn/64: Fix spurious failures on AMD CPUs Andy Lutomirski 2018-06-27 5:17 ` [PATCH 2/2] selftests/x86/sigreturn: Minor cleanups Andy Lutomirski 0 siblings, 2 replies; 5+ messages in thread From: Andy Lutomirski @ 2018-06-27 5:17 UTC (permalink / raw) To: x86, LKML; +Cc: Borislav Petkov, Andy Lutomirski The sigreturn_64 selftest has always failed on AMD CPUs. Fix it and clean it up a bit. Andy Lutomirski (2): selftests/x86/sigreturn/64: Fix spurious failures on AMD CPUs selftests/x86/sigreturn: Minor cleanups tools/testing/selftests/x86/sigreturn.c | 59 +++++++++++++++---------- 1 file changed, 36 insertions(+), 23 deletions(-) -- 2.17.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/2] selftests/x86/sigreturn/64: Fix spurious failures on AMD CPUs 2018-06-27 5:17 [PATCH 0/2] A fix and a cleanup to the x86 sigreturn selftests Andy Lutomirski @ 2018-06-27 5:17 ` Andy Lutomirski 2018-06-27 9:07 ` [tip:x86/urgent] " tip-bot for Andy Lutomirski 2018-06-27 5:17 ` [PATCH 2/2] selftests/x86/sigreturn: Minor cleanups Andy Lutomirski 1 sibling, 1 reply; 5+ messages in thread From: Andy Lutomirski @ 2018-06-27 5:17 UTC (permalink / raw) To: x86, LKML; +Cc: Borislav Petkov, Andy Lutomirski When I wrote the sigreturn test, I didn't realize that AMD's busted IRET behavior was different from Intel's busted IRET behavior. On AMD CPUs, the CPU leaks the high 32 bits of the kernel stack pointer to certain userspace contexts. Gee, thanks. There's very little the kernel can do about it. Modify the test so it passes. Signed-off-by: Andy Lutomirski <luto@kernel.org> --- tools/testing/selftests/x86/sigreturn.c | 46 ++++++++++++++++--------- 1 file changed, 29 insertions(+), 17 deletions(-) diff --git a/tools/testing/selftests/x86/sigreturn.c b/tools/testing/selftests/x86/sigreturn.c index 246145b84a12..2559e2c01793 100644 --- a/tools/testing/selftests/x86/sigreturn.c +++ b/tools/testing/selftests/x86/sigreturn.c @@ -612,19 +612,38 @@ static int test_valid_sigreturn(int cs_bits, bool use_16bit_ss, int force_ss) greg_t req = requested_regs[i], res = resulting_regs[i]; if (i == REG_TRAPNO || i == REG_IP) continue; /* don't care */ - if (i == REG_SP) { - printf("\tSP: %llx -> %llx\n", (unsigned long long)req, - (unsigned long long)res); + if (i == REG_SP) { /* - * In many circumstances, the high 32 bits of rsp - * are zeroed. For example, we could be a real - * 32-bit program, or we could hit any of a number - * of poorly-documented IRET or segmented ESP - * oddities. If this happens, it's okay. + * If we were using a 16-bit stack segment, then + * the kernel is a bit stuck: IRET only restores + * the low 16 bits of ESP/RSP if SS is 16-bit. + * The kernel uses a hack to restore bits 31:16, + * but that hack doesn't help with bits 63:32. + * On Intel CPUs, bits 63:32 end up zeroed, and, on + * AMD CPUs, they leak the high bits of the kernel + * espfix64 stack pointer. There's very little that + * the kernel can do about it. + * + * Similarly, if we are returning to a 32-bit context, + * the CPU will often lose the high 32 bits of RSP. */ - if (res == (req & 0xFFFFFFFF)) - continue; /* OK; not expected to work */ + + if (res == req) + continue; + + if (cs_bits != 64 && ((res ^ req) & 0xFFFFFFFF) == 0) { + printf("[NOTE]\tSP: %llx -> %llx\n", + (unsigned long long)req, + (unsigned long long)res); + continue; + } + + printf("[FAIL]\tSP mismatch: requested 0x%llx; got 0x%llx\n", + (unsigned long long)requested_regs[i], + (unsigned long long)resulting_regs[i]); + nerrs++; + continue; } bool ignore_reg = false; @@ -663,13 +682,6 @@ static int test_valid_sigreturn(int cs_bits, bool use_16bit_ss, int force_ss) } if (requested_regs[i] != resulting_regs[i] && !ignore_reg) { - /* - * SP is particularly interesting here. The - * usual cause of failures is that we hit the - * nasty IRET case of returning to a 16-bit SS, - * in which case bits 16:31 of the *kernel* - * stack pointer persist in ESP. - */ printf("[FAIL]\tReg %d mismatch: requested 0x%llx; got 0x%llx\n", i, (unsigned long long)requested_regs[i], (unsigned long long)resulting_regs[i]); -- 2.17.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [tip:x86/urgent] selftests/x86/sigreturn/64: Fix spurious failures on AMD CPUs 2018-06-27 5:17 ` [PATCH 1/2] selftests/x86/sigreturn/64: Fix spurious failures on AMD CPUs Andy Lutomirski @ 2018-06-27 9:07 ` tip-bot for Andy Lutomirski 0 siblings, 0 replies; 5+ messages in thread From: tip-bot for Andy Lutomirski @ 2018-06-27 9:07 UTC (permalink / raw) To: linux-tip-commits Cc: torvalds, linux-kernel, mingo, hpa, peterz, bp, tglx, luto Commit-ID: ec348020566009d3da9b99f07c05814d13969c78 Gitweb: https://git.kernel.org/tip/ec348020566009d3da9b99f07c05814d13969c78 Author: Andy Lutomirski <luto@kernel.org> AuthorDate: Tue, 26 Jun 2018 22:17:17 -0700 Committer: Ingo Molnar <mingo@kernel.org> CommitDate: Wed, 27 Jun 2018 09:36:56 +0200 selftests/x86/sigreturn/64: Fix spurious failures on AMD CPUs When I wrote the sigreturn test, I didn't realize that AMD's busted IRET behavior was different from Intel's busted IRET behavior: On AMD CPUs, the CPU leaks the high 32 bits of the kernel stack pointer to certain userspace contexts. Gee, thanks. There's very little the kernel can do about it. Modify the test so it passes. Signed-off-by: Andy Lutomirski <luto@kernel.org> Cc: Borislav Petkov <bp@alien8.de> Cc: Linus Torvalds <torvalds@linux-foundation.org> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Thomas Gleixner <tglx@linutronix.de> Link: http://lkml.kernel.org/r/86e7fd3564497f657de30a36da4505799eebef01.1530076529.git.luto@kernel.org Signed-off-by: Ingo Molnar <mingo@kernel.org> --- tools/testing/selftests/x86/sigreturn.c | 46 +++++++++++++++++++++------------ 1 file changed, 29 insertions(+), 17 deletions(-) diff --git a/tools/testing/selftests/x86/sigreturn.c b/tools/testing/selftests/x86/sigreturn.c index 246145b84a12..2559e2c01793 100644 --- a/tools/testing/selftests/x86/sigreturn.c +++ b/tools/testing/selftests/x86/sigreturn.c @@ -612,19 +612,38 @@ static int test_valid_sigreturn(int cs_bits, bool use_16bit_ss, int force_ss) greg_t req = requested_regs[i], res = resulting_regs[i]; if (i == REG_TRAPNO || i == REG_IP) continue; /* don't care */ - if (i == REG_SP) { - printf("\tSP: %llx -> %llx\n", (unsigned long long)req, - (unsigned long long)res); + if (i == REG_SP) { /* - * In many circumstances, the high 32 bits of rsp - * are zeroed. For example, we could be a real - * 32-bit program, or we could hit any of a number - * of poorly-documented IRET or segmented ESP - * oddities. If this happens, it's okay. + * If we were using a 16-bit stack segment, then + * the kernel is a bit stuck: IRET only restores + * the low 16 bits of ESP/RSP if SS is 16-bit. + * The kernel uses a hack to restore bits 31:16, + * but that hack doesn't help with bits 63:32. + * On Intel CPUs, bits 63:32 end up zeroed, and, on + * AMD CPUs, they leak the high bits of the kernel + * espfix64 stack pointer. There's very little that + * the kernel can do about it. + * + * Similarly, if we are returning to a 32-bit context, + * the CPU will often lose the high 32 bits of RSP. */ - if (res == (req & 0xFFFFFFFF)) - continue; /* OK; not expected to work */ + + if (res == req) + continue; + + if (cs_bits != 64 && ((res ^ req) & 0xFFFFFFFF) == 0) { + printf("[NOTE]\tSP: %llx -> %llx\n", + (unsigned long long)req, + (unsigned long long)res); + continue; + } + + printf("[FAIL]\tSP mismatch: requested 0x%llx; got 0x%llx\n", + (unsigned long long)requested_regs[i], + (unsigned long long)resulting_regs[i]); + nerrs++; + continue; } bool ignore_reg = false; @@ -663,13 +682,6 @@ static int test_valid_sigreturn(int cs_bits, bool use_16bit_ss, int force_ss) } if (requested_regs[i] != resulting_regs[i] && !ignore_reg) { - /* - * SP is particularly interesting here. The - * usual cause of failures is that we hit the - * nasty IRET case of returning to a 16-bit SS, - * in which case bits 16:31 of the *kernel* - * stack pointer persist in ESP. - */ printf("[FAIL]\tReg %d mismatch: requested 0x%llx; got 0x%llx\n", i, (unsigned long long)requested_regs[i], (unsigned long long)resulting_regs[i]); ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] selftests/x86/sigreturn: Minor cleanups 2018-06-27 5:17 [PATCH 0/2] A fix and a cleanup to the x86 sigreturn selftests Andy Lutomirski 2018-06-27 5:17 ` [PATCH 1/2] selftests/x86/sigreturn/64: Fix spurious failures on AMD CPUs Andy Lutomirski @ 2018-06-27 5:17 ` Andy Lutomirski 2018-06-27 9:08 ` [tip:x86/urgent] selftests/x86/sigreturn: Do minor cleanups tip-bot for Andy Lutomirski 1 sibling, 1 reply; 5+ messages in thread From: Andy Lutomirski @ 2018-06-27 5:17 UTC (permalink / raw) To: x86, LKML; +Cc: Borislav Petkov, Andy Lutomirski We have short names for the requested and resulting register values. Use them instead of spelling out the whole register entry for each case. Signed-off-by: Andy Lutomirski <luto@kernel.org> --- tools/testing/selftests/x86/sigreturn.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/tools/testing/selftests/x86/sigreturn.c b/tools/testing/selftests/x86/sigreturn.c index 2559e2c01793..4d9dc3f2fd70 100644 --- a/tools/testing/selftests/x86/sigreturn.c +++ b/tools/testing/selftests/x86/sigreturn.c @@ -610,6 +610,7 @@ static int test_valid_sigreturn(int cs_bits, bool use_16bit_ss, int force_ss) */ for (int i = 0; i < NGREG; i++) { greg_t req = requested_regs[i], res = resulting_regs[i]; + if (i == REG_TRAPNO || i == REG_IP) continue; /* don't care */ @@ -673,18 +674,18 @@ static int test_valid_sigreturn(int cs_bits, bool use_16bit_ss, int force_ss) #endif /* Sanity check on the kernel */ - if (i == REG_CX && requested_regs[i] != resulting_regs[i]) { + if (i == REG_CX && req != res) { printf("[FAIL]\tCX (saved SP) mismatch: requested 0x%llx; got 0x%llx\n", - (unsigned long long)requested_regs[i], - (unsigned long long)resulting_regs[i]); + (unsigned long long)req, + (unsigned long long)res); nerrs++; continue; } - if (requested_regs[i] != resulting_regs[i] && !ignore_reg) { + if (req != res && !ignore_reg) { printf("[FAIL]\tReg %d mismatch: requested 0x%llx; got 0x%llx\n", - i, (unsigned long long)requested_regs[i], - (unsigned long long)resulting_regs[i]); + i, (unsigned long long)req, + (unsigned long long)res); nerrs++; } } -- 2.17.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [tip:x86/urgent] selftests/x86/sigreturn: Do minor cleanups 2018-06-27 5:17 ` [PATCH 2/2] selftests/x86/sigreturn: Minor cleanups Andy Lutomirski @ 2018-06-27 9:08 ` tip-bot for Andy Lutomirski 0 siblings, 0 replies; 5+ messages in thread From: tip-bot for Andy Lutomirski @ 2018-06-27 9:08 UTC (permalink / raw) To: linux-tip-commits Cc: peterz, bp, mingo, linux-kernel, luto, tglx, hpa, torvalds Commit-ID: e8a445dea219c32727016af14f847d2e8f7ebec8 Gitweb: https://git.kernel.org/tip/e8a445dea219c32727016af14f847d2e8f7ebec8 Author: Andy Lutomirski <luto@kernel.org> AuthorDate: Tue, 26 Jun 2018 22:17:18 -0700 Committer: Ingo Molnar <mingo@kernel.org> CommitDate: Wed, 27 Jun 2018 09:36:56 +0200 selftests/x86/sigreturn: Do minor cleanups We have short names for the requested and resulting register values. Use them instead of spelling out the whole register entry for each case. Signed-off-by: Andy Lutomirski <luto@kernel.org> Cc: Borislav Petkov <bp@alien8.de> Cc: Linus Torvalds <torvalds@linux-foundation.org> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Thomas Gleixner <tglx@linutronix.de> Link: http://lkml.kernel.org/r/bb3bc1f923a2f6fe7912d22a1068fe29d6033d38.1530076529.git.luto@kernel.org Signed-off-by: Ingo Molnar <mingo@kernel.org> --- tools/testing/selftests/x86/sigreturn.c | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/tools/testing/selftests/x86/sigreturn.c b/tools/testing/selftests/x86/sigreturn.c index 2559e2c01793..4d9dc3f2fd70 100644 --- a/tools/testing/selftests/x86/sigreturn.c +++ b/tools/testing/selftests/x86/sigreturn.c @@ -610,6 +610,7 @@ static int test_valid_sigreturn(int cs_bits, bool use_16bit_ss, int force_ss) */ for (int i = 0; i < NGREG; i++) { greg_t req = requested_regs[i], res = resulting_regs[i]; + if (i == REG_TRAPNO || i == REG_IP) continue; /* don't care */ @@ -673,18 +674,18 @@ static int test_valid_sigreturn(int cs_bits, bool use_16bit_ss, int force_ss) #endif /* Sanity check on the kernel */ - if (i == REG_CX && requested_regs[i] != resulting_regs[i]) { + if (i == REG_CX && req != res) { printf("[FAIL]\tCX (saved SP) mismatch: requested 0x%llx; got 0x%llx\n", - (unsigned long long)requested_regs[i], - (unsigned long long)resulting_regs[i]); + (unsigned long long)req, + (unsigned long long)res); nerrs++; continue; } - if (requested_regs[i] != resulting_regs[i] && !ignore_reg) { + if (req != res && !ignore_reg) { printf("[FAIL]\tReg %d mismatch: requested 0x%llx; got 0x%llx\n", - i, (unsigned long long)requested_regs[i], - (unsigned long long)resulting_regs[i]); + i, (unsigned long long)req, + (unsigned long long)res); nerrs++; } } ^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-06-27 9:08 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-06-27 5:17 [PATCH 0/2] A fix and a cleanup to the x86 sigreturn selftests Andy Lutomirski 2018-06-27 5:17 ` [PATCH 1/2] selftests/x86/sigreturn/64: Fix spurious failures on AMD CPUs Andy Lutomirski 2018-06-27 9:07 ` [tip:x86/urgent] " tip-bot for Andy Lutomirski 2018-06-27 5:17 ` [PATCH 2/2] selftests/x86/sigreturn: Minor cleanups Andy Lutomirski 2018-06-27 9:08 ` [tip:x86/urgent] selftests/x86/sigreturn: Do minor cleanups tip-bot for Andy Lutomirski
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.