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

[LTP] [PATCH 0/4] syscalls: Fix various syscalls 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 syscalls
tests.

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: Avoid dereferencing invalid buf in libc

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

-- 
2.25.1


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

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

[Tudor Cretu]

For any libc that doesn't define PTHREAD_RECURSIVE_MUTEX_INITIALIZER_NP,
don't assume that the type of the mutex is the first member. Use a
runtime initializer instead.

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..2f66ec40c 100644
--- a/lib/tst_res.c
+++ b/lib/tst_res.c
@@ -82,17 +82,27 @@ 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
+/* MUSL: http://www.openwall.com/lists/musl/2017/02/20/5 */
+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 1/4] lib: Fix initialization of recursive mutex

[Cyril Hrubis]

Hi!
> 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..2f66ec40c 100644
> --- a/lib/tst_res.c
> +++ b/lib/tst_res.c
> @@ -82,17 +82,27 @@ 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
> +/* MUSL: http://www.openwall.com/lists/musl/2017/02/20/5 */

I would have put this URL to the commit message instead, otherwise the
change looks good.

> +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

-- 
Cyril Hrubis
chrubis@suse.cz

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

[LTP] [PATCH 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 2/4] syscalls/mprotect01: Invoke the syscall directly instead of the libc wrapper

[Cyril Hrubis]

Hi!
> 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.

So Musl mproctect() does clear a few bits of the start and rounds the
len. That sounds like a dangerous thing to do to be honest, since you
end up with a different region than you requested.

I guess that this patch is reasonable solution since the glibc
mprotect() seems to be just wrapper for the syscall.

> 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

-- 
Cyril Hrubis
chrubis@suse.cz

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

[LTP] [PATCH 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>
---
 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 3/4] syscalls/prctl04: Allow rt_sigprocmask in the syscall filter

[Cyril Hrubis]

Hi!
Reviewed-by: Cyril Hrubis <chrubis@suse.cz>

-- 
Cyril Hrubis
chrubis@suse.cz

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

[LTP] [PATCH 4/4] syscalls/statfs: Avoid dereferencing invalid buf in libc

[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.

To avoid dereferencing an invalid buf in libc, bypass the [f]statfs wrapper
and call the syscall directly. Consistently with the libc wrappers,
choose [f]statfs64 instead of [f]statfs if the target supports it.

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

diff --git a/testcases/kernel/syscalls/fstatfs/fstatfs02.c b/testcases/kernel/syscalls/fstatfs/fstatfs02.c
index db2230f82..c1af07070 100644
--- a/testcases/kernel/syscalls/fstatfs/fstatfs02.c
+++ b/testcases/kernel/syscalls/fstatfs/fstatfs02.c
@@ -25,6 +25,7 @@
 #include <sys/types.h>
 #include <sys/statfs.h>
 #include <errno.h>
+#include "lapi/syscalls.h"
 #include "test.h"
 #include "safe_macros.h"
 
@@ -68,7 +69,11 @@ int main(int ac, char **av)
 
 		for (i = 0; i < TST_TOTAL; i++) {
 
-			TEST(fstatfs(TC[i].fd, TC[i].sbuf));
+#if __NR_fstatfs64 != __LTP__NR_INVALID_SYSCALL
+			TEST(tst_syscall(__NR_fstatfs64, TC[i].fd, TC[i].sbuf));
+#else
+			TEST(tst_syscall(__NR_fstatfs, TC[i].fd, TC[i].sbuf));
+#endif
 
 			if (TEST_RETURN != -1) {
 				tst_resm(TFAIL, "call succeeded unexpectedly");
diff --git a/testcases/kernel/syscalls/statfs/statfs02.c b/testcases/kernel/syscalls/statfs/statfs02.c
index 279665f86..e1afbda39 100644
--- a/testcases/kernel/syscalls/statfs/statfs02.c
+++ b/testcases/kernel/syscalls/statfs/statfs02.c
@@ -39,6 +39,7 @@
 #include <sys/vfs.h>
 #include <sys/mman.h>
 #include <errno.h>
+#include "lapi/syscalls.h"
 #include "test.h"
 #include "safe_macros.h"
 
@@ -116,7 +117,11 @@ static void setup(void)
 
 static void statfs_verify(const struct test_case_t *test)
 {
-	TEST(statfs(test->path, test->buf));
+#if __NR_statfs64 != __LTP__NR_INVALID_SYSCALL
+	TEST(tst_syscall(__NR_statfs64, test->path, test->buf));
+#else
+	TEST(tst_syscall(__NR_statfs, test->path, test->buf));
+#endif
 
 	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 4/4] syscalls/statfs: Avoid dereferencing invalid buf in libc

[Cyril Hrubis]

Hi!
> 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.
> 
> To avoid dereferencing an invalid buf in libc, bypass the [f]statfs wrapper
> and call the syscall directly. Consistently with the libc wrappers,
> choose [f]statfs64 instead of [f]statfs if the target supports it.

Another solution that we used in the past would be installing a signal
handler for segfault and treat that signal as a PASS for the EFAULT
test.

> Signed-off-by: Tudor Cretu <tudor.cretu@arm.com>
> ---
>  testcases/kernel/syscalls/fstatfs/fstatfs02.c | 7 ++++++-
>  testcases/kernel/syscalls/statfs/statfs02.c   | 7 ++++++-
>  2 files changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/testcases/kernel/syscalls/fstatfs/fstatfs02.c b/testcases/kernel/syscalls/fstatfs/fstatfs02.c
> index db2230f82..c1af07070 100644
> --- a/testcases/kernel/syscalls/fstatfs/fstatfs02.c
> +++ b/testcases/kernel/syscalls/fstatfs/fstatfs02.c
> @@ -25,6 +25,7 @@
>  #include <sys/types.h>
>  #include <sys/statfs.h>
>  #include <errno.h>
> +#include "lapi/syscalls.h"
>  #include "test.h"
>  #include "safe_macros.h"
>  
> @@ -68,7 +69,11 @@ int main(int ac, char **av)
>  
>  		for (i = 0; i < TST_TOTAL; i++) {
>  
> -			TEST(fstatfs(TC[i].fd, TC[i].sbuf));
> +#if __NR_fstatfs64 != __LTP__NR_INVALID_SYSCALL
> +			TEST(tst_syscall(__NR_fstatfs64, TC[i].fd, TC[i].sbuf));
> +#else
> +			TEST(tst_syscall(__NR_fstatfs, TC[i].fd, TC[i].sbuf));
> +#endif

Even if we wanted to go with this version this should be put into an
header in lapi/ and not added into each test that calls statfs().

>  			if (TEST_RETURN != -1) {
>  				tst_resm(TFAIL, "call succeeded unexpectedly");
> diff --git a/testcases/kernel/syscalls/statfs/statfs02.c b/testcases/kernel/syscalls/statfs/statfs02.c
> index 279665f86..e1afbda39 100644
> --- a/testcases/kernel/syscalls/statfs/statfs02.c
> +++ b/testcases/kernel/syscalls/statfs/statfs02.c
> @@ -39,6 +39,7 @@
>  #include <sys/vfs.h>
>  #include <sys/mman.h>
>  #include <errno.h>
> +#include "lapi/syscalls.h"
>  #include "test.h"
>  #include "safe_macros.h"
>  
> @@ -116,7 +117,11 @@ static void setup(void)
>  
>  static void statfs_verify(const struct test_case_t *test)
>  {
> -	TEST(statfs(test->path, test->buf));
> +#if __NR_statfs64 != __LTP__NR_INVALID_SYSCALL
> +	TEST(tst_syscall(__NR_statfs64, test->path, test->buf));
> +#else
> +	TEST(tst_syscall(__NR_statfs, test->path, test->buf));
> +#endif
>  
>  	if (TEST_RETURN != -1) {
>  		tst_resm(TFAIL, "call succeeded unexpectedly");
> -- 
> 2.25.1
> 
> 
> -- 
> Mailing list info: https://lists.linux.it/listinfo/ltp

-- 
Cyril Hrubis
chrubis@suse.cz

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