Applied (with a Fixes tag), thanks!
On Thu, Jul 09, 2026 at 06:43:40PM +0200, Thomas Huth wrote:
> From: Thomas Huth <[email protected]>
>
> 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 <[email protected]>
> ---
> .../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
>
>
