tools/testing/selftests/x86/test_shadow_stack.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
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.0Adding 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.
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.
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()) {
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!
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;
> }
>
© 2016 - 2026 Red Hat, Inc.