* [PATCH] selftests/landlock: Fix screwed up pointers in the scoped_signal_test
@ 2026-07-09 16:43 Thomas Huth
2026-07-10 11:09 ` Mickaël Salaün
0 siblings, 1 reply; 2+ messages in thread
From: Thomas Huth @ 2026-07-09 16:43 UTC (permalink / raw)
To: Mickaël Salaün, Günther Noack,
linux-security-module
Cc: Shuah Khan, linux-kselftest, linux-kernel, rbriggs
From: Thomas Huth <thuth@redhat.com>
The scoped_signal_test uses pthread_join(..., (void **)&ret)) in
a couple of places, i.e. the return value of the thread is stored
in the shape of a "void *" into the memory location of &ret.
Pointers are 64-bit on modern computers, but the ret variable is
declared as a simple "enum thread_return" which is only 32 bits.
So the pthread_join() will overflow the ret variable by 4 byte.
The problem is very visible on big endian systems like s390x
where the test is failing: The least significant byte that carries
the return code of the thread is not written into the ret variable
here, but somewhere else in the stack frame, so the comparison
for the right return code is failing here.
Fix it by getting rid of the enum and defining the THREAD_* constants
and "ret" variables as proper "void *" pointers. This way we can
also get rid of some ugly (void *) castings in a couple of spots.
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
.../selftests/landlock/scoped_signal_test.c | 44 +++++++++----------
1 file changed, 21 insertions(+), 23 deletions(-)
diff --git a/tools/testing/selftests/landlock/scoped_signal_test.c b/tools/testing/selftests/landlock/scoped_signal_test.c
index f24f2c28f62e5..58d25157fe781 100644
--- a/tools/testing/selftests/landlock/scoped_signal_test.c
+++ b/tools/testing/selftests/landlock/scoped_signal_test.c
@@ -249,12 +249,10 @@ TEST_F(scoped_domains, check_access_signal)
_metadata->exit_code = KSFT_FAIL;
}
-enum thread_return {
- THREAD_INVALID = 0,
- THREAD_SUCCESS = 1,
- THREAD_ERROR = 2,
- THREAD_TEST_FAILED = 3,
-};
+#define THREAD_INVALID ((void *)0)
+#define THREAD_SUCCESS ((void *)1)
+#define THREAD_ERROR ((void *)2)
+#define THREAD_TEST_FAILED ((void *)3)
static void *thread_sync(void *arg)
{
@@ -262,15 +260,15 @@ static void *thread_sync(void *arg)
char buf;
if (read(pipe_read, &buf, 1) != 1)
- return (void *)THREAD_ERROR;
+ return THREAD_ERROR;
- return (void *)THREAD_SUCCESS;
+ return THREAD_SUCCESS;
}
TEST(signal_scoping_thread_before)
{
pthread_t no_sandbox_thread;
- enum thread_return ret = THREAD_INVALID;
+ void *ret = THREAD_INVALID;
int thread_pipe[2];
drop_caps(_metadata);
@@ -285,7 +283,7 @@ TEST(signal_scoping_thread_before)
EXPECT_EQ(0, pthread_kill(no_sandbox_thread, 0));
EXPECT_EQ(1, write(thread_pipe[1], ".", 1));
- EXPECT_EQ(0, pthread_join(no_sandbox_thread, (void **)&ret));
+ EXPECT_EQ(0, pthread_join(no_sandbox_thread, &ret));
EXPECT_EQ(THREAD_SUCCESS, ret);
EXPECT_EQ(0, close(thread_pipe[0]));
@@ -295,7 +293,7 @@ TEST(signal_scoping_thread_before)
TEST(signal_scoping_thread_after)
{
pthread_t scoped_thread;
- enum thread_return ret = THREAD_INVALID;
+ void *ret = THREAD_INVALID;
int thread_pipe[2];
drop_caps(_metadata);
@@ -310,7 +308,7 @@ TEST(signal_scoping_thread_after)
EXPECT_EQ(0, pthread_kill(scoped_thread, 0));
EXPECT_EQ(1, write(thread_pipe[1], ".", 1));
- EXPECT_EQ(0, pthread_join(scoped_thread, (void **)&ret));
+ EXPECT_EQ(0, pthread_join(scoped_thread, &ret));
EXPECT_EQ(THREAD_SUCCESS, ret);
EXPECT_EQ(0, close(thread_pipe[0]));
@@ -327,20 +325,20 @@ void *thread_setuid(void *ptr)
char buf;
if (read(arg->pipe_read, &buf, 1) != 1)
- return (void *)THREAD_ERROR;
+ return THREAD_ERROR;
/* libc's setuid() should update all thread's credentials. */
if (getuid() != arg->new_uid)
- return (void *)THREAD_TEST_FAILED;
+ return THREAD_TEST_FAILED;
- return (void *)THREAD_SUCCESS;
+ return THREAD_SUCCESS;
}
TEST(signal_scoping_thread_setuid)
{
struct thread_setuid_args arg;
pthread_t no_sandbox_thread;
- enum thread_return ret = THREAD_INVALID;
+ void *ret = THREAD_INVALID;
int pipe_parent[2];
int prev_uid;
@@ -367,7 +365,7 @@ TEST(signal_scoping_thread_setuid)
EXPECT_EQ(arg.new_uid, getuid());
EXPECT_EQ(1, write(pipe_parent[1], ".", 1));
- EXPECT_EQ(0, pthread_join(no_sandbox_thread, (void **)&ret));
+ EXPECT_EQ(0, pthread_join(no_sandbox_thread, &ret));
EXPECT_EQ(THREAD_SUCCESS, ret);
clear_cap(_metadata, CAP_SETUID);
@@ -667,20 +665,20 @@ static void *thread_setown_scoped(void *arg)
ruleset_fd =
landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
if (ruleset_fd < 0)
- return (void *)THREAD_ERROR;
+ return THREAD_ERROR;
if (prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0) ||
landlock_restrict_self(ruleset_fd, 0)) {
close(ruleset_fd);
- return (void *)THREAD_ERROR;
+ return THREAD_ERROR;
}
close(ruleset_fd);
/* Makes this process group own the SIGIO source. */
if (fcntl(fd, F_SETSIG, SIGURG) || fcntl(fd, F_SETOWN, -getpgrp()) ||
fcntl(fd, F_SETFL, O_ASYNC))
- return (void *)THREAD_ERROR;
+ return THREAD_ERROR;
- return (void *)THREAD_SUCCESS;
+ return THREAD_SUCCESS;
}
/*
@@ -702,7 +700,7 @@ TEST(sigio_to_pgid_self)
{
int trigger[2];
pthread_t thread;
- enum thread_return ret = THREAD_INVALID;
+ void *ret = THREAD_INVALID;
int i;
drop_caps(_metadata);
@@ -722,7 +720,7 @@ TEST(sigio_to_pgid_self)
*/
ASSERT_EQ(0, pthread_create(&thread, NULL, thread_setown_scoped,
&trigger[0]));
- ASSERT_EQ(0, pthread_join(thread, (void **)&ret));
+ ASSERT_EQ(0, pthread_join(thread, &ret));
ASSERT_EQ(THREAD_SUCCESS, ret);
/* Fans SIGURG out to the process group. */
--
2.55.0
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] selftests/landlock: Fix screwed up pointers in the scoped_signal_test
2026-07-09 16:43 [PATCH] selftests/landlock: Fix screwed up pointers in the scoped_signal_test Thomas Huth
@ 2026-07-10 11:09 ` Mickaël Salaün
0 siblings, 0 replies; 2+ messages in thread
From: Mickaël Salaün @ 2026-07-10 11:09 UTC (permalink / raw)
To: Thomas Huth
Cc: Günther Noack, linux-security-module, Shuah Khan,
linux-kselftest, linux-kernel, rbriggs
Applied (with a Fixes tag), thanks!
On Thu, Jul 09, 2026 at 06:43:40PM +0200, Thomas Huth wrote:
> From: Thomas Huth <thuth@redhat.com>
>
> The scoped_signal_test uses pthread_join(..., (void **)&ret)) in
> a couple of places, i.e. the return value of the thread is stored
> in the shape of a "void *" into the memory location of &ret.
> Pointers are 64-bit on modern computers, but the ret variable is
> declared as a simple "enum thread_return" which is only 32 bits.
> So the pthread_join() will overflow the ret variable by 4 byte.
>
> The problem is very visible on big endian systems like s390x
> where the test is failing: The least significant byte that carries
> the return code of the thread is not written into the ret variable
> here, but somewhere else in the stack frame, so the comparison
> for the right return code is failing here.
>
> Fix it by getting rid of the enum and defining the THREAD_* constants
> and "ret" variables as proper "void *" pointers. This way we can
> also get rid of some ugly (void *) castings in a couple of spots.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
> .../selftests/landlock/scoped_signal_test.c | 44 +++++++++----------
> 1 file changed, 21 insertions(+), 23 deletions(-)
>
> diff --git a/tools/testing/selftests/landlock/scoped_signal_test.c b/tools/testing/selftests/landlock/scoped_signal_test.c
> index f24f2c28f62e5..58d25157fe781 100644
> --- a/tools/testing/selftests/landlock/scoped_signal_test.c
> +++ b/tools/testing/selftests/landlock/scoped_signal_test.c
> @@ -249,12 +249,10 @@ TEST_F(scoped_domains, check_access_signal)
> _metadata->exit_code = KSFT_FAIL;
> }
>
> -enum thread_return {
> - THREAD_INVALID = 0,
> - THREAD_SUCCESS = 1,
> - THREAD_ERROR = 2,
> - THREAD_TEST_FAILED = 3,
> -};
I just added clang-format markups.
> +#define THREAD_INVALID ((void *)0)
> +#define THREAD_SUCCESS ((void *)1)
> +#define THREAD_ERROR ((void *)2)
> +#define THREAD_TEST_FAILED ((void *)3)
>
> static void *thread_sync(void *arg)
> {
> @@ -262,15 +260,15 @@ static void *thread_sync(void *arg)
> char buf;
>
> if (read(pipe_read, &buf, 1) != 1)
> - return (void *)THREAD_ERROR;
> + return THREAD_ERROR;
>
> - return (void *)THREAD_SUCCESS;
> + return THREAD_SUCCESS;
> }
>
> TEST(signal_scoping_thread_before)
> {
> pthread_t no_sandbox_thread;
> - enum thread_return ret = THREAD_INVALID;
> + void *ret = THREAD_INVALID;
> int thread_pipe[2];
>
> drop_caps(_metadata);
> @@ -285,7 +283,7 @@ TEST(signal_scoping_thread_before)
> EXPECT_EQ(0, pthread_kill(no_sandbox_thread, 0));
> EXPECT_EQ(1, write(thread_pipe[1], ".", 1));
>
> - EXPECT_EQ(0, pthread_join(no_sandbox_thread, (void **)&ret));
> + EXPECT_EQ(0, pthread_join(no_sandbox_thread, &ret));
> EXPECT_EQ(THREAD_SUCCESS, ret);
>
> EXPECT_EQ(0, close(thread_pipe[0]));
> @@ -295,7 +293,7 @@ TEST(signal_scoping_thread_before)
> TEST(signal_scoping_thread_after)
> {
> pthread_t scoped_thread;
> - enum thread_return ret = THREAD_INVALID;
> + void *ret = THREAD_INVALID;
> int thread_pipe[2];
>
> drop_caps(_metadata);
> @@ -310,7 +308,7 @@ TEST(signal_scoping_thread_after)
> EXPECT_EQ(0, pthread_kill(scoped_thread, 0));
> EXPECT_EQ(1, write(thread_pipe[1], ".", 1));
>
> - EXPECT_EQ(0, pthread_join(scoped_thread, (void **)&ret));
> + EXPECT_EQ(0, pthread_join(scoped_thread, &ret));
> EXPECT_EQ(THREAD_SUCCESS, ret);
>
> EXPECT_EQ(0, close(thread_pipe[0]));
> @@ -327,20 +325,20 @@ void *thread_setuid(void *ptr)
> char buf;
>
> if (read(arg->pipe_read, &buf, 1) != 1)
> - return (void *)THREAD_ERROR;
> + return THREAD_ERROR;
>
> /* libc's setuid() should update all thread's credentials. */
> if (getuid() != arg->new_uid)
> - return (void *)THREAD_TEST_FAILED;
> + return THREAD_TEST_FAILED;
>
> - return (void *)THREAD_SUCCESS;
> + return THREAD_SUCCESS;
> }
>
> TEST(signal_scoping_thread_setuid)
> {
> struct thread_setuid_args arg;
> pthread_t no_sandbox_thread;
> - enum thread_return ret = THREAD_INVALID;
> + void *ret = THREAD_INVALID;
> int pipe_parent[2];
> int prev_uid;
>
> @@ -367,7 +365,7 @@ TEST(signal_scoping_thread_setuid)
> EXPECT_EQ(arg.new_uid, getuid());
> EXPECT_EQ(1, write(pipe_parent[1], ".", 1));
>
> - EXPECT_EQ(0, pthread_join(no_sandbox_thread, (void **)&ret));
> + EXPECT_EQ(0, pthread_join(no_sandbox_thread, &ret));
> EXPECT_EQ(THREAD_SUCCESS, ret);
>
> clear_cap(_metadata, CAP_SETUID);
> @@ -667,20 +665,20 @@ static void *thread_setown_scoped(void *arg)
> ruleset_fd =
> landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
> if (ruleset_fd < 0)
> - return (void *)THREAD_ERROR;
> + return THREAD_ERROR;
> if (prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0) ||
> landlock_restrict_self(ruleset_fd, 0)) {
> close(ruleset_fd);
> - return (void *)THREAD_ERROR;
> + return THREAD_ERROR;
> }
> close(ruleset_fd);
>
> /* Makes this process group own the SIGIO source. */
> if (fcntl(fd, F_SETSIG, SIGURG) || fcntl(fd, F_SETOWN, -getpgrp()) ||
> fcntl(fd, F_SETFL, O_ASYNC))
> - return (void *)THREAD_ERROR;
> + return THREAD_ERROR;
>
> - return (void *)THREAD_SUCCESS;
> + return THREAD_SUCCESS;
> }
>
> /*
> @@ -702,7 +700,7 @@ TEST(sigio_to_pgid_self)
> {
> int trigger[2];
> pthread_t thread;
> - enum thread_return ret = THREAD_INVALID;
> + void *ret = THREAD_INVALID;
> int i;
>
> drop_caps(_metadata);
> @@ -722,7 +720,7 @@ TEST(sigio_to_pgid_self)
> */
> ASSERT_EQ(0, pthread_create(&thread, NULL, thread_setown_scoped,
> &trigger[0]));
> - ASSERT_EQ(0, pthread_join(thread, (void **)&ret));
> + ASSERT_EQ(0, pthread_join(thread, &ret));
> ASSERT_EQ(THREAD_SUCCESS, ret);
>
> /* Fans SIGURG out to the process group. */
> --
> 2.55.0
>
>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-07-10 11:10 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-09 16:43 [PATCH] selftests/landlock: Fix screwed up pointers in the scoped_signal_test Thomas Huth
2026-07-10 11:09 ` Mickaël Salaün
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox