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 <[email protected]>.
>>
>> 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()) {
