* [PATCH] selftests: x86: test_shadow_stack: return KSFT_SKIP when test is skipped
@ 2026-03-01 1:47 Aleksei Oladko
2026-03-15 19:08 ` Aleksei Oladko
2026-03-19 16:38 ` Pavel Tikhomirov
0 siblings, 2 replies; 6+ messages in thread
From: Aleksei Oladko @ 2026-03-01 1:47 UTC (permalink / raw)
To: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Shuah Khan, H . Peter Anvin
Cc: linux-kselftest, linux-kernel, Aleksei Oladko
test_shadow_stack prints a message indicating that the test is
skipped in some cases, but still returns 1. This causes the test
to be reported as failed instead of skipped.
Return KSFT_SKIP in the skip path so the result is reported
correctly.
Signed-off-by: Aleksei Oladko <aleksey.oladko@virtuozzo.com>
---
tools/testing/selftests/x86/test_shadow_stack.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/x86/test_shadow_stack.c b/tools/testing/selftests/x86/test_shadow_stack.c
index 21af54d5f4ea..1747ea4cb725 100644
--- a/tools/testing/selftests/x86/test_shadow_stack.c
+++ b/tools/testing/selftests/x86/test_shadow_stack.c
@@ -35,6 +35,7 @@
#include <sys/signal.h>
#include <linux/elf.h>
#include <linux/perf_event.h>
+#include "kselftest.h"
/*
* Define the ABI defines if needed, so people can run the tests
@@ -981,7 +982,7 @@ int main(int argc, char *argv[])
if (ARCH_PRCTL(ARCH_SHSTK_ENABLE, ARCH_SHSTK_SHSTK)) {
printf("[SKIP]\tCould not enable Shadow stack\n");
- return 1;
+ return KSFT_SKIP;
}
if (ARCH_PRCTL(ARCH_SHSTK_DISABLE, ARCH_SHSTK_SHSTK)) {
@@ -991,12 +992,12 @@ int main(int argc, char *argv[])
if (ARCH_PRCTL(ARCH_SHSTK_ENABLE, ARCH_SHSTK_SHSTK)) {
printf("[SKIP]\tCould not re-enable Shadow stack\n");
- return 1;
+ return KSFT_SKIP;
}
if (ARCH_PRCTL(ARCH_SHSTK_ENABLE, ARCH_SHSTK_WRSS)) {
printf("[SKIP]\tCould not enable WRSS\n");
- ret = 1;
+ ret = KSFT_SKIP;
goto out;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] selftests: x86: test_shadow_stack: return KSFT_SKIP when test is skipped
2026-03-01 1:47 [PATCH] selftests: x86: test_shadow_stack: return KSFT_SKIP when test is skipped Aleksei Oladko
@ 2026-03-15 19:08 ` Aleksei Oladko
2026-03-19 16:38 ` Pavel Tikhomirov
1 sibling, 0 replies; 6+ messages in thread
From: Aleksei Oladko @ 2026-03-15 19:08 UTC (permalink / raw)
To: Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
Shuah Khan, H . Peter Anvin
Cc: linux-kselftest, linux-kernel
Hi,
I just wanted to gently follow up to see if you had a chance to review
this patch. Please let me know if there is anything I can clarify or
improve.
Thanks!
On 3/1/26 2:47 AM, Aleksei Oladko wrote:
> test_shadow_stack prints a message indicating that the test is
> skipped in some cases, but still returns 1. This causes the test
> to be reported as failed instead of skipped.
>
> Return KSFT_SKIP in the skip path so the result is reported
> correctly.
>
> Signed-off-by: Aleksei Oladko <aleksey.oladko@virtuozzo.com>
> ---
> tools/testing/selftests/x86/test_shadow_stack.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/tools/testing/selftests/x86/test_shadow_stack.c b/tools/testing/selftests/x86/test_shadow_stack.c
> index 21af54d5f4ea..1747ea4cb725 100644
> --- a/tools/testing/selftests/x86/test_shadow_stack.c
> +++ b/tools/testing/selftests/x86/test_shadow_stack.c
> @@ -35,6 +35,7 @@
> #include <sys/signal.h>
> #include <linux/elf.h>
> #include <linux/perf_event.h>
> +#include "kselftest.h"
>
> /*
> * Define the ABI defines if needed, so people can run the tests
> @@ -981,7 +982,7 @@ int main(int argc, char *argv[])
>
> if (ARCH_PRCTL(ARCH_SHSTK_ENABLE, ARCH_SHSTK_SHSTK)) {
> printf("[SKIP]\tCould not enable Shadow stack\n");
> - return 1;
> + return KSFT_SKIP;
> }
>
> if (ARCH_PRCTL(ARCH_SHSTK_DISABLE, ARCH_SHSTK_SHSTK)) {
> @@ -991,12 +992,12 @@ int main(int argc, char *argv[])
>
> if (ARCH_PRCTL(ARCH_SHSTK_ENABLE, ARCH_SHSTK_SHSTK)) {
> printf("[SKIP]\tCould not re-enable Shadow stack\n");
> - return 1;
> + return KSFT_SKIP;
> }
>
> if (ARCH_PRCTL(ARCH_SHSTK_ENABLE, ARCH_SHSTK_WRSS)) {
> printf("[SKIP]\tCould not enable WRSS\n");
> - ret = 1;
> + ret = KSFT_SKIP;
> goto out;
> }
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] selftests: x86: test_shadow_stack: return KSFT_SKIP when test is skipped
2026-03-01 1:47 [PATCH] selftests: x86: test_shadow_stack: return KSFT_SKIP when test is skipped Aleksei Oladko
2026-03-15 19:08 ` Aleksei Oladko
@ 2026-03-19 16:38 ` Pavel Tikhomirov
2026-03-19 17:42 ` Edgecombe, Rick P
1 sibling, 1 reply; 6+ messages in thread
From: Pavel Tikhomirov @ 2026-03-19 16:38 UTC (permalink / raw)
To: Aleksei Oladko
Cc: linux-kselftest, linux-kernel, H . Peter Anvin, Shuah Khan, x86,
Borislav Petkov, Dave Hansen, Ingo Molnar, Thomas Gleixner,
Rick Edgecombe
Adding original author to CC: Rick Edgecombe <rick.p.edgecombe@intel.com>.
Maybe he has some insight on why we have this SKIP / return 1 inconsistency.
On 3/1/26 02:47, Aleksei Oladko wrote:
> test_shadow_stack prints a message indicating that the test is
> skipped in some cases, but still returns 1. This causes the test
> to be reported as failed instead of skipped.
>
> Return KSFT_SKIP in the skip path so the result is reported
> correctly.
Should we also return KSFT_SKIP in other 3 SKIP paths, which currently return 0?
I guess that means that those skips are currently reported as success, right?
>
> Signed-off-by: Aleksei Oladko <aleksey.oladko@virtuozzo.com>
> ---
> tools/testing/selftests/x86/test_shadow_stack.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/tools/testing/selftests/x86/test_shadow_stack.c b/tools/testing/selftests/x86/test_shadow_stack.c
> index 21af54d5f4ea..1747ea4cb725 100644
> --- a/tools/testing/selftests/x86/test_shadow_stack.c
> +++ b/tools/testing/selftests/x86/test_shadow_stack.c
> @@ -35,6 +35,7 @@
> #include <sys/signal.h>
> #include <linux/elf.h>
> #include <linux/perf_event.h>
> +#include "kselftest.h"
>
> /*
> * Define the ABI defines if needed, so people can run the tests
> @@ -981,7 +982,7 @@ int main(int argc, char *argv[])
>
> if (ARCH_PRCTL(ARCH_SHSTK_ENABLE, ARCH_SHSTK_SHSTK)) {
> printf("[SKIP]\tCould not enable Shadow stack\n");
> - return 1;
> + return KSFT_SKIP;
> }
>
> if (ARCH_PRCTL(ARCH_SHSTK_DISABLE, ARCH_SHSTK_SHSTK)) {
> @@ -991,12 +992,12 @@ int main(int argc, char *argv[])
>
> if (ARCH_PRCTL(ARCH_SHSTK_ENABLE, ARCH_SHSTK_SHSTK)) {
> printf("[SKIP]\tCould not re-enable Shadow stack\n");
> - return 1;
> + return KSFT_SKIP;
> }
>
> if (ARCH_PRCTL(ARCH_SHSTK_ENABLE, ARCH_SHSTK_WRSS)) {
> printf("[SKIP]\tCould not enable WRSS\n");
> - ret = 1;
> + ret = KSFT_SKIP;
> goto out;
> }
>
--
Best regards, Pavel Tikhomirov
Senior Software Developer, Virtuozzo.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] selftests: x86: test_shadow_stack: return KSFT_SKIP when test is skipped
2026-03-19 16:38 ` Pavel Tikhomirov
@ 2026-03-19 17:42 ` Edgecombe, Rick P
2026-03-20 9:52 ` Pavel Tikhomirov
0 siblings, 1 reply; 6+ messages in thread
From: Edgecombe, Rick P @ 2026-03-19 17:42 UTC (permalink / raw)
To: aleksey.oladko@virtuozzo.com, ptikhomirov@virtuozzo.com
Cc: x86@kernel.org, bp@alien8.de, hpa@zytor.com, mingo@redhat.com,
linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org,
tglx@kernel.org, shuah@kernel.org, dave.hansen@linux.intel.com
On Thu, 2026-03-19 at 17:38 +0100, Pavel Tikhomirov wrote:
> Adding original author to CC: Rick Edgecombe <rick.p.edgecombe@intel.com>.
>
> Maybe he has some insight on why we have this SKIP / return 1 inconsistency.
>
> On 3/1/26 02:47, Aleksei Oladko wrote:
> > test_shadow_stack prints a message indicating that the test is
> > skipped in some cases, but still returns 1. This causes the test
> > to be reported as failed instead of skipped.
> >
> > Return KSFT_SKIP in the skip path so the result is reported
> > correctly.
>
> Should we also return KSFT_SKIP in other 3 SKIP paths, which currently return 0?
> I guess that means that those skips are currently reported as success, right?
Oh, yea I think the first skip, "Could not enable Shadow stack", should return
0, but the rest of them should succeed if shadow stack gets enabled, so they are
more indicative of kernel issues. The "[SKIP]" message is wrong for those. If
you want to leave the error code as 1 for them, I can send a patch to correct
the message. But please feel welcome to fix the message part up too.
Also, "Could not re-enable Shadow stack" is a prelude to a crash. The test can't
return from main if shadow stack is enabled because the shadow stack won't
match. Functionally it makes no difference because it will crash, but it is a
sign that there is something wrong with the kernel. So having 1 there will have
the code make more sense.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] selftests: x86: test_shadow_stack: return KSFT_SKIP when test is skipped
2026-03-19 17:42 ` Edgecombe, Rick P
@ 2026-03-20 9:52 ` Pavel Tikhomirov
2026-03-20 17:23 ` Edgecombe, Rick P
0 siblings, 1 reply; 6+ messages in thread
From: Pavel Tikhomirov @ 2026-03-20 9:52 UTC (permalink / raw)
To: Edgecombe, Rick P, aleksey.oladko@virtuozzo.com
Cc: x86@kernel.org, bp@alien8.de, hpa@zytor.com, mingo@redhat.com,
linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org,
tglx@kernel.org, shuah@kernel.org, dave.hansen@linux.intel.com
On 3/19/26 18:42, Edgecombe, Rick P wrote:
> On Thu, 2026-03-19 at 17:38 +0100, Pavel Tikhomirov wrote:
>> Adding original author to CC: Rick Edgecombe <rick.p.edgecombe@intel.com>.
>>
>> Maybe he has some insight on why we have this SKIP / return 1 inconsistency.
>>
>> On 3/1/26 02:47, Aleksei Oladko wrote:
>>> test_shadow_stack prints a message indicating that the test is
>>> skipped in some cases, but still returns 1. This causes the test
>>> to be reported as failed instead of skipped.
>>>
>>> Return KSFT_SKIP in the skip path so the result is reported
>>> correctly.
>>
>> Should we also return KSFT_SKIP in other 3 SKIP paths, which currently return 0?
>> I guess that means that those skips are currently reported as success, right?
>
> Oh, yea I think the first skip, "Could not enable Shadow stack", should return
> 0, but the rest of them should succeed if shadow stack gets enabled, so they are
> more indicative of kernel issues. The "[SKIP]" message is wrong for those. If
> you want to leave the error code as 1 for them, I can send a patch to correct
> the message. But please feel welcome to fix the message part up too.
>
> Also, "Could not re-enable Shadow stack" is a prelude to a crash. The test can't
> return from main if shadow stack is enabled because the shadow stack won't
> match. Functionally it makes no difference because it will crash, but it is a
> sign that there is something wrong with the kernel. So having 1 there will have
> the code make more sense.
>
I think it should be something like below then. Aleksey - feel free to incorporate the below change in your patch if that is OK.
Note: I also added earlier exits after failures in ARCH_SHSTK_DISABLE and test_ptrace. Plus made test_uretprobe only skip when we print SKIP message, else fail.
diff --git a/tools/testing/selftests/x86/test_shadow_stack.c b/tools/testing/selftests/x86/test_shadow_stack.c
index 21af54d5f4ea..3a0019bf65df 100644
--- a/tools/testing/selftests/x86/test_shadow_stack.c
+++ b/tools/testing/selftests/x86/test_shadow_stack.c
@@ -36,6 +36,8 @@
#include <linux/elf.h>
#include <linux/perf_event.h>
+#include "kselftest.h"
+
/*
* Define the ABI defines if needed, so people can run the tests
* without building the headers.
@@ -64,7 +66,7 @@
int main(int argc, char *argv[])
{
printf("[SKIP]\tCompiler does not support CET.\n");
- return 0;
+ return KSFT_SKIP;
}
#else
void write_shstk(unsigned long *addr, unsigned long val)
@@ -820,9 +822,11 @@ static int test_uretprobe(void)
type = determine_uprobe_perf_type();
if (type < 0) {
- if (type == -ENOENT)
+ if (type == -ENOENT) {
printf("[SKIP]\tUretprobe test, uprobes are not available\n");
- return 0;
+ return 0;
+ }
+ return 1;
}
offset = get_uprobe_offset(uretprobe_trigger);
@@ -981,21 +985,21 @@ int main(int argc, char *argv[])
if (ARCH_PRCTL(ARCH_SHSTK_ENABLE, ARCH_SHSTK_SHSTK)) {
printf("[SKIP]\tCould not enable Shadow stack\n");
- return 1;
+ return KSFT_SKIP;
}
if (ARCH_PRCTL(ARCH_SHSTK_DISABLE, ARCH_SHSTK_SHSTK)) {
- ret = 1;
printf("[FAIL]\tDisabling shadow stack failed\n");
+ return 1;
}
if (ARCH_PRCTL(ARCH_SHSTK_ENABLE, ARCH_SHSTK_SHSTK)) {
- printf("[SKIP]\tCould not re-enable Shadow stack\n");
+ printf("[FAIL]\tCould not re-enable Shadow stack\n");
return 1;
}
if (ARCH_PRCTL(ARCH_SHSTK_ENABLE, ARCH_SHSTK_WRSS)) {
- printf("[SKIP]\tCould not enable WRSS\n");
+ printf("[FAIL]\tCould not enable WRSS\n");
ret = 1;
goto out;
}
@@ -1057,6 +1061,7 @@ int main(int argc, char *argv[])
if (test_ptrace()) {
ret = 1;
printf("[FAIL]\tptrace test\n");
+ goto out;
}
if (test_32bit()) {
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] selftests: x86: test_shadow_stack: return KSFT_SKIP when test is skipped
2026-03-20 9:52 ` Pavel Tikhomirov
@ 2026-03-20 17:23 ` Edgecombe, Rick P
0 siblings, 0 replies; 6+ messages in thread
From: Edgecombe, Rick P @ 2026-03-20 17:23 UTC (permalink / raw)
To: aleksey.oladko@virtuozzo.com, ptikhomirov@virtuozzo.com
Cc: dave.hansen@linux.intel.com, x86@kernel.org, bp@alien8.de,
hpa@zytor.com, shuah@kernel.org, linux-kernel@vger.kernel.org,
linux-kselftest@vger.kernel.org, tglx@kernel.org,
mingo@redhat.com
On Fri, 2026-03-20 at 10:52 +0100, Pavel Tikhomirov wrote:
> if (ARCH_PRCTL(ARCH_SHSTK_ENABLE, ARCH_SHSTK_SHSTK)) {
> - printf("[SKIP]\tCould not re-enable Shadow stack\n");
> + printf("[FAIL]\tCould not re-enable Shadow stack\n");
> return 1;
> }
>
> if (ARCH_PRCTL(ARCH_SHSTK_ENABLE, ARCH_SHSTK_WRSS)) {
> - printf("[SKIP]\tCould not enable WRSS\n");
> + printf("[FAIL]\tCould not enable WRSS\n");
> ret = 1;
> goto out;
> }
> @@ -1057,6 +1061,7 @@ int main(int argc, char *argv[])
> if (test_ptrace()) {
> ret = 1;
> printf("[FAIL]\tptrace test\n");
> + goto out;
> }
Seems fine. The rest of the test could continue, but it's probably better to
have everything match. This whole function could be cleaned up actually, to be
more understandable and cleaner, so don't want to dig any further.
>
> if (test_32bit()) {
The rest looks good, thanks!
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-03-20 17:23 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-01 1:47 [PATCH] selftests: x86: test_shadow_stack: return KSFT_SKIP when test is skipped Aleksei Oladko
2026-03-15 19:08 ` Aleksei Oladko
2026-03-19 16:38 ` Pavel Tikhomirov
2026-03-19 17:42 ` Edgecombe, Rick P
2026-03-20 9:52 ` Pavel Tikhomirov
2026-03-20 17:23 ` Edgecombe, Rick P
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox