[LTP] [PATCH v2 0/4] syscalls: Fix various syscall tests when compiled with Musl

[LTP] [PATCH v2 0/4] syscalls: Fix various syscall tests when compiled with Musl

[Tudor Cretu]

Hi,

There were a few issues with some syscalls tests when they were compiled
with Musl. This series attempts to improve the robustness of some syscall
tests.

changes v1->v2:
* Patch 1/1: Moved the Musl link from source to the commit message
* Patch 4/4: Implement a sighandler to catch segfaults and allow the EFAULT
             tests to pass, instead of just calling the raw [f]statfs syscall.

Tudor Cretu (4):
  lib: Fix initialization of recursive mutex
  syscalls/mprotect01: Invoke the syscall directly instead of the libc
    wrapper
  syscalls/prctl04: Allow rt_sigprocmask in the syscall filter
  syscalls/statfs: Accept segfault instead of EFAULT

 lib/tst_res.c                                 | 25 +++++---
 testcases/kernel/syscalls/fstatfs/fstatfs02.c | 60 ++++++++++++++-----
 .../kernel/syscalls/mprotect/mprotect01.c     |  3 +-
 testcases/kernel/syscalls/prctl/prctl04.c     |  1 +
 testcases/kernel/syscalls/statfs/statfs02.c   | 32 +++++++++-
 5 files changed, 93 insertions(+), 28 deletions(-)

-- 
2.25.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v2 1/4] lib: Fix initialization of recursive mutex

[Tudor Cretu]

For any libc that doesn't define PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP
(e.g. Musl [1]), don't assume that the type of the mutex is the first
member. Use a runtime initializer instead.

[1] https://www.openwall.com/lists/musl/2017/02/20/5

Signed-off-by: Tudor Cretu <tudor.cretu@arm.com>
---
 lib/tst_res.c | 25 +++++++++++++++++--------
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/lib/tst_res.c b/lib/tst_res.c
index 8d86b48a4..e0896eb05 100644
--- a/lib/tst_res.c
+++ b/lib/tst_res.c
@@ -82,17 +82,26 @@ void *TST_RET_PTR;
 	assert(strlen(buf) > 0);		\
 } while (0)
 
-#ifndef PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP
-# ifdef __ANDROID__
-#  define PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP \
-	PTHREAD_RECURSIVE_MUTEX_INITIALIZER
-# else
-/* MUSL: http://www.openwall.com/lists/musl/2017/02/20/5 */
-#  define PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP  { {PTHREAD_MUTEX_RECURSIVE} }
-# endif
+#if !defined(PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP) && defined(__ANDROID__)
+#define PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP PTHREAD_RECURSIVE_MUTEX_INITIALIZER
 #endif
 
+#ifdef PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP
 static pthread_mutex_t tmutex = PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP;
+#else
+static pthread_mutex_t tmutex;
+
+__attribute__((constructor))
+static void init_tmutex(void)
+{
+	pthread_mutexattr_t mutattr = {0};
+
+	pthread_mutexattr_init(&mutattr);
+	pthread_mutexattr_settype(&mutattr, PTHREAD_MUTEX_RECURSIVE);
+	pthread_mutex_init(&tmutex, &mutattr);
+	pthread_mutexattr_destroy(&mutattr);
+}
+#endif
 
 static void check_env(void);
 static void tst_condense(int tnum, int ttype, const char *tmesg);
-- 
2.25.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2 1/4] lib: Fix initialization of recursive mutex

[Petr Vorel]

Hi Tudor,

merged this one, thanks!

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v2 2/4] syscalls/mprotect01: Invoke the syscall directly instead of the libc wrapper

[Tudor Cretu]

per POSIX: The mprotect() function shall change the access protections
to be that specified by prot for those whole pages containing any part
of the address space of the process starting at address addr and
continuing for len bytes.

Issue 6 of POSIX introduces: The implementation may require that addr
be a multiple of the page size as returned by sysconf().

Therefore it's not strictly required that addr is a multiple of the page
size. Some libcs (e.g. Musl) indeed don't have this requirement, so calling
the C standard library function doesn't fail in their case. As the
testsuite focuses on mprotect(2), the testcases should call the syscall
directly instead of the libc function.

Signed-off-by: Tudor Cretu <tudor.cretu@arm.com>
---
 testcases/kernel/syscalls/mprotect/mprotect01.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/testcases/kernel/syscalls/mprotect/mprotect01.c b/testcases/kernel/syscalls/mprotect/mprotect01.c
index be4d982ea..aa4685258 100644
--- a/testcases/kernel/syscalls/mprotect/mprotect01.c
+++ b/testcases/kernel/syscalls/mprotect/mprotect01.c
@@ -43,6 +43,7 @@
 #include <stdlib.h>
 #include <unistd.h>
 #include "test.h"
+#include "lapi/syscalls.h"
 #include "safe_macros.h"
 
 char *TCID = "mprotect01";
@@ -97,7 +98,7 @@ int main(int ac, char **av)
 			if (TC[i].setupfunc != NULL)
 				TC[i].setupfunc(&TC[i]);
 
-			TEST(mprotect(TC[i].addr, TC[i].len, TC[i].prot));
+			TEST(tst_syscall(__NR_mprotect, TC[i].addr, TC[i].len, TC[i].prot));
 
 			if (TEST_RETURN != -1) {
 				tst_resm(TFAIL, "call succeeded unexpectedly");
-- 
2.25.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2 2/4] syscalls/mprotect01: Invoke the syscall directly instead of the libc wrapper

[Petr Vorel]

Hi,

this one also merged, thanks!

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v2 3/4] syscalls/prctl04: Allow rt_sigprocmask in the syscall filter

[Tudor Cretu]

Some libcs (e.g. Musl) call rt_sigprocmask as part of their fork
implementation. To successfully call fork, rt_sigprocmask must be allowed
as well in the filter.

Signed-off-by: Tudor Cretu <tudor.cretu@arm.com>
Reviewed-by: Cyril Hrubis <chrubis@suse.cz>
---
 testcases/kernel/syscalls/prctl/prctl04.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/testcases/kernel/syscalls/prctl/prctl04.c b/testcases/kernel/syscalls/prctl/prctl04.c
index 1cc19bbd3..2f7e9a1ac 100644
--- a/testcases/kernel/syscalls/prctl/prctl04.c
+++ b/testcases/kernel/syscalls/prctl/prctl04.c
@@ -45,6 +45,7 @@
 static const struct sock_filter  strict_filter[] = {
 	BPF_STMT(BPF_LD | BPF_W | BPF_ABS, (offsetof(struct seccomp_data, nr))),
 
+	BPF_JUMP(BPF_JMP | BPF_JEQ, __NR_rt_sigprocmask, 6, 0),
 	BPF_JUMP(BPF_JMP | BPF_JEQ, __NR_close, 5, 0),
 	BPF_JUMP(BPF_JMP | BPF_JEQ, __NR_exit,  4, 0),
 	BPF_JUMP(BPF_JMP | BPF_JEQ, __NR_wait4, 3, 0),
-- 
2.25.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2 3/4] syscalls/prctl04: Allow rt_sigprocmask in the syscall filter

[Petr Vorel]

Hi Tudor,

> Some libcs (e.g. Musl) call rt_sigprocmask as part of their fork
> implementation. To successfully call fork, rt_sigprocmask must be allowed
> as well in the filter.

When tested on lastest update Alpine with 1.2.3 it in both cases fails:

prctl04.c:205: TPASS: SECCOMP_MODE_FILTER doesn't permit exit()
prctl04.c:207: TFAIL: SECCOMP_MODE_FILTER doesn't permit exit()

What can be wrong?

@Yang Xu: Also this test (written in new API) should use tst_reinit()
https://lore.kernel.org/ltp/70476626-2c7a-fcd6-4cf4-de7cbd572f99@fujitsu.com/T/#mb3e34713dd15f2050ec2dc01615fefb7ee66c880

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2 3/4] syscalls/prctl04: Allow rt_sigprocmask in the syscall filter

[Cyril Hrubis]

Hi!
> > Some libcs (e.g. Musl) call rt_sigprocmask as part of their fork
> > implementation. To successfully call fork, rt_sigprocmask must be allowed
> > as well in the filter.
> 
> When tested on lastest update Alpine with 1.2.3 it in both cases fails:
> 
> prctl04.c:205: TPASS: SECCOMP_MODE_FILTER doesn't permit exit()
> prctl04.c:207: TFAIL: SECCOMP_MODE_FILTER doesn't permit exit()
>
> What can be wrong?

And does the test work without the patch?

As far as I can tell the change to the bpf is correct and shouldn't
affect anything, it just allows one more syscall to be called.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2 3/4] syscalls/prctl04: Allow rt_sigprocmask in the syscall filter

[Tudor Cretu]

Hi Petr,

On 23-08-2022 10:41, Petr Vorel wrote:
> Hi Tudor,
>
>> Some libcs (e.g. Musl) call rt_sigprocmask as part of their fork
>> implementation. To successfully call fork, rt_sigprocmask must be allowed
>> as well in the filter.
>
> When tested on lastest update Alpine with 1.2.3 it in both cases fails:
>
> prctl04.c:205: TPASS: SECCOMP_MODE_FILTER doesn't permit exit()
> prctl04.c:207: TFAIL: SECCOMP_MODE_FILTER doesn't permit exit()

Sorry, I am not sure I understand. Which are the both cases in which the
test fails? I am also using Musl 1.2.3 and the test passes for me on
BusyBox (both x86_64 and AArch64). I don't have an Alpine environment,
and I am having issues with docker and seccomp, so I am not able to test
on Alpine yet.

Kind regards,
Tudor

>
> What can be wrong?
>
> @Yang Xu: Also this test (written in new API) should use tst_reinit()
> https://lore.kernel.org/ltp/70476626-2c7a-fcd6-4cf4-de7cbd572f99@fujitsu.com/T/#mb3e34713dd15f2050ec2dc01615fefb7ee66c880
>
> Kind regards,
> Petr
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2 3/4] syscalls/prctl04: Allow rt_sigprocmask in the syscall filter

[Petr Vorel]

Hi Tudor, all,

> Hi Petr,

> On 23-08-2022 10:41, Petr Vorel wrote:
> > Hi Tudor,

> > > Some libcs (e.g. Musl) call rt_sigprocmask as part of their fork
> > > implementation. To successfully call fork, rt_sigprocmask must be allowed
> > > as well in the filter.

> > When tested on lastest update Alpine with 1.2.3 it in both cases fails:

> > prctl04.c:205: TPASS: SECCOMP_MODE_FILTER doesn't permit exit()
> > prctl04.c:207: TFAIL: SECCOMP_MODE_FILTER doesn't permit exit()

> Sorry, I am not sure I understand. Which are the both cases in which the
> test fails? I am also using Musl 1.2.3 and the test passes for me on
> BusyBox (both x86_64 and AArch64). I don't have an Alpine environment,
> and I am having issues with docker and seccomp, so I am not able to test
> on Alpine yet.

I'm sorry I wasn't clear: on Alpine distro (which uses musl libc) it fails
without this patch *and* with it (i.e. patch does not fixes it).
But as you report that it fixes it on musl 1.2.3, I guess it's some Alpine
specific bug, I merged it.

Thanks!

Kind regards,
Petr

> Kind regards,
> Tudor

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v2 4/4] syscalls/statfs: Accept segfault instead of EFAULT

[Tudor Cretu]

The [f]statfs02 testsuites check that [f]statfs returns EFUALT when the
provided buf parameter is invalid. There are cases in which the supported
libcs don't exhibit this behaviour.

glibc versions newer than 2.34 and on systems that support [f]statfs64,
call the syscall with a local struct statfs and then copy the result
into buf. This throws a segfault for an invalid buf. musl dereferences buf
before the syscall is called and, similarly, throws a segfault.

Implement a sighandler to handle the cases where a segfault is thrown
instead of returning EFAULT.

Signed-off-by: Tudor Cretu <tudor.cretu@arm.com>
---
 testcases/kernel/syscalls/fstatfs/fstatfs02.c | 60 ++++++++++++++-----
 testcases/kernel/syscalls/statfs/statfs02.c   | 32 +++++++++-
 2 files changed, 73 insertions(+), 19 deletions(-)

diff --git a/testcases/kernel/syscalls/fstatfs/fstatfs02.c b/testcases/kernel/syscalls/fstatfs/fstatfs02.c
index db2230f82..e46b9df23 100644
--- a/testcases/kernel/syscalls/fstatfs/fstatfs02.c
+++ b/testcases/kernel/syscalls/fstatfs/fstatfs02.c
@@ -21,6 +21,8 @@
  *	Testcase to check fstatfs() sets errno correctly.
  */
 
+#include <setjmp.h>
+#include <signal.h>
 #include <sys/vfs.h>
 #include <sys/types.h>
 #include <sys/statfs.h>
@@ -28,9 +30,6 @@
 #include "test.h"
 #include "safe_macros.h"
 
-static void setup(void);
-static void cleanup(void);
-
 char *TCID = "fstatfs02";
 
 static struct statfs buf;
@@ -53,6 +52,13 @@ static struct test_case_t {
 
 int TST_TOTAL = ARRAY_SIZE(TC);
 
+static int sig_caught;
+static sigjmp_buf env;
+
+static void setup(void);
+static void cleanup(void);
+static void fstatfs_verify(const struct test_case_t *);
+
 int main(int ac, char **av)
 {
 	int lc;
@@ -67,23 +73,20 @@ int main(int ac, char **av)
 		tst_count = 0;
 
 		for (i = 0; i < TST_TOTAL; i++) {
+			sig_caught = 0;
+			if (sigsetjmp(env, 1) == 0)
+				fstatfs_verify(&TC[i]);
 
-			TEST(fstatfs(TC[i].fd, TC[i].sbuf));
+			if (!sig_caught)
+				continue;
 
-			if (TEST_RETURN != -1) {
-				tst_resm(TFAIL, "call succeeded unexpectedly");
+			if (TC[i].error == EFAULT && sig_caught == SIGSEGV) {
+				tst_resm(TINFO, "received SIGSEGV instead of returning EFAULT");
+				tst_resm(TPASS | TTERRNO, "expected failure");
 				continue;
 			}
 
-			if (TEST_ERRNO == TC[i].error) {
-				tst_resm(TPASS, "expected failure - "
-					 "errno = %d : %s", TEST_ERRNO,
-					 strerror(TEST_ERRNO));
-			} else {
-				tst_resm(TFAIL, "unexpected error - %d : %s - "
-					 "expected %d", TEST_ERRNO,
-					 strerror(TEST_ERRNO), TC[i].error);
-			}
+			tst_resm(TFAIL, "Received an unexpected signal: %d", sig_caught);
 		}
 	}
 	cleanup();
@@ -91,9 +94,16 @@ int main(int ac, char **av)
 	tst_exit();
 }
 
+static void sighandler(int sig)
+{
+	sig_caught = sig;
+	siglongjmp(env, 1);
+
+}
+
 static void setup(void)
 {
-	tst_sig(NOFORK, DEF_HANDLER, cleanup);
+	tst_sig(NOFORK, sighandler, cleanup);
 
 	TEST_PAUSE;
 
@@ -103,6 +113,24 @@ static void setup(void)
 #endif
 }
 
+static void fstatfs_verify(const struct test_case_t *test)
+{
+	TEST(fstatfs(test->fd, test->sbuf));
+
+	if (TEST_RETURN != -1) {
+		tst_resm(TFAIL, "call succeeded unexpectedly");
+		return;
+	}
+
+	if (TEST_ERRNO == test->error) {
+		tst_resm(TPASS, "expected failure - errno = %d : %s",
+			 TEST_ERRNO, strerror(TEST_ERRNO));
+	} else {
+		tst_resm(TFAIL, "unexpected error - %d : %s - expected %d",
+			 TEST_ERRNO, strerror(TEST_ERRNO), test->error);
+	}
+}
+
 static void cleanup(void)
 {
 #ifndef UCLINUX
diff --git a/testcases/kernel/syscalls/statfs/statfs02.c b/testcases/kernel/syscalls/statfs/statfs02.c
index 279665f86..7f150d1d9 100644
--- a/testcases/kernel/syscalls/statfs/statfs02.c
+++ b/testcases/kernel/syscalls/statfs/statfs02.c
@@ -32,6 +32,8 @@
  *		ELOOP.
  */
 
+#include <setjmp.h>
+#include <signal.h>
 #include <sys/types.h>
 #include <sys/statfs.h>
 #include <sys/stat.h>
@@ -70,6 +72,10 @@ static struct test_case_t {
 };
 
 int TST_TOTAL = ARRAY_SIZE(TC);
+
+static int sig_caught;
+static sigjmp_buf env;
+
 static void setup(void);
 static void cleanup(void);
 static void statfs_verify(const struct test_case_t *);
@@ -85,17 +91,37 @@ int main(int ac, char **av)
 
 	for (lc = 0; TEST_LOOPING(lc); lc++) {
 		tst_count = 0;
-		for (i = 0; i < TST_TOTAL; i++)
-			statfs_verify(&TC[i]);
+		for (i = 0; i < TST_TOTAL; i++) {
+			sig_caught = 0;
+			if (sigsetjmp(env, 1) == 0)
+				statfs_verify(&TC[i]);
+
+			if (!sig_caught)
+				continue;
+
+			if (TC[i].exp_error == EFAULT && sig_caught == SIGSEGV) {
+				tst_resm(TINFO, "received SIGSEGV instead of returning EFAULT");
+				tst_resm(TPASS | TTERRNO, "expected failure");
+				continue;
+			}
+
+			tst_resm(TFAIL, "Received an unexpected signal: %d", sig_caught);
+		}
 	}
 
 	cleanup();
 	tst_exit();
 }
 
+static void sighandler(int sig)
+{
+	sig_caught = sig;
+	siglongjmp(env, 1);
+}
+
 static void setup(void)
 {
-	tst_sig(NOFORK, DEF_HANDLER, cleanup);
+	tst_sig(NOFORK, sighandler, cleanup);
 
 	TEST_PAUSE;
 
-- 
2.25.1


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v2 4/4] syscalls/statfs: Accept segfault instead of EFAULT

[Cyril Hrubis]

>  int main(int ac, char **av)
>  {
>  	int lc;
> @@ -67,23 +73,20 @@ int main(int ac, char **av)
>  		tst_count = 0;
>  
>  		for (i = 0; i < TST_TOTAL; i++) {
> +			sig_caught = 0;
> +			if (sigsetjmp(env, 1) == 0)
> +				fstatfs_verify(&TC[i]);
>  
> -			TEST(fstatfs(TC[i].fd, TC[i].sbuf));
> +			if (!sig_caught)
> +				continue;
>  
> -			if (TEST_RETURN != -1) {
> -				tst_resm(TFAIL, "call succeeded unexpectedly");
> +			if (TC[i].error == EFAULT && sig_caught == SIGSEGV) {
> +				tst_resm(TINFO, "received SIGSEGV instead of returning EFAULT");
> +				tst_resm(TPASS | TTERRNO, "expected failure");

This can be just a signle message tst_resm(TPASS | TERRNO, "Got SIGSEGV intead of EFAULT");

>  				continue;
>  			}
>  
> -			if (TEST_ERRNO == TC[i].error) {
> -				tst_resm(TPASS, "expected failure - "
> -					 "errno = %d : %s", TEST_ERRNO,
> -					 strerror(TEST_ERRNO));
> -			} else {
> -				tst_resm(TFAIL, "unexpected error - %d : %s - "
> -					 "expected %d", TEST_ERRNO,
> -					 strerror(TEST_ERRNO), TC[i].error);
> -			}
> +			tst_resm(TFAIL, "Received an unexpected signal: %d", sig_caught);
>  		}
>  	}
>  	cleanup();
> @@ -91,9 +94,16 @@ int main(int ac, char **av)
>  	tst_exit();
>  }
>  
> +static void sighandler(int sig)
> +{
> +	sig_caught = sig;
> +	siglongjmp(env, 1);
> +
> +}
> +
>  static void setup(void)
>  {
> -	tst_sig(NOFORK, DEF_HANDLER, cleanup);
> +	tst_sig(NOFORK, sighandler, cleanup);

Can we just setup handler for the SIGSEGV signal and keep everything
else for the DEF_HANDLER?

>  	TEST_PAUSE;
>  
> @@ -103,6 +113,24 @@ static void setup(void)
>  #endif
>  }
>  
> +static void fstatfs_verify(const struct test_case_t *test)
> +{
> +	TEST(fstatfs(test->fd, test->sbuf));
> +
> +	if (TEST_RETURN != -1) {
> +		tst_resm(TFAIL, "call succeeded unexpectedly");
> +		return;
> +	}
> +
> +	if (TEST_ERRNO == test->error) {
> +		tst_resm(TPASS, "expected failure - errno = %d : %s",
> +			 TEST_ERRNO, strerror(TEST_ERRNO));
> +	} else {
> +		tst_resm(TFAIL, "unexpected error - %d : %s - expected %d",
> +			 TEST_ERRNO, strerror(TEST_ERRNO), test->error);
> +	}
> +}

If we converted the test to the new test API this would be a single line as:

	TST_EXP_FAIL(fstatfs(test->fd, test->sbuf), test->error, "fstatfs()");

Generally with the new test api the code would be much shorter...

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp