- Linux-kselftest-mirror - lists.linaro.org

[PATCH 0/4] selftests/sgx: Early enclave loading error path fixes

by Reinette Chatre

Hi Everybody, Please find included a few fixes that address problems encountered after venturing into the enclave loading error handling code of the SGX selftests. Reinette Reinette Chatre (4): selftests/sgx: Fix segfault upon early test failure selftests/sgx: Do not attempt enclave build without valid enclave selftests/sgx: Ensure enclave data available during debug print selftests/sgx: Remove extra newlines in test output tools/testing/selftests/sgx/load.c | 9 +++++---- tools/testing/selftests/sgx/main.c | 9 +++++---- 2 files changed, 10 insertions(+), 8 deletions(-) -- 2.25.1

3 years, 4 months

3
17
0 0

[PATCH v2] selftests/seccomp: Fix seccomp failure by adding missing headers

by Sherry Yang

seccomp_bpf failed on tests 47 global.user_notification_filter_empty and 48 global.user_notification_filter_empty_threaded when it's tested on updated kernel but with old kernel headers. Because old kernel headers don't have definition of macro __NR_clone3 which is required for these two tests. Since under selftests/, we can install headers once for all tests (the default INSTALL_HDR_PATH is usr/include), fix it by adding usr/include to the list of directories to be searched. Use "-isystem" to indicate it's a system directory as the real kernel headers directories are. Signed-off-by: Sherry Yang <sherry.yang(a)oracle.com> Tested-by: Sherry Yang <sherry.yang(a)oracle.com> --- tools/testing/selftests/seccomp/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/testing/selftests/seccomp/Makefile b/tools/testing/selftests/seccomp/Makefile index 0ebfe8b0e147..585f7a0c10cb 100644 --- a/tools/testing/selftests/seccomp/Makefile +++ b/tools/testing/selftests/seccomp/Makefile @@ -1,5 +1,5 @@ # SPDX-License-Identifier: GPL-2.0 -CFLAGS += -Wl,-no-as-needed -Wall +CFLAGS += -Wl,-no-as-needed -Wall -isystem ../../../../usr/include/ LDFLAGS += -lpthread TEST_GEN_PROGS := seccomp_bpf seccomp_benchmark -- 2.31.1

3 years, 4 months

4
10
0 0

[PATCH v3] selftests/seccomp: Fix seccomp failure by adding missing headers

by Sherry Yang

seccomp_bpf failed on tests 47 global.user_notification_filter_empty and 48 global.user_notification_filter_empty_threaded when it's tested on updated kernel but with old kernel headers. Because old kernel headers don't have definition of macro __NR_clone3 which is required for these two tests. Use KHDR_INCLUDES to correctly reach the installed headers. Signed-off-by: Sherry Yang <sherry.yang(a)oracle.com> Tested-by: Sherry Yang <sherry.yang(a)oracle.com> --- tools/testing/selftests/seccomp/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/testing/selftests/seccomp/Makefile b/tools/testing/selftests/seccomp/Makefile index 0ebfe8b0e147..7eaed95ba4b3 100644 --- a/tools/testing/selftests/seccomp/Makefile +++ b/tools/testing/selftests/seccomp/Makefile @@ -1,5 +1,5 @@ # SPDX-License-Identifier: GPL-2.0 -CFLAGS += -Wl,-no-as-needed -Wall +CFLAGS += -Wl,-no-as-needed -Wall $(KHDR_INCLUDES) LDFLAGS += -lpthread TEST_GEN_PROGS := seccomp_bpf seccomp_benchmark -- 2.31.1

3 years, 4 months

2
1
0 0

[PATCH AUTOSEL 5.10 11/23] selftests: fixup build warnings in pidfd / clone3 tests

by Sasha Levin

From: Axel Rasmussen <axelrasmussen(a)google.com> [ Upstream commit e2aa5e650b07693477dff554053605976789fd68 ] These are some trivial fixups, which were needed to build the tests with clang and -Werror. The following issues are fixed: - Remove various unused variables. - In child_poll_leader_exit_test, clang isn't smart enough to realize syscall(SYS_exit, 0) won't return, so it complains we never return from a non-void function. Add an extra exit(0) to appease it. - In test_pidfd_poll_leader_exit, ret may be branched on despite being uninitialized, if we have !use_waitpid. Initialize it to zero to get the right behavior in that case. Signed-off-by: Axel Rasmussen <axelrasmussen(a)google.com> Acked-by: Christian Brauner <brauner(a)kernel.org> Signed-off-by: Shuah Khan <skhan(a)linuxfoundation.org> Signed-off-by: Sasha Levin <sashal(a)kernel.org> --- tools/testing/selftests/clone3/clone3.c | 2 -- tools/testing/selftests/pidfd/pidfd_test.c | 6 +++--- tools/testing/selftests/pidfd/pidfd_wait.c | 5 ++--- 3 files changed, 5 insertions(+), 8 deletions(-) diff --git a/tools/testing/selftests/clone3/clone3.c b/tools/testing/selftests/clone3/clone3.c index 076cf4325f783..cd4582129c7d6 100644 --- a/tools/testing/selftests/clone3/clone3.c +++ b/tools/testing/selftests/clone3/clone3.c @@ -126,8 +126,6 @@ static void test_clone3(uint64_t flags, size_t size, int expected, int main(int argc, char *argv[]) { - pid_t pid; - uid_t uid = getuid(); ksft_print_header(); diff --git a/tools/testing/selftests/pidfd/pidfd_test.c b/tools/testing/selftests/pidfd/pidfd_test.c index 529eb700ac26a..9a2d64901d591 100644 --- a/tools/testing/selftests/pidfd/pidfd_test.c +++ b/tools/testing/selftests/pidfd/pidfd_test.c @@ -441,7 +441,6 @@ static void test_pidfd_poll_exec(int use_waitpid) { int pid, pidfd = 0; int status, ret; - pthread_t t1; time_t prog_start = time(NULL); const char *test_name = "pidfd_poll check for premature notification on child thread exec"; @@ -500,13 +499,14 @@ static int child_poll_leader_exit_test(void *args) */ *child_exit_secs = time(NULL); syscall(SYS_exit, 0); + /* Never reached, but appeases compiler thinking we should return. */ + exit(0); } static void test_pidfd_poll_leader_exit(int use_waitpid) { int pid, pidfd = 0; - int status, ret; - time_t prog_start = time(NULL); + int status, ret = 0; const char *test_name = "pidfd_poll check for premature notification on non-empty" "group leader exit"; diff --git a/tools/testing/selftests/pidfd/pidfd_wait.c b/tools/testing/selftests/pidfd/pidfd_wait.c index be2943f072f60..17999e082aa71 100644 --- a/tools/testing/selftests/pidfd/pidfd_wait.c +++ b/tools/testing/selftests/pidfd/pidfd_wait.c @@ -39,7 +39,7 @@ static int sys_waitid(int which, pid_t pid, siginfo_t *info, int options, TEST(wait_simple) { - int pidfd = -1, status = 0; + int pidfd = -1; pid_t parent_tid = -1; struct clone_args args = { .parent_tid = ptr_to_u64(&parent_tid), @@ -47,7 +47,6 @@ TEST(wait_simple) .flags = CLONE_PIDFD | CLONE_PARENT_SETTID, .exit_signal = SIGCHLD, }; - int ret; pid_t pid; siginfo_t info = { .si_signo = 0, @@ -88,7 +87,7 @@ TEST(wait_simple) TEST(wait_states) { - int pidfd = -1, status = 0; + int pidfd = -1; pid_t parent_tid = -1; struct clone_args args = { .parent_tid = ptr_to_u64(&parent_tid), -- 2.34.1

3 years, 4 months

1
0
0 0

[PATCH AUTOSEL 5.10 10/23] pidfd: fix test failure due to stack overflow on some arches

by Sasha Levin

From: Axel Rasmussen <axelrasmussen(a)google.com> [ Upstream commit 4cbd93c3c110447adc66cb67c08af21f939ae2d7 ] When running the pidfd_fdinfo_test on arm64, it fails for me. After some digging, the reason is that the child exits due to SIGBUS, because it overflows the 1024 byte stack we've reserved for it. To fix the issue, increase the stack size to 8192 bytes (this number is somewhat arbitrary, and was arrived at through experimentation -- I kept doubling until the failure no longer occurred). Also, let's make the issue easier to debug. wait_for_pid() returns an ambiguous value: it may return -1 in all of these cases: 1. waitpid() itself returned -1 2. waitpid() returned success, but we found !WIFEXITED(status). 3. The child process exited, but it did so with a -1 exit code. There's no way for the caller to tell the difference. So, at least log which occurred, so the test runner can debug things. While debugging this, I found that we had !WIFEXITED(), because the child exited due to a signal. This seems like a reasonably common case, so also print out whether or not we have WIFSIGNALED(), and the associated WTERMSIG() (if any). This lets us see the SIGBUS I'm fixing clearly when it occurs. Finally, I'm suspicious of allocating the child's stack on our stack. man clone(2) suggests that the correct way to do this is with mmap(), and in particular by setting MAP_STACK. So, switch to doing it that way instead. Signed-off-by: Axel Rasmussen <axelrasmussen(a)google.com> Acked-by: Christian Brauner <brauner(a)kernel.org> Signed-off-by: Shuah Khan <skhan(a)linuxfoundation.org> Signed-off-by: Sasha Levin <sashal(a)kernel.org> --- tools/testing/selftests/pidfd/pidfd.h | 13 ++++++++--- .../selftests/pidfd/pidfd_fdinfo_test.c | 22 +++++++++++++++---- 2 files changed, 28 insertions(+), 7 deletions(-) diff --git a/tools/testing/selftests/pidfd/pidfd.h b/tools/testing/selftests/pidfd/pidfd.h index 01f8d3c0cf2cb..6922d6417e1cf 100644 --- a/tools/testing/selftests/pidfd/pidfd.h +++ b/tools/testing/selftests/pidfd/pidfd.h @@ -68,7 +68,7 @@ #define PIDFD_SKIP 3 #define PIDFD_XFAIL 4 -int wait_for_pid(pid_t pid) +static inline int wait_for_pid(pid_t pid) { int status, ret; @@ -78,13 +78,20 @@ int wait_for_pid(pid_t pid) if (errno == EINTR) goto again; + ksft_print_msg("waitpid returned -1, errno=%d\n", errno); return -1; } - if (!WIFEXITED(status)) + if (!WIFEXITED(status)) { + ksft_print_msg( + "waitpid !WIFEXITED, WIFSIGNALED=%d, WTERMSIG=%d\n", + WIFSIGNALED(status), WTERMSIG(status)); return -1; + } - return WEXITSTATUS(status); + ret = WEXITSTATUS(status); + ksft_print_msg("waitpid WEXITSTATUS=%d\n", ret); + return ret; } static inline int sys_pidfd_open(pid_t pid, unsigned int flags) diff --git a/tools/testing/selftests/pidfd/pidfd_fdinfo_test.c b/tools/testing/selftests/pidfd/pidfd_fdinfo_test.c index 22558524f71c3..3fd8e903118f5 100644 --- a/tools/testing/selftests/pidfd/pidfd_fdinfo_test.c +++ b/tools/testing/selftests/pidfd/pidfd_fdinfo_test.c @@ -12,6 +12,7 @@ #include <string.h> #include <syscall.h> #include <sys/wait.h> +#include <sys/mman.h> #include "pidfd.h" #include "../kselftest.h" @@ -80,7 +81,10 @@ static inline int error_check(struct error *err, const char *test_name) return err->code; } +#define CHILD_STACK_SIZE 8192 + struct child { + char *stack; pid_t pid; int fd; }; @@ -89,17 +93,22 @@ static struct child clone_newns(int (*fn)(void *), void *args, struct error *err) { static int flags = CLONE_PIDFD | CLONE_NEWPID | CLONE_NEWNS | SIGCHLD; - size_t stack_size = 1024; - char *stack[1024] = { 0 }; struct child ret; if (!(flags & CLONE_NEWUSER) && geteuid() != 0) flags |= CLONE_NEWUSER; + ret.stack = mmap(NULL, CHILD_STACK_SIZE, PROT_READ | PROT_WRITE, + MAP_PRIVATE | MAP_ANONYMOUS | MAP_STACK, -1, 0); + if (ret.stack == MAP_FAILED) { + error_set(err, -1, "mmap of stack failed (errno %d)", errno); + return ret; + } + #ifdef __ia64__ - ret.pid = __clone2(fn, stack, stack_size, flags, args, &ret.fd); + ret.pid = __clone2(fn, ret.stack, CHILD_STACK_SIZE, flags, args, &ret.fd); #else - ret.pid = clone(fn, stack + stack_size, flags, args, &ret.fd); + ret.pid = clone(fn, ret.stack + CHILD_STACK_SIZE, flags, args, &ret.fd); #endif if (ret.pid < 0) { @@ -129,6 +138,11 @@ static inline int child_join(struct child *child, struct error *err) else if (r > 0) error_set(err, r, "child %d reported: %d", child->pid, r); + if (munmap(child->stack, CHILD_STACK_SIZE)) { + error_set(err, -1, "munmap of child stack failed (errno %d)", errno); + r = -1; + } + return r; } -- 2.34.1

3 years, 4 months

1
0
0 0

[PATCH AUTOSEL 5.15 16/33] selftests: fixup build warnings in pidfd / clone3 tests

by Sasha Levin

From: Axel Rasmussen <axelrasmussen(a)google.com> [ Upstream commit e2aa5e650b07693477dff554053605976789fd68 ] These are some trivial fixups, which were needed to build the tests with clang and -Werror. The following issues are fixed: - Remove various unused variables. - In child_poll_leader_exit_test, clang isn't smart enough to realize syscall(SYS_exit, 0) won't return, so it complains we never return from a non-void function. Add an extra exit(0) to appease it. - In test_pidfd_poll_leader_exit, ret may be branched on despite being uninitialized, if we have !use_waitpid. Initialize it to zero to get the right behavior in that case. Signed-off-by: Axel Rasmussen <axelrasmussen(a)google.com> Acked-by: Christian Brauner <brauner(a)kernel.org> Signed-off-by: Shuah Khan <skhan(a)linuxfoundation.org> Signed-off-by: Sasha Levin <sashal(a)kernel.org> --- tools/testing/selftests/clone3/clone3.c | 2 -- tools/testing/selftests/pidfd/pidfd_test.c | 6 +++--- tools/testing/selftests/pidfd/pidfd_wait.c | 5 ++--- 3 files changed, 5 insertions(+), 8 deletions(-) diff --git a/tools/testing/selftests/clone3/clone3.c b/tools/testing/selftests/clone3/clone3.c index 076cf4325f783..cd4582129c7d6 100644 --- a/tools/testing/selftests/clone3/clone3.c +++ b/tools/testing/selftests/clone3/clone3.c @@ -126,8 +126,6 @@ static void test_clone3(uint64_t flags, size_t size, int expected, int main(int argc, char *argv[]) { - pid_t pid; - uid_t uid = getuid(); ksft_print_header(); diff --git a/tools/testing/selftests/pidfd/pidfd_test.c b/tools/testing/selftests/pidfd/pidfd_test.c index 529eb700ac26a..9a2d64901d591 100644 --- a/tools/testing/selftests/pidfd/pidfd_test.c +++ b/tools/testing/selftests/pidfd/pidfd_test.c @@ -441,7 +441,6 @@ static void test_pidfd_poll_exec(int use_waitpid) { int pid, pidfd = 0; int status, ret; - pthread_t t1; time_t prog_start = time(NULL); const char *test_name = "pidfd_poll check for premature notification on child thread exec"; @@ -500,13 +499,14 @@ static int child_poll_leader_exit_test(void *args) */ *child_exit_secs = time(NULL); syscall(SYS_exit, 0); + /* Never reached, but appeases compiler thinking we should return. */ + exit(0); } static void test_pidfd_poll_leader_exit(int use_waitpid) { int pid, pidfd = 0; - int status, ret; - time_t prog_start = time(NULL); + int status, ret = 0; const char *test_name = "pidfd_poll check for premature notification on non-empty" "group leader exit"; diff --git a/tools/testing/selftests/pidfd/pidfd_wait.c b/tools/testing/selftests/pidfd/pidfd_wait.c index be2943f072f60..17999e082aa71 100644 --- a/tools/testing/selftests/pidfd/pidfd_wait.c +++ b/tools/testing/selftests/pidfd/pidfd_wait.c @@ -39,7 +39,7 @@ static int sys_waitid(int which, pid_t pid, siginfo_t *info, int options, TEST(wait_simple) { - int pidfd = -1, status = 0; + int pidfd = -1; pid_t parent_tid = -1; struct clone_args args = { .parent_tid = ptr_to_u64(&parent_tid), @@ -47,7 +47,6 @@ TEST(wait_simple) .flags = CLONE_PIDFD | CLONE_PARENT_SETTID, .exit_signal = SIGCHLD, }; - int ret; pid_t pid; siginfo_t info = { .si_signo = 0, @@ -88,7 +87,7 @@ TEST(wait_simple) TEST(wait_states) { - int pidfd = -1, status = 0; + int pidfd = -1; pid_t parent_tid = -1; struct clone_args args = { .parent_tid = ptr_to_u64(&parent_tid), -- 2.34.1

3 years, 4 months

1
0
0 0

[PATCH AUTOSEL 5.15 15/33] pidfd: fix test failure due to stack overflow on some arches

by Sasha Levin

From: Axel Rasmussen <axelrasmussen(a)google.com> [ Upstream commit 4cbd93c3c110447adc66cb67c08af21f939ae2d7 ] When running the pidfd_fdinfo_test on arm64, it fails for me. After some digging, the reason is that the child exits due to SIGBUS, because it overflows the 1024 byte stack we've reserved for it. To fix the issue, increase the stack size to 8192 bytes (this number is somewhat arbitrary, and was arrived at through experimentation -- I kept doubling until the failure no longer occurred). Also, let's make the issue easier to debug. wait_for_pid() returns an ambiguous value: it may return -1 in all of these cases: 1. waitpid() itself returned -1 2. waitpid() returned success, but we found !WIFEXITED(status). 3. The child process exited, but it did so with a -1 exit code. There's no way for the caller to tell the difference. So, at least log which occurred, so the test runner can debug things. While debugging this, I found that we had !WIFEXITED(), because the child exited due to a signal. This seems like a reasonably common case, so also print out whether or not we have WIFSIGNALED(), and the associated WTERMSIG() (if any). This lets us see the SIGBUS I'm fixing clearly when it occurs. Finally, I'm suspicious of allocating the child's stack on our stack. man clone(2) suggests that the correct way to do this is with mmap(), and in particular by setting MAP_STACK. So, switch to doing it that way instead. Signed-off-by: Axel Rasmussen <axelrasmussen(a)google.com> Acked-by: Christian Brauner <brauner(a)kernel.org> Signed-off-by: Shuah Khan <skhan(a)linuxfoundation.org> Signed-off-by: Sasha Levin <sashal(a)kernel.org> --- tools/testing/selftests/pidfd/pidfd.h | 13 ++++++++--- .../selftests/pidfd/pidfd_fdinfo_test.c | 22 +++++++++++++++---- 2 files changed, 28 insertions(+), 7 deletions(-) diff --git a/tools/testing/selftests/pidfd/pidfd.h b/tools/testing/selftests/pidfd/pidfd.h index 01f8d3c0cf2cb..6922d6417e1cf 100644 --- a/tools/testing/selftests/pidfd/pidfd.h +++ b/tools/testing/selftests/pidfd/pidfd.h @@ -68,7 +68,7 @@ #define PIDFD_SKIP 3 #define PIDFD_XFAIL 4 -int wait_for_pid(pid_t pid) +static inline int wait_for_pid(pid_t pid) { int status, ret; @@ -78,13 +78,20 @@ int wait_for_pid(pid_t pid) if (errno == EINTR) goto again; + ksft_print_msg("waitpid returned -1, errno=%d\n", errno); return -1; } - if (!WIFEXITED(status)) + if (!WIFEXITED(status)) { + ksft_print_msg( + "waitpid !WIFEXITED, WIFSIGNALED=%d, WTERMSIG=%d\n", + WIFSIGNALED(status), WTERMSIG(status)); return -1; + } - return WEXITSTATUS(status); + ret = WEXITSTATUS(status); + ksft_print_msg("waitpid WEXITSTATUS=%d\n", ret); + return ret; } static inline int sys_pidfd_open(pid_t pid, unsigned int flags) diff --git a/tools/testing/selftests/pidfd/pidfd_fdinfo_test.c b/tools/testing/selftests/pidfd/pidfd_fdinfo_test.c index 22558524f71c3..3fd8e903118f5 100644 --- a/tools/testing/selftests/pidfd/pidfd_fdinfo_test.c +++ b/tools/testing/selftests/pidfd/pidfd_fdinfo_test.c @@ -12,6 +12,7 @@ #include <string.h> #include <syscall.h> #include <sys/wait.h> +#include <sys/mman.h> #include "pidfd.h" #include "../kselftest.h" @@ -80,7 +81,10 @@ static inline int error_check(struct error *err, const char *test_name) return err->code; } +#define CHILD_STACK_SIZE 8192 + struct child { + char *stack; pid_t pid; int fd; }; @@ -89,17 +93,22 @@ static struct child clone_newns(int (*fn)(void *), void *args, struct error *err) { static int flags = CLONE_PIDFD | CLONE_NEWPID | CLONE_NEWNS | SIGCHLD; - size_t stack_size = 1024; - char *stack[1024] = { 0 }; struct child ret; if (!(flags & CLONE_NEWUSER) && geteuid() != 0) flags |= CLONE_NEWUSER; + ret.stack = mmap(NULL, CHILD_STACK_SIZE, PROT_READ | PROT_WRITE, + MAP_PRIVATE | MAP_ANONYMOUS | MAP_STACK, -1, 0); + if (ret.stack == MAP_FAILED) { + error_set(err, -1, "mmap of stack failed (errno %d)", errno); + return ret; + } + #ifdef __ia64__ - ret.pid = __clone2(fn, stack, stack_size, flags, args, &ret.fd); + ret.pid = __clone2(fn, ret.stack, CHILD_STACK_SIZE, flags, args, &ret.fd); #else - ret.pid = clone(fn, stack + stack_size, flags, args, &ret.fd); + ret.pid = clone(fn, ret.stack + CHILD_STACK_SIZE, flags, args, &ret.fd); #endif if (ret.pid < 0) { @@ -129,6 +138,11 @@ static inline int child_join(struct child *child, struct error *err) else if (r > 0) error_set(err, r, "child %d reported: %d", child->pid, r); + if (munmap(child->stack, CHILD_STACK_SIZE)) { + error_set(err, -1, "munmap of child stack failed (errno %d)", errno); + r = -1; + } + return r; } -- 2.34.1

3 years, 4 months

1
0
0 0

[PATCH AUTOSEL 5.16 16/34] selftests: fixup build warnings in pidfd / clone3 tests

by Sasha Levin

From: Axel Rasmussen <axelrasmussen(a)google.com> [ Upstream commit e2aa5e650b07693477dff554053605976789fd68 ] These are some trivial fixups, which were needed to build the tests with clang and -Werror. The following issues are fixed: - Remove various unused variables. - In child_poll_leader_exit_test, clang isn't smart enough to realize syscall(SYS_exit, 0) won't return, so it complains we never return from a non-void function. Add an extra exit(0) to appease it. - In test_pidfd_poll_leader_exit, ret may be branched on despite being uninitialized, if we have !use_waitpid. Initialize it to zero to get the right behavior in that case. Signed-off-by: Axel Rasmussen <axelrasmussen(a)google.com> Acked-by: Christian Brauner <brauner(a)kernel.org> Signed-off-by: Shuah Khan <skhan(a)linuxfoundation.org> Signed-off-by: Sasha Levin <sashal(a)kernel.org> --- tools/testing/selftests/clone3/clone3.c | 2 -- tools/testing/selftests/pidfd/pidfd_test.c | 6 +++--- tools/testing/selftests/pidfd/pidfd_wait.c | 5 ++--- 3 files changed, 5 insertions(+), 8 deletions(-) diff --git a/tools/testing/selftests/clone3/clone3.c b/tools/testing/selftests/clone3/clone3.c index 076cf4325f783..cd4582129c7d6 100644 --- a/tools/testing/selftests/clone3/clone3.c +++ b/tools/testing/selftests/clone3/clone3.c @@ -126,8 +126,6 @@ static void test_clone3(uint64_t flags, size_t size, int expected, int main(int argc, char *argv[]) { - pid_t pid; - uid_t uid = getuid(); ksft_print_header(); diff --git a/tools/testing/selftests/pidfd/pidfd_test.c b/tools/testing/selftests/pidfd/pidfd_test.c index 529eb700ac26a..9a2d64901d591 100644 --- a/tools/testing/selftests/pidfd/pidfd_test.c +++ b/tools/testing/selftests/pidfd/pidfd_test.c @@ -441,7 +441,6 @@ static void test_pidfd_poll_exec(int use_waitpid) { int pid, pidfd = 0; int status, ret; - pthread_t t1; time_t prog_start = time(NULL); const char *test_name = "pidfd_poll check for premature notification on child thread exec"; @@ -500,13 +499,14 @@ static int child_poll_leader_exit_test(void *args) */ *child_exit_secs = time(NULL); syscall(SYS_exit, 0); + /* Never reached, but appeases compiler thinking we should return. */ + exit(0); } static void test_pidfd_poll_leader_exit(int use_waitpid) { int pid, pidfd = 0; - int status, ret; - time_t prog_start = time(NULL); + int status, ret = 0; const char *test_name = "pidfd_poll check for premature notification on non-empty" "group leader exit"; diff --git a/tools/testing/selftests/pidfd/pidfd_wait.c b/tools/testing/selftests/pidfd/pidfd_wait.c index be2943f072f60..17999e082aa71 100644 --- a/tools/testing/selftests/pidfd/pidfd_wait.c +++ b/tools/testing/selftests/pidfd/pidfd_wait.c @@ -39,7 +39,7 @@ static int sys_waitid(int which, pid_t pid, siginfo_t *info, int options, TEST(wait_simple) { - int pidfd = -1, status = 0; + int pidfd = -1; pid_t parent_tid = -1; struct clone_args args = { .parent_tid = ptr_to_u64(&parent_tid), @@ -47,7 +47,6 @@ TEST(wait_simple) .flags = CLONE_PIDFD | CLONE_PARENT_SETTID, .exit_signal = SIGCHLD, }; - int ret; pid_t pid; siginfo_t info = { .si_signo = 0, @@ -88,7 +87,7 @@ TEST(wait_simple) TEST(wait_states) { - int pidfd = -1, status = 0; + int pidfd = -1; pid_t parent_tid = -1; struct clone_args args = { .parent_tid = ptr_to_u64(&parent_tid), -- 2.34.1

3 years, 4 months

1
0
0 0

[PATCH AUTOSEL 5.16 15/34] pidfd: fix test failure due to stack overflow on some arches

by Sasha Levin

From: Axel Rasmussen <axelrasmussen(a)google.com> [ Upstream commit 4cbd93c3c110447adc66cb67c08af21f939ae2d7 ] When running the pidfd_fdinfo_test on arm64, it fails for me. After some digging, the reason is that the child exits due to SIGBUS, because it overflows the 1024 byte stack we've reserved for it. To fix the issue, increase the stack size to 8192 bytes (this number is somewhat arbitrary, and was arrived at through experimentation -- I kept doubling until the failure no longer occurred). Also, let's make the issue easier to debug. wait_for_pid() returns an ambiguous value: it may return -1 in all of these cases: 1. waitpid() itself returned -1 2. waitpid() returned success, but we found !WIFEXITED(status). 3. The child process exited, but it did so with a -1 exit code. There's no way for the caller to tell the difference. So, at least log which occurred, so the test runner can debug things. While debugging this, I found that we had !WIFEXITED(), because the child exited due to a signal. This seems like a reasonably common case, so also print out whether or not we have WIFSIGNALED(), and the associated WTERMSIG() (if any). This lets us see the SIGBUS I'm fixing clearly when it occurs. Finally, I'm suspicious of allocating the child's stack on our stack. man clone(2) suggests that the correct way to do this is with mmap(), and in particular by setting MAP_STACK. So, switch to doing it that way instead. Signed-off-by: Axel Rasmussen <axelrasmussen(a)google.com> Acked-by: Christian Brauner <brauner(a)kernel.org> Signed-off-by: Shuah Khan <skhan(a)linuxfoundation.org> Signed-off-by: Sasha Levin <sashal(a)kernel.org> --- tools/testing/selftests/pidfd/pidfd.h | 13 ++++++++--- .../selftests/pidfd/pidfd_fdinfo_test.c | 22 +++++++++++++++---- 2 files changed, 28 insertions(+), 7 deletions(-) diff --git a/tools/testing/selftests/pidfd/pidfd.h b/tools/testing/selftests/pidfd/pidfd.h index 01f8d3c0cf2cb..6922d6417e1cf 100644 --- a/tools/testing/selftests/pidfd/pidfd.h +++ b/tools/testing/selftests/pidfd/pidfd.h @@ -68,7 +68,7 @@ #define PIDFD_SKIP 3 #define PIDFD_XFAIL 4 -int wait_for_pid(pid_t pid) +static inline int wait_for_pid(pid_t pid) { int status, ret; @@ -78,13 +78,20 @@ int wait_for_pid(pid_t pid) if (errno == EINTR) goto again; + ksft_print_msg("waitpid returned -1, errno=%d\n", errno); return -1; } - if (!WIFEXITED(status)) + if (!WIFEXITED(status)) { + ksft_print_msg( + "waitpid !WIFEXITED, WIFSIGNALED=%d, WTERMSIG=%d\n", + WIFSIGNALED(status), WTERMSIG(status)); return -1; + } - return WEXITSTATUS(status); + ret = WEXITSTATUS(status); + ksft_print_msg("waitpid WEXITSTATUS=%d\n", ret); + return ret; } static inline int sys_pidfd_open(pid_t pid, unsigned int flags) diff --git a/tools/testing/selftests/pidfd/pidfd_fdinfo_test.c b/tools/testing/selftests/pidfd/pidfd_fdinfo_test.c index 22558524f71c3..3fd8e903118f5 100644 --- a/tools/testing/selftests/pidfd/pidfd_fdinfo_test.c +++ b/tools/testing/selftests/pidfd/pidfd_fdinfo_test.c @@ -12,6 +12,7 @@ #include <string.h> #include <syscall.h> #include <sys/wait.h> +#include <sys/mman.h> #include "pidfd.h" #include "../kselftest.h" @@ -80,7 +81,10 @@ static inline int error_check(struct error *err, const char *test_name) return err->code; } +#define CHILD_STACK_SIZE 8192 + struct child { + char *stack; pid_t pid; int fd; }; @@ -89,17 +93,22 @@ static struct child clone_newns(int (*fn)(void *), void *args, struct error *err) { static int flags = CLONE_PIDFD | CLONE_NEWPID | CLONE_NEWNS | SIGCHLD; - size_t stack_size = 1024; - char *stack[1024] = { 0 }; struct child ret; if (!(flags & CLONE_NEWUSER) && geteuid() != 0) flags |= CLONE_NEWUSER; + ret.stack = mmap(NULL, CHILD_STACK_SIZE, PROT_READ | PROT_WRITE, + MAP_PRIVATE | MAP_ANONYMOUS | MAP_STACK, -1, 0); + if (ret.stack == MAP_FAILED) { + error_set(err, -1, "mmap of stack failed (errno %d)", errno); + return ret; + } + #ifdef __ia64__ - ret.pid = __clone2(fn, stack, stack_size, flags, args, &ret.fd); + ret.pid = __clone2(fn, ret.stack, CHILD_STACK_SIZE, flags, args, &ret.fd); #else - ret.pid = clone(fn, stack + stack_size, flags, args, &ret.fd); + ret.pid = clone(fn, ret.stack + CHILD_STACK_SIZE, flags, args, &ret.fd); #endif if (ret.pid < 0) { @@ -129,6 +138,11 @@ static inline int child_join(struct child *child, struct error *err) else if (r > 0) error_set(err, r, "child %d reported: %d", child->pid, r); + if (munmap(child->stack, CHILD_STACK_SIZE)) { + error_set(err, -1, "munmap of child stack failed (errno %d)", errno); + r = -1; + } + return r; } -- 2.34.1

3 years, 4 months

1
0
0 0

[PATCH] ima: Calculate digest in ima_inode_hash() if not available

by Roberto Sassu

__ima_inode_hash() checks if a digest has been already calculated by looking for the integrity_iint_cache structure associated to the passed inode. Users of ima_file_hash() and ima_inode_hash() (e.g. eBPF) might be interested in obtaining the information without having to setup an IMA policy so that the digest is always available at the time they call one of those functions. Open a new file descriptor in __ima_inode_hash(), so that this function could invoke ima_collect_measurement() to calculate the digest if it is not available. Still return -EOPNOTSUPP if the calculation failed. Instead of opening a new file descriptor, the one from ima_file_hash() could have been used. However, since ima_inode_hash() was created to obtain the digest when the file descriptor is not available, it could benefit from this change too. Also, the opened file descriptor might be not suitable for use (file descriptor opened not for reading). This change does not cause memory usage increase, due to using a temporary integrity_iint_cache structure for the digest calculation, and due to freeing the ima_digest_data structure inside integrity_iint_cache before exiting from __ima_inode_hash(). Finally, update the test by removing ima_setup.sh (it is not necessary anymore to set an IMA policy) and by directly executing /bin/true. Signed-off-by: Roberto Sassu <roberto.sassu(a)huawei.com> --- security/integrity/ima/ima_main.c | 46 ++++++- tools/testing/selftests/bpf/Makefile | 1 - tools/testing/selftests/bpf/ima_setup.sh | 123 ------------------ .../selftests/bpf/prog_tests/test_ima.c | 25 +--- 4 files changed, 43 insertions(+), 152 deletions(-) delete mode 100755 tools/testing/selftests/bpf/ima_setup.sh diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index 8c6e4514d494..44df3f990950 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -26,6 +26,7 @@ #include <linux/ima.h> #include <linux/iversion.h> #include <linux/fs.h> +#include <linux/fs_struct.h> #include "ima.h" @@ -521,15 +522,43 @@ EXPORT_SYMBOL_GPL(ima_file_check); static int __ima_inode_hash(struct inode *inode, char *buf, size_t buf_size) { - struct integrity_iint_cache *iint; - int hash_algo; + struct integrity_iint_cache *iint = NULL, tmp_iint; + struct file *file; + struct path root, path; + int rc, hash_algo; + + if (ima_policy_flag) + iint = integrity_iint_find(inode); + + if (!iint) { + memset(&tmp_iint, 0, sizeof(tmp_iint)); + tmp_iint.inode = inode; + iint = &tmp_iint; + + path.dentry = d_find_alias(inode); + if (!path.dentry) + return -EOPNOTSUPP; + + get_fs_root(current->fs, &root); + path.mnt = root.mnt; + + file = dentry_open(&path, O_RDONLY, current_cred()); + if (IS_ERR(file)) { + dput(path.dentry); + path_put(&root); + return -EOPNOTSUPP; + } - if (!ima_policy_flag) - return -EOPNOTSUPP; + rc = ima_collect_measurement(iint, file, NULL, 0, ima_hash_algo, + NULL); - iint = integrity_iint_find(inode); - if (!iint) - return -EOPNOTSUPP; + fput(file); + dput(path.dentry); + path_put(&root); + + if (rc != 0) + return -EOPNOTSUPP; + } mutex_lock(&iint->mutex); @@ -551,6 +580,9 @@ static int __ima_inode_hash(struct inode *inode, char *buf, size_t buf_size) hash_algo = iint->ima_hash->algo; mutex_unlock(&iint->mutex); + if (iint == &tmp_iint) + kfree(iint->ima_hash); + return hash_algo; } diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile index 42ffc24e9e71..f7f850b2cf26 100644 --- a/tools/testing/selftests/bpf/Makefile +++ b/tools/testing/selftests/bpf/Makefile @@ -480,7 +480,6 @@ TRUNNER_EXTRA_SOURCES := test_progs.c cgroup_helpers.c trace_helpers.c \ network_helpers.c testing_helpers.c \ btf_helpers.c flow_dissector_load.h TRUNNER_EXTRA_FILES := $(OUTPUT)/urandom_read $(OUTPUT)/bpf_testmod.ko \ - ima_setup.sh \ $(wildcard progs/btf_dump_test_case_*.c) TRUNNER_BPF_BUILD_RULE := CLANG_BPF_BUILD_RULE TRUNNER_BPF_CFLAGS := $(BPF_CFLAGS) $(CLANG_CFLAGS) -DENABLE_ATOMICS_TESTS diff --git a/tools/testing/selftests/bpf/ima_setup.sh b/tools/testing/selftests/bpf/ima_setup.sh deleted file mode 100755 index 8e62581113a3..000000000000 --- a/tools/testing/selftests/bpf/ima_setup.sh +++ /dev/null @@ -1,123 +0,0 @@ -#!/bin/bash -# SPDX-License-Identifier: GPL-2.0 - -set -e -set -u -set -o pipefail - -IMA_POLICY_FILE="/sys/kernel/security/ima/policy" -TEST_BINARY="/bin/true" -VERBOSE="${SELFTESTS_VERBOSE:=0}" -LOG_FILE="$(mktemp /tmp/ima_setup.XXXX.log)" - -usage() -{ - echo "Usage: $0 <setup|cleanup|run> <existing_tmp_dir>" - exit 1 -} - -ensure_mount_securityfs() -{ - local securityfs_dir=$(grep "securityfs" /proc/mounts | awk '{print $2}') - - if [ -z "${securityfs_dir}" ]; then - securityfs_dir=/sys/kernel/security - mount -t securityfs security "${securityfs_dir}" - fi - - if [ ! -d "${securityfs_dir}" ]; then - echo "${securityfs_dir}: securityfs is not mounted" && exit 1 - fi -} - -setup() -{ - local tmp_dir="$1" - local mount_img="${tmp_dir}/test.img" - local mount_dir="${tmp_dir}/mnt" - local copied_bin_path="${mount_dir}/$(basename ${TEST_BINARY})" - mkdir -p ${mount_dir} - - dd if=/dev/zero of="${mount_img}" bs=1M count=10 - - losetup -f "${mount_img}" - local loop_device=$(losetup -a | grep ${mount_img:?} | cut -d ":" -f1) - - mkfs.ext2 "${loop_device:?}" - mount "${loop_device}" "${mount_dir}" - - cp "${TEST_BINARY}" "${mount_dir}" - local mount_uuid="$(blkid ${loop_device} | sed 's/.*UUID="$[^"]*$".*/\1/')" - - ensure_mount_securityfs - echo "measure func=BPRM_CHECK fsuuid=${mount_uuid}" > ${IMA_POLICY_FILE} -} - -cleanup() { - local tmp_dir="$1" - local mount_img="${tmp_dir}/test.img" - local mount_dir="${tmp_dir}/mnt" - - local loop_devices=$(losetup -a | grep ${mount_img:?} | cut -d ":" -f1) - - for loop_dev in "${loop_devices}"; do - losetup -d $loop_dev - done - - umount ${mount_dir} - rm -rf ${tmp_dir} -} - -run() -{ - local tmp_dir="$1" - local mount_dir="${tmp_dir}/mnt" - local copied_bin_path="${mount_dir}/$(basename ${TEST_BINARY})" - - exec "${copied_bin_path}" -} - -catch() -{ - local exit_code="$1" - local log_file="$2" - - if [[ "${exit_code}" -ne 0 ]]; then - cat "${log_file}" >&3 - fi - - rm -f "${log_file}" - exit ${exit_code} -} - -main() -{ - [[ $# -ne 2 ]] && usage - - local action="$1" - local tmp_dir="$2" - - [[ ! -d "${tmp_dir}" ]] && echo "Directory ${tmp_dir} doesn't exist" && exit 1 - - if [[ "${action}" == "setup" ]]; then - setup "${tmp_dir}" - elif [[ "${action}" == "cleanup" ]]; then - cleanup "${tmp_dir}" - elif [[ "${action}" == "run" ]]; then - run "${tmp_dir}" - else - echo "Unknown action: ${action}" - exit 1 - fi -} - -trap 'catch "$?" "${LOG_FILE}"' EXIT - -if [[ "${VERBOSE}" -eq 0 ]]; then - # Save the stderr to 3 so that we can output back to - # it incase of an error. - exec 3>&2 1>"${LOG_FILE}" 2>&1 -fi - -main "$@" -rm -f "${LOG_FILE}" diff --git a/tools/testing/selftests/bpf/prog_tests/test_ima.c b/tools/testing/selftests/bpf/prog_tests/test_ima.c index 97d8a6f84f4a..82427549f45a 100644 --- a/tools/testing/selftests/bpf/prog_tests/test_ima.c +++ b/tools/testing/selftests/bpf/prog_tests/test_ima.c @@ -13,15 +13,14 @@ #include "ima.skel.h" -static int run_measured_process(const char *measured_dir, u32 *monitored_pid) +static int run_measured_process(u32 *monitored_pid) { int child_pid, child_status; child_pid = fork(); if (child_pid == 0) { *monitored_pid = getpid(); - execlp("./ima_setup.sh", "./ima_setup.sh", "run", measured_dir, - NULL); + execlp("/bin/true", "/bin/true", NULL); exit(errno); } else if (child_pid > 0) { @@ -42,10 +41,7 @@ static int process_sample(void *ctx, void *data, size_t len) void test_test_ima(void) { - char measured_dir_template[] = "/tmp/ima_measuredXXXXXX"; struct ring_buffer *ringbuf = NULL; - const char *measured_dir; - char cmd[256]; int err, duration = 0; struct ima *skel = NULL; @@ -63,27 +59,14 @@ void test_test_ima(void) if (CHECK(err, "attach", "attach failed: %d\n", err)) goto close_prog; - measured_dir = mkdtemp(measured_dir_template); - if (CHECK(measured_dir == NULL, "mkdtemp", "err %d\n", errno)) - goto close_prog; - - snprintf(cmd, sizeof(cmd), "./ima_setup.sh setup %s", measured_dir); - err = system(cmd); - if (CHECK(err, "failed to run command", "%s, errno = %d\n", cmd, errno)) - goto close_clean; - - err = run_measured_process(measured_dir, &skel->bss->monitored_pid); + err = run_measured_process(&skel->bss->monitored_pid); if (CHECK(err, "run_measured_process", "err = %d\n", err)) - goto close_clean; + goto close_prog; err = ring_buffer__consume(ringbuf); ASSERT_EQ(err, 1, "num_samples_or_err"); ASSERT_NEQ(ima_hash_from_bpf, 0, "ima_hash"); -close_clean: - snprintf(cmd, sizeof(cmd), "./ima_setup.sh cleanup %s", measured_dir); - err = system(cmd); - CHECK(err, "failed to run command", "%s, errno = %d\n", cmd, errno); close_prog: ring_buffer__free(ringbuf); ima__destroy(skel); -- 2.32.0

3 years, 4 months

3
8
0 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

Linux-kselftest-mirror