* [LTP] [PATCH 1/5] close_range: Add syscall number
2021-02-11 11:03 [LTP] [PATCH 0/5] Add close_range01, SAFE_DUP2 and SAFE_CLONE Richard Palethorpe
@ 2021-02-11 11:03 ` Richard Palethorpe
2021-02-11 11:03 ` [LTP] [PATCH 2/5] API: Add SAFE_DUP2 Richard Palethorpe
` (3 subsequent siblings)
4 siblings, 0 replies; 12+ messages in thread
From: Richard Palethorpe @ 2021-02-11 11:03 UTC (permalink / raw)
To: ltp
Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
---
include/lapi/syscalls/aarch64.in | 1 +
include/lapi/syscalls/arc.in | 1 +
include/lapi/syscalls/arm.in | 1 +
include/lapi/syscalls/hppa.in | 1 +
include/lapi/syscalls/i386.in | 1 +
include/lapi/syscalls/ia64.in | 1 +
include/lapi/syscalls/mips_n32.in | 1 +
include/lapi/syscalls/mips_n64.in | 1 +
include/lapi/syscalls/mips_o32.in | 1 +
include/lapi/syscalls/powerpc.in | 1 +
include/lapi/syscalls/powerpc64.in | 1 +
include/lapi/syscalls/s390.in | 1 +
include/lapi/syscalls/s390x.in | 1 +
include/lapi/syscalls/sh.in | 1 +
include/lapi/syscalls/sparc.in | 1 +
include/lapi/syscalls/sparc64.in | 1 +
include/lapi/syscalls/x86_64.in | 1 +
17 files changed, 17 insertions(+)
diff --git a/include/lapi/syscalls/aarch64.in b/include/lapi/syscalls/aarch64.in
index 07556933f..2def6ba3d 100644
--- a/include/lapi/syscalls/aarch64.in
+++ b/include/lapi/syscalls/aarch64.in
@@ -310,6 +310,7 @@ fsmount 432
fspick 433
pidfd_open 434
clone3 435
+close_range 436
openat2 437
pidfd_getfd 438
_sysctl 1078
diff --git a/include/lapi/syscalls/arc.in b/include/lapi/syscalls/arc.in
index 0cadb150c..9bcd84706 100644
--- a/include/lapi/syscalls/arc.in
+++ b/include/lapi/syscalls/arc.in
@@ -310,5 +310,6 @@ fsmount 432
fspick 433
pidfd_open 434
clone3 435
+close_range 436
openat2 437
pidfd_getfd 438
diff --git a/include/lapi/syscalls/arm.in b/include/lapi/syscalls/arm.in
index 395d53a68..98c840cb8 100644
--- a/include/lapi/syscalls/arm.in
+++ b/include/lapi/syscalls/arm.in
@@ -388,5 +388,6 @@ fsmount (__NR_SYSCALL_BASE+432)
fspick (__NR_SYSCALL_BASE+433)
pidfd_open (__NR_SYSCALL_BASE+434)
clone3 (__NR_SYSCALL_BASE+435)
+close_range (__NR_SYSCALL_BASE+436)
openat2 (__NR_SYSCALL_BASE+437)
pidfd_getfd (__NR_SYSCALL_BASE+438)
diff --git a/include/lapi/syscalls/hppa.in b/include/lapi/syscalls/hppa.in
index 7f9b5e0a0..e1628c4b1 100644
--- a/include/lapi/syscalls/hppa.in
+++ b/include/lapi/syscalls/hppa.in
@@ -39,3 +39,4 @@ fsconfig 431
fsmount 432
fspick 433
pidfd_open 434
+close_range 436
\ No newline at end of file
diff --git a/include/lapi/syscalls/i386.in b/include/lapi/syscalls/i386.in
index d6773abcb..aaa02c7bf 100644
--- a/include/lapi/syscalls/i386.in
+++ b/include/lapi/syscalls/i386.in
@@ -424,5 +424,6 @@ fsmount 432
fspick 433
pidfd_open 434
clone3 435
+close_range 436
openat2 437
pidfd_getfd 438
diff --git a/include/lapi/syscalls/ia64.in b/include/lapi/syscalls/ia64.in
index 427b711b1..5467f80f2 100644
--- a/include/lapi/syscalls/ia64.in
+++ b/include/lapi/syscalls/ia64.in
@@ -337,5 +337,6 @@ fsconfig 1455
fsmount 1456
fspick 1457
pidfd_open 1458
+close_range 1460
openat2 1461
pidfd_getfd 1462
diff --git a/include/lapi/syscalls/mips_n32.in b/include/lapi/syscalls/mips_n32.in
index eb6140ebc..a509ca5c6 100644
--- a/include/lapi/syscalls/mips_n32.in
+++ b/include/lapi/syscalls/mips_n32.in
@@ -362,5 +362,6 @@ fsmount 432
fspick 433
pidfd_open 434
clone3 435
+close_range 436
openat2 437
pidfd_getfd 438
diff --git a/include/lapi/syscalls/mips_n64.in b/include/lapi/syscalls/mips_n64.in
index 5480aa3c9..532c69dd8 100644
--- a/include/lapi/syscalls/mips_n64.in
+++ b/include/lapi/syscalls/mips_n64.in
@@ -338,5 +338,6 @@ fsmount 432
fspick 433
pidfd_open 434
clone3 435
+close_range 436
openat2 437
pidfd_getfd 438
diff --git a/include/lapi/syscalls/mips_o32.in b/include/lapi/syscalls/mips_o32.in
index feee8fbf3..c71a62a35 100644
--- a/include/lapi/syscalls/mips_o32.in
+++ b/include/lapi/syscalls/mips_o32.in
@@ -408,5 +408,6 @@ fsmount 432
fspick 433
pidfd_open 434
clone3 435
+close_range 436
openat2 437
pidfd_getfd 438
diff --git a/include/lapi/syscalls/powerpc.in b/include/lapi/syscalls/powerpc.in
index cdbebc62d..2d287a606 100644
--- a/include/lapi/syscalls/powerpc.in
+++ b/include/lapi/syscalls/powerpc.in
@@ -417,5 +417,6 @@ fsmount 432
fspick 433
pidfd_open 434
clone3 435
+close_range 436
openat2 437
pidfd_getfd 438
diff --git a/include/lapi/syscalls/powerpc64.in b/include/lapi/syscalls/powerpc64.in
index cdbebc62d..2d287a606 100644
--- a/include/lapi/syscalls/powerpc64.in
+++ b/include/lapi/syscalls/powerpc64.in
@@ -417,5 +417,6 @@ fsmount 432
fspick 433
pidfd_open 434
clone3 435
+close_range 436
openat2 437
pidfd_getfd 438
diff --git a/include/lapi/syscalls/s390.in b/include/lapi/syscalls/s390.in
index c3f249aed..c978b6660 100644
--- a/include/lapi/syscalls/s390.in
+++ b/include/lapi/syscalls/s390.in
@@ -404,5 +404,6 @@ fsmount 432
fspick 433
pidfd_open 434
clone3 435
+close_range 436
openat2 437
pidfd_getfd 438
diff --git a/include/lapi/syscalls/s390x.in b/include/lapi/syscalls/s390x.in
index 88cc9b86b..d123db6cb 100644
--- a/include/lapi/syscalls/s390x.in
+++ b/include/lapi/syscalls/s390x.in
@@ -352,5 +352,6 @@ fsmount 432
fspick 433
pidfd_open 434
clone3 435
+close_range 436
openat2 437
pidfd_getfd 438
diff --git a/include/lapi/syscalls/sh.in b/include/lapi/syscalls/sh.in
index 06055ed10..22da7d6ff 100644
--- a/include/lapi/syscalls/sh.in
+++ b/include/lapi/syscalls/sh.in
@@ -398,5 +398,6 @@ fsconfig 431
fsmount 432
fspick 433
pidfd_open 434
+close_range 436
openat2 437
pidfd_getfd 438
diff --git a/include/lapi/syscalls/sparc.in b/include/lapi/syscalls/sparc.in
index 522e3c997..7324b4ac6 100644
--- a/include/lapi/syscalls/sparc.in
+++ b/include/lapi/syscalls/sparc.in
@@ -403,5 +403,6 @@ fsconfig 431
fsmount 432
fspick 433
pidfd_open 434
+close_range 436
openat2 437
pidfd_getfd 438
diff --git a/include/lapi/syscalls/sparc64.in b/include/lapi/syscalls/sparc64.in
index 6f884a7bf..862d806fa 100644
--- a/include/lapi/syscalls/sparc64.in
+++ b/include/lapi/syscalls/sparc64.in
@@ -368,5 +368,6 @@ fsconfig 431
fsmount 432
fspick 433
pidfd_open 434
+close_range 436
openat2 437
pidfd_getfd 438
diff --git a/include/lapi/syscalls/x86_64.in b/include/lapi/syscalls/x86_64.in
index c76328c93..1345002f5 100644
--- a/include/lapi/syscalls/x86_64.in
+++ b/include/lapi/syscalls/x86_64.in
@@ -345,6 +345,7 @@ fsmount 432
fspick 433
pidfd_open 434
clone3 435
+close_range 436
openat2 437
pidfd_getfd 438
rt_sigaction 512
--
2.30.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [LTP] [PATCH 2/5] API: Add SAFE_DUP2
2021-02-11 11:03 [LTP] [PATCH 0/5] Add close_range01, SAFE_DUP2 and SAFE_CLONE Richard Palethorpe
2021-02-11 11:03 ` [LTP] [PATCH 1/5] close_range: Add syscall number Richard Palethorpe
@ 2021-02-11 11:03 ` Richard Palethorpe
2021-02-11 11:03 ` [LTP] [PATCH 3/5] API: Fix clone.h Richard Palethorpe
` (2 subsequent siblings)
4 siblings, 0 replies; 12+ messages in thread
From: Richard Palethorpe @ 2021-02-11 11:03 UTC (permalink / raw)
To: ltp
Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
---
include/tst_safe_macros.h | 5 +++++
lib/tst_safe_macros.c | 18 ++++++++++++++++++
2 files changed, 23 insertions(+)
diff --git a/include/tst_safe_macros.h b/include/tst_safe_macros.h
index ee3df4142..2a2b0088a 100644
--- a/include/tst_safe_macros.h
+++ b/include/tst_safe_macros.h
@@ -50,6 +50,11 @@ int safe_dup(const char *file, const int lineno, int oldfd);
#define SAFE_DUP(oldfd) \
safe_dup(__FILE__, __LINE__, (oldfd))
+int safe_dup2(const char *file, const int lineno, int oldfd, int newfd);
+
+#define SAFE_DUP2(oldfd, newfd) \
+ safe_dup2(__FILE__, __LINE__, (oldfd), (newfd))
+
#define SAFE_GETCWD(buf, size) \
safe_getcwd(__FILE__, __LINE__, NULL, (buf), (size))
diff --git a/lib/tst_safe_macros.c b/lib/tst_safe_macros.c
index aa03a6d5c..182b690bb 100644
--- a/lib/tst_safe_macros.c
+++ b/lib/tst_safe_macros.c
@@ -415,6 +415,24 @@ int safe_dup(const char *file, const int lineno, int oldfd)
return rval;
}
+int safe_dup2(const char *file, const int lineno, int oldfd, int newfd)
+{
+ int rval;
+
+ rval = dup2(oldfd, newfd);
+
+ if (rval == -1) {
+ tst_brk_(file, lineno, TBROK | TERRNO,
+ "dup2(%i, %i) failed", oldfd, newfd);
+ } else if (rval != newfd) {
+ tst_brk_(file, lineno, TBROK | TERRNO,
+ "Invalid dup2(%i, %i) return value %d",
+ oldfd, newfd, rval);
+ }
+
+ return rval;
+}
+
sighandler_t safe_signal(const char *file, const int lineno,
int signum, sighandler_t handler)
{
--
2.30.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [LTP] [PATCH 3/5] API: Fix clone.h
2021-02-11 11:03 [LTP] [PATCH 0/5] Add close_range01, SAFE_DUP2 and SAFE_CLONE Richard Palethorpe
2021-02-11 11:03 ` [LTP] [PATCH 1/5] close_range: Add syscall number Richard Palethorpe
2021-02-11 11:03 ` [LTP] [PATCH 2/5] API: Add SAFE_DUP2 Richard Palethorpe
@ 2021-02-11 11:03 ` Richard Palethorpe
2021-02-11 11:03 ` [LTP] [PATCH 4/5] API: Add tst_clone Richard Palethorpe
2021-02-11 11:03 ` [LTP] [PATCH 5/5] close_range: Add test Richard Palethorpe
4 siblings, 0 replies; 12+ messages in thread
From: Richard Palethorpe @ 2021-02-11 11:03 UTC (permalink / raw)
To: ltp
Functions in headers should be static inline. Also add stdint.h
because it results in confusing error messages if it is missing.
Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
---
include/lapi/clone.h | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/include/lapi/clone.h b/include/lapi/clone.h
index 2b8cbdbe0..81db443c9 100644
--- a/include/lapi/clone.h
+++ b/include/lapi/clone.h
@@ -10,6 +10,7 @@
#include <sys/syscall.h>
#include <linux/types.h>
#include <sched.h>
+#include <stdint.h>
#include "config.h"
#include "lapi/syscalls.h"
@@ -26,7 +27,7 @@ struct clone_args {
uint64_t __attribute__((aligned(8))) tls;
};
-int clone3(struct clone_args *args, size_t size)
+static inline int clone3(struct clone_args *args, size_t size)
{
return tst_syscall(__NR_clone3, args, size);
}
@@ -36,7 +37,7 @@ int clone3(struct clone_args *args, size_t size)
#define CLONE_PIDFD 0x00001000 /* set if a pidfd should be placed in parent */
#endif
-void clone3_supported_by_kernel(void)
+static inline void clone3_supported_by_kernel(void)
{
if ((tst_kvercmp(5, 3, 0)) < 0) {
/* Check if the syscall is backported on an older kernel */
--
2.30.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [LTP] [PATCH 4/5] API: Add tst_clone
2021-02-11 11:03 [LTP] [PATCH 0/5] Add close_range01, SAFE_DUP2 and SAFE_CLONE Richard Palethorpe
` (2 preceding siblings ...)
2021-02-11 11:03 ` [LTP] [PATCH 3/5] API: Fix clone.h Richard Palethorpe
@ 2021-02-11 11:03 ` Richard Palethorpe
2021-02-11 12:51 ` Cyril Hrubis
2021-02-11 11:03 ` [LTP] [PATCH 5/5] close_range: Add test Richard Palethorpe
4 siblings, 1 reply; 12+ messages in thread
From: Richard Palethorpe @ 2021-02-11 11:03 UTC (permalink / raw)
To: ltp
The raw clone system call and clone3 have relatively simple interfaces
if the stack pointer is set to NULL. The libc wrapper complicates
things hugely. So introduce an interface similar to clone3, but that
falls back to clone.
Not all features of clone3 are implemented in clone, but we could
either return TCONF or implement them in user land.
Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
---
include/tst_clone.h | 28 +++++++++++++++++++++++++++
include/tst_test.h | 2 +-
lib/tst_clone.c | 46 +++++++++++++++++++++++++++++++++++++++++++++
lib/tst_test.c | 25 ++++++++++++++++++++++++
4 files changed, 100 insertions(+), 1 deletion(-)
create mode 100644 lib/tst_clone.c
diff --git a/include/tst_clone.h b/include/tst_clone.h
index 88188525d..9ffdc68d1 100644
--- a/include/tst_clone.h
+++ b/include/tst_clone.h
@@ -5,6 +5,34 @@
#ifndef TST_CLONE_H__
#define TST_CLONE_H__
+#ifdef TST_TEST_H__
+
+/* The parts of clone3's clone_args we support */
+struct tst_clone_args {
+ uint64_t flags;
+ uint64_t exit_signal;
+};
+
+/* clone3 with fallbacks to clone when possible. Be aware that it
+ * returns -1 if clone3 fails (except ENOSYS), but -2 if clone fails.
+ *
+ * Without CLONE_VM this acts like fork so you may want to set
+ * tst_test.forks_child (safe_clone requires this).
+ *
+ * You should set exit_signal to SIGCHLD for
+ * tst_reap_children. Otherwise you must call wait with the
+ * appropriate parameters.
+ */
+pid_t tst_clone(const struct tst_clone_args *args);
+
+pid_t safe_clone(const char *file, const int lineno,
+ const struct tst_clone_args *args);
+
+/* "Safe" version of tst_clone */
+#define SAFE_CLONE(args) safe_clone(__FILE__, __LINE__, args)
+
+#endif /* TST_TEST_H__ */
+
/* Functions from lib/cloner.c */
int ltp_clone(unsigned long flags, int (*fn)(void *arg), void *arg,
size_t stack_size, void *stack);
diff --git a/include/tst_test.h b/include/tst_test.h
index c87251870..7dab5f761 100644
--- a/include/tst_test.h
+++ b/include/tst_test.h
@@ -29,7 +29,6 @@
#include "tst_process_state.h"
#include "tst_atomic.h"
#include "tst_kvercmp.h"
-#include "tst_clone.h"
#include "tst_kernel.h"
#include "tst_minmax.h"
#include "tst_get_bad_addr.h"
@@ -94,6 +93,7 @@ pid_t safe_fork(const char *filename, unsigned int lineno);
#include "tst_safe_macros.h"
#include "tst_safe_file_ops.h"
#include "tst_safe_net.h"
+#include "tst_clone.h"
/*
* Wait for all children and exit with TBROK if
diff --git a/lib/tst_clone.c b/lib/tst_clone.c
new file mode 100644
index 000000000..07e7f0767
--- /dev/null
+++ b/lib/tst_clone.c
@@ -0,0 +1,46 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/* Copyright (c) 2021 SUSE LLC
+ * Richard Palethorpe <rpalethorpe@suse.com>
+ */
+
+#define TST_NO_DEFAULT_MAIN
+
+#include <stddef.h>
+
+#include "tst_test.h"
+#include "lapi/clone.h"
+
+pid_t tst_clone(const struct tst_clone_args *tst_args)
+{
+ struct clone_args args = {
+ .flags = tst_args->flags,
+ .exit_signal = tst_args->exit_signal,
+ };
+ int flags;
+ pid_t pid = -1;
+
+ tst_flush();
+
+ errno = ENOSYS;
+ if (__NR_clone3 != __LTP__NR_INVALID_SYSCALL)
+ pid = syscall(__NR_clone3, &args, sizeof(args));
+
+ if (pid == -1 && errno != ENOSYS)
+ return -1;
+
+ if (pid != -1)
+ return pid;
+
+ flags = args.exit_signal | args.flags;
+
+#ifdef __s390x__
+ pid = syscall(__NR_clone, NULL, flags);
+#else
+ pid = syscall(__NR_clone, flags, NULL);
+#endif
+
+ if (pid == -1)
+ return -2;
+
+ return pid;
+}
diff --git a/lib/tst_test.c b/lib/tst_test.c
index 0714f0a0e..6bbee030b 100644
--- a/lib/tst_test.c
+++ b/lib/tst_test.c
@@ -424,6 +424,31 @@ pid_t safe_fork(const char *filename, unsigned int lineno)
return pid;
}
+pid_t safe_clone(const char *file, const int lineno,
+ const struct tst_clone_args *args)
+{
+ pid_t pid;
+
+ if (!tst_test->forks_child)
+ tst_brk(TBROK, "test.forks_child must be set!");
+
+ pid = tst_clone(args);
+
+ switch (pid) {
+ case -1:
+ tst_brk_(file, lineno, TBROK | TERRNO, "clone3 failed");
+ break;
+ case -2:
+ tst_brk_(file, lineno, TBROK | TERRNO, "clone failed");
+ return -1;
+ }
+
+ if (!pid)
+ atexit(tst_free_all);
+
+ return pid;
+}
+
static struct option {
char *optstr;
char *help;
--
2.30.0
^ permalink raw reply related [flat|nested] 12+ messages in thread* [LTP] [PATCH 4/5] API: Add tst_clone
2021-02-11 11:03 ` [LTP] [PATCH 4/5] API: Add tst_clone Richard Palethorpe
@ 2021-02-11 12:51 ` Cyril Hrubis
2021-02-11 14:24 ` Richard Palethorpe
0 siblings, 1 reply; 12+ messages in thread
From: Cyril Hrubis @ 2021-02-11 12:51 UTC (permalink / raw)
To: ltp
Hi!
> + int flags;
> + pid_t pid = -1;
> +
> + tst_flush();
> +
> + errno = ENOSYS;
> + if (__NR_clone3 != __LTP__NR_INVALID_SYSCALL)
> + pid = syscall(__NR_clone3, &args, sizeof(args));
> +
> + if (pid == -1 && errno != ENOSYS)
> + return -1;
As far as I can tell when kernel is too old we would get EINVAL because
the syscall number is not allocated. ENOSYS happens mostly when syscall
number is allocated and kernel does not implement the functionality,
e.g. it's disabled in .config.
I wonder if it's even menaningful to handle ENOSYS here, I doubt that
clone3() can be disabled, or do I miss something?
> + if (pid != -1)
> + return pid;
> +
> + flags = args.exit_signal | args.flags;
> +
> +#ifdef __s390x__
> + pid = syscall(__NR_clone, NULL, flags);
> +#else
> + pid = syscall(__NR_clone, flags, NULL);
> +#endif
> +
> + if (pid == -1)
> + return -2;
> +
> + return pid;
> +}
> diff --git a/lib/tst_test.c b/lib/tst_test.c
> index 0714f0a0e..6bbee030b 100644
> --- a/lib/tst_test.c
> +++ b/lib/tst_test.c
> @@ -424,6 +424,31 @@ pid_t safe_fork(const char *filename, unsigned int lineno)
> return pid;
> }
>
> +pid_t safe_clone(const char *file, const int lineno,
> + const struct tst_clone_args *args)
> +{
> + pid_t pid;
> +
> + if (!tst_test->forks_child)
> + tst_brk(TBROK, "test.forks_child must be set!");
> +
> + pid = tst_clone(args);
> +
> + switch (pid) {
> + case -1:
> + tst_brk_(file, lineno, TBROK | TERRNO, "clone3 failed");
> + break;
> + case -2:
> + tst_brk_(file, lineno, TBROK | TERRNO, "clone failed");
> + return -1;
> + }
> +
> + if (!pid)
> + atexit(tst_free_all);
> +
> + return pid;
> +}
> +
> static struct option {
> char *optstr;
> char *help;
> --
> 2.30.0
>
>
> --
> Mailing list info: https://lists.linux.it/listinfo/ltp
--
Cyril Hrubis
chrubis@suse.cz
^ permalink raw reply [flat|nested] 12+ messages in thread* [LTP] [PATCH 4/5] API: Add tst_clone
2021-02-11 12:51 ` Cyril Hrubis
@ 2021-02-11 14:24 ` Richard Palethorpe
2021-02-11 14:35 ` Cyril Hrubis
0 siblings, 1 reply; 12+ messages in thread
From: Richard Palethorpe @ 2021-02-11 14:24 UTC (permalink / raw)
To: ltp
Hello,
Cyril Hrubis <chrubis@suse.cz> writes:
> Hi!
>> + int flags;
>> + pid_t pid = -1;
>> +
>> + tst_flush();
>> +
>> + errno = ENOSYS;
>> + if (__NR_clone3 != __LTP__NR_INVALID_SYSCALL)
>> + pid = syscall(__NR_clone3, &args, sizeof(args));
>> +
>> + if (pid == -1 && errno != ENOSYS)
>> + return -1;
>
> As far as I can tell when kernel is too old we would get EINVAL because
> the syscall number is not allocated. ENOSYS happens mostly when syscall
> number is allocated and kernel does not implement the functionality,
> e.g. it's disabled in .config.
>
> I wonder if it's even menaningful to handle ENOSYS here, I doubt that
> clone3() can be disabled, or do I miss something?
AFAICT it should return ENOSYS if the syscall number is greater than the
current maximum. This is certainly true for riscv and also apears to be
true for arm64 and x86. It is also written in a kernel book I have from
2010 :-p
--
Thank you,
Richard.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [LTP] [PATCH 4/5] API: Add tst_clone
2021-02-11 14:24 ` Richard Palethorpe
@ 2021-02-11 14:35 ` Cyril Hrubis
2021-02-11 15:07 ` Richard Palethorpe
0 siblings, 1 reply; 12+ messages in thread
From: Cyril Hrubis @ 2021-02-11 14:35 UTC (permalink / raw)
To: ltp
Hi!
> >> + int flags;
> >> + pid_t pid = -1;
> >> +
> >> + tst_flush();
> >> +
> >> + errno = ENOSYS;
> >> + if (__NR_clone3 != __LTP__NR_INVALID_SYSCALL)
> >> + pid = syscall(__NR_clone3, &args, sizeof(args));
> >> +
> >> + if (pid == -1 && errno != ENOSYS)
> >> + return -1;
> >
> > As far as I can tell when kernel is too old we would get EINVAL because
> > the syscall number is not allocated. ENOSYS happens mostly when syscall
> > number is allocated and kernel does not implement the functionality,
> > e.g. it's disabled in .config.
> >
> > I wonder if it's even menaningful to handle ENOSYS here, I doubt that
> > clone3() can be disabled, or do I miss something?
>
> AFAICT it should return ENOSYS if the syscall number is greater than the
> current maximum. This is certainly true for riscv and also apears to be
> true for arm64 and x86. It is also written in a kernel book I have from
> 2010 :-p
Sounds sane, so we get EINVAL if the syscall number is out of the
syscall table. So I guess that we have to handle both.
--
Cyril Hrubis
chrubis@suse.cz
^ permalink raw reply [flat|nested] 12+ messages in thread
* [LTP] [PATCH 4/5] API: Add tst_clone
2021-02-11 14:35 ` Cyril Hrubis
@ 2021-02-11 15:07 ` Richard Palethorpe
2021-02-11 15:30 ` Cyril Hrubis
0 siblings, 1 reply; 12+ messages in thread
From: Richard Palethorpe @ 2021-02-11 15:07 UTC (permalink / raw)
To: ltp
Hello,
Cyril Hrubis <chrubis@suse.cz> writes:
> Hi!
>> >> + int flags;
>> >> + pid_t pid = -1;
>> >> +
>> >> + tst_flush();
>> >> +
>> >> + errno = ENOSYS;
>> >> + if (__NR_clone3 != __LTP__NR_INVALID_SYSCALL)
>> >> + pid = syscall(__NR_clone3, &args, sizeof(args));
>> >> +
>> >> + if (pid == -1 && errno != ENOSYS)
>> >> + return -1;
>> >
>> > As far as I can tell when kernel is too old we would get EINVAL because
>> > the syscall number is not allocated. ENOSYS happens mostly when syscall
>> > number is allocated and kernel does not implement the functionality,
>> > e.g. it's disabled in .config.
>> >
>> > I wonder if it's even menaningful to handle ENOSYS here, I doubt that
>> > clone3() can be disabled, or do I miss something?
>>
>> AFAICT it should return ENOSYS if the syscall number is greater than the
>> current maximum. This is certainly true for riscv and also apears to be
>> true for arm64 and x86. It is also written in a kernel book I have from
>> 2010 :-p
>
> Sounds sane, so we get EINVAL if the syscall number is out of the
> syscall table. So I guess that we have to handle both.
I don't know where you are getting EINVAL from?
Try the following
#include <errno.h>
#include <unistd.h>
#include <sys/syscall.h>
int main(int argc, const char* argv[])
{
syscall(~0ULL);
return errno;
}
It returns ENOSYS and strace shows this is not due to any sanity
checking by glibc.
I guess it would actually be a bug if it returned EINVAL although I am
not sure this is specified anywhere.
--
Thank you,
Richard.
^ permalink raw reply [flat|nested] 12+ messages in thread* [LTP] [PATCH 4/5] API: Add tst_clone
2021-02-11 15:07 ` Richard Palethorpe
@ 2021-02-11 15:30 ` Cyril Hrubis
0 siblings, 0 replies; 12+ messages in thread
From: Cyril Hrubis @ 2021-02-11 15:30 UTC (permalink / raw)
To: ltp
Hi!
> I don't know where you are getting EINVAL from?
>
> Try the following
>
> #include <errno.h>
> #include <unistd.h>
> #include <sys/syscall.h>
>
> int main(int argc, const char* argv[])
> {
> syscall(~0ULL);
>
> return errno;
> }
>
> It returns ENOSYS and strace shows this is not due to any sanity
> checking by glibc.
>
> I guess it would actually be a bug if it returned EINVAL although I am
> not sure this is specified anywhere.
And I guess that I got confused again. You get EINVAL when you pass
unsupported flags to a syscall, not unsupported syscall number.
--
Cyril Hrubis
chrubis@suse.cz
^ permalink raw reply [flat|nested] 12+ messages in thread
* [LTP] [PATCH 5/5] close_range: Add test
2021-02-11 11:03 [LTP] [PATCH 0/5] Add close_range01, SAFE_DUP2 and SAFE_CLONE Richard Palethorpe
` (3 preceding siblings ...)
2021-02-11 11:03 ` [LTP] [PATCH 4/5] API: Add tst_clone Richard Palethorpe
@ 2021-02-11 11:03 ` Richard Palethorpe
2021-02-11 12:54 ` Cyril Hrubis
4 siblings, 1 reply; 12+ messages in thread
From: Richard Palethorpe @ 2021-02-11 11:03 UTC (permalink / raw)
To: ltp
Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
---
.../kernel/syscalls/close_range/.gitignore | 1 +
.../kernel/syscalls/close_range/Makefile | 10 +
.../syscalls/close_range/close_range01.c | 208 ++++++++++++++++++
3 files changed, 219 insertions(+)
create mode 100644 testcases/kernel/syscalls/close_range/.gitignore
create mode 100644 testcases/kernel/syscalls/close_range/Makefile
create mode 100644 testcases/kernel/syscalls/close_range/close_range01.c
diff --git a/testcases/kernel/syscalls/close_range/.gitignore b/testcases/kernel/syscalls/close_range/.gitignore
new file mode 100644
index 000000000..291a0379c
--- /dev/null
+++ b/testcases/kernel/syscalls/close_range/.gitignore
@@ -0,0 +1 @@
+close_range01
\ No newline at end of file
diff --git a/testcases/kernel/syscalls/close_range/Makefile b/testcases/kernel/syscalls/close_range/Makefile
new file mode 100644
index 000000000..dc6413b10
--- /dev/null
+++ b/testcases/kernel/syscalls/close_range/Makefile
@@ -0,0 +1,10 @@
+# SPDX-License-Identifier: GPL-2.0-or-later
+# Copyright (c) 2019-2021 Linux Test Project
+
+top_srcdir ?= ../../../..
+
+include $(top_srcdir)/include/mk/testcases.mk
+
+CFLAGS += -D_GNU_SOURCE
+
+include $(top_srcdir)/include/mk/generic_leaf_target.mk
diff --git a/testcases/kernel/syscalls/close_range/close_range01.c b/testcases/kernel/syscalls/close_range/close_range01.c
new file mode 100644
index 000000000..2e3320d0f
--- /dev/null
+++ b/testcases/kernel/syscalls/close_range/close_range01.c
@@ -0,0 +1,208 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Taken from the kernel self tests, which in turn were based on
+ * a Syzkaller reproducer.
+ *
+ * Self test author and close_range author:
+ * Christian Brauner <christian.brauner@ubuntu.com>
+ *
+ * LTP Author: Richard Palethorpe <rpalethorpe@suse.com>
+ * Copyright (c) 2021 SUSE LLC, other copyrights may apply.
+ *
+ * - First we test if close_range closes some FDs.
+ * - Then we test if it UNSHARES some FDs before closing them.
+ * - Then we test if it sets CLOEXEC (in both cloned process and parent).
+ * - Finally we test a combination of CLOEXEC and UNSHARE.
+ *
+ * The final test is the actual bug reproducer. Note that we call
+ * clone directly to share the file table.
+ */
+
+#include <stdlib.h>
+
+#include "tst_test.h"
+#include "tst_clone.h"
+
+#include "lapi/clone.h"
+#include "lapi/syscalls.h"
+
+#ifndef CLOSE_RANGE_UNSHARE
+#define CLOSE_RANGE_UNSHARE (1U << 1)
+#endif
+
+#ifndef CLOSE_RANGE_CLOEXEC
+#define CLOSE_RANGE_CLOEXEC (1U << 2)
+#endif
+
+static int fd[3];
+
+static inline void do_close_range(unsigned int fd, unsigned int max_fd,
+ unsigned int flags)
+{
+ int ret = tst_syscall(__NR_close_range, fd, max_fd, flags);
+
+ if (!ret)
+ return;
+
+ if (errno == EINVAL) {
+ if (flags & CLOSE_RANGE_UNSHARE)
+ tst_brk(TCONF | TERRNO, "No CLOSE_RANGE_UNSHARE");
+ if (flags & CLOSE_RANGE_CLOEXEC)
+ tst_brk(TCONF | TERRNO, "No CLOSE_RANGE_CLOEXEC");
+ }
+
+ tst_brk(TBROK | TERRNO, "close_range(%d, %d, %d)", fd, max_fd, flags);
+}
+
+static void setup(void)
+{
+ struct rlimit nfd;
+
+ SAFE_GETRLIMIT(RLIMIT_NOFILE, &nfd);
+
+ if (nfd.rlim_max < 1000) {
+ tst_brk(TCONF, "NOFILE limit max too low: %lu < 1000",
+ nfd.rlim_max);
+ }
+
+ nfd.rlim_cur = nfd.rlim_max;
+ SAFE_SETRLIMIT(RLIMIT_NOFILE, &nfd);
+}
+
+static void check_cloexec(int i, int expected)
+{
+ int present = SAFE_FCNTL(fd[i], F_GETFD) & FD_CLOEXEC;
+
+ if (expected && !present)
+ tst_res(TFAIL, "fd[%d] flags do not contain FD_CLOEXEC", i);
+
+ if (!expected && present)
+ tst_res(TFAIL, "fd[%d] flags contain FD_CLOEXEC", i);
+}
+
+static void check_closed(int min)
+{
+ int i;
+
+ for (i = min; i < 3; i++) {
+ if (fcntl(fd[i], F_GETFD) > -1)
+ tst_res(TFAIL, "fd[%d] is still open", i);
+ }
+}
+
+static void child(unsigned int n)
+{
+ switch (n) {
+ case 0:
+ SAFE_DUP2(fd[1], fd[2]);
+ do_close_range(3, ~0U, 0);
+ check_closed(0);
+ break;
+ case 1:
+ SAFE_DUP2(fd[1], fd[2]);
+ do_close_range(3, ~0U, CLOSE_RANGE_UNSHARE);
+ check_closed(0);
+ break;
+ case 2:
+ do_close_range(3, ~0U, CLOSE_RANGE_CLOEXEC);
+ check_cloexec(0, 1);
+ check_cloexec(1, 1);
+
+ SAFE_DUP2(fd[1], fd[2]);
+ check_cloexec(2, 0);
+ break;
+ case 3:
+ do_close_range(3, ~0U,
+ CLOSE_RANGE_CLOEXEC | CLOSE_RANGE_UNSHARE);
+ check_cloexec(0, 1);
+ check_cloexec(1, 1);
+
+ SAFE_DUP2(fd[1], fd[2]);
+ check_cloexec(2, 0);
+ break;
+ }
+
+ exit(0);
+}
+
+static void run(unsigned int n)
+{
+ const struct tst_clone_args args = {
+ .flags = CLONE_FILES,
+ .exit_signal = SIGCHLD,
+ };
+
+ switch (n) {
+ case 0:
+ tst_res(TINFO, "Plain close range");
+ do_close_range(3, ~0U, 0);
+ break;
+ case 1:
+ tst_res(TINFO, "Set UNSHARE and close range");
+ do_close_range(3, ~0U, CLOSE_RANGE_UNSHARE);
+ break;
+ case 2:
+ tst_res(TINFO, "Set CLOEXEC on range");
+ do_close_range(3, ~0U, CLOSE_RANGE_CLOEXEC);
+ break;
+ case 3:
+ tst_res(TINFO, "Set UNSHARE and CLOEXEC on range");
+ do_close_range(3, ~0U,
+ CLOSE_RANGE_CLOEXEC | CLOSE_RANGE_UNSHARE);
+ break;
+ }
+
+ fd[0] = SAFE_OPEN("mnt/tmpfile", O_RDWR | O_CREAT, 0644);
+ fd[1] = SAFE_DUP2(fd[0], 1000);
+ fd[2] = 42;
+
+ if (!SAFE_CLONE(&args))
+ child(n);
+
+ tst_reap_children();
+
+ switch (n) {
+ case 0:
+ check_closed(0);
+ break;
+ case 1:
+ check_cloexec(0, 0);
+ check_cloexec(1, 0);
+ check_cloexec(2, 0);
+ break;
+ case 2:
+ check_cloexec(0, 1);
+ check_cloexec(1, 1);
+ check_cloexec(2, 0);
+ break;
+ case 3:
+ check_cloexec(0, 0);
+ check_cloexec(1, 0);
+ check_closed(2);
+ break;
+ }
+
+ do_close_range(3, ~0U, 0);
+ check_closed(0);
+
+ if (tst_taint_check())
+ tst_res(TFAIL, "Kernel tainted");
+ else
+ tst_res(TPASS, "No kernel taints");
+}
+
+static struct tst_test test = {
+ .tcnt = 4,
+ .needs_tmpdir = 1,
+ .forks_child = 1,
+ .mount_device = 1,
+ .mntpoint = "mnt",
+ .all_filesystems = 1,
+ .test = run,
+ .taint_check = TST_TAINT_W | TST_TAINT_D,
+ .setup = setup,
+ .tags = (const struct tst_tag[]) {
+ {"linux-git", "fec8a6a691033f2538cd46848f17f337f0739923"},
+ {},
+ },
+};
--
2.30.0
^ permalink raw reply related [flat|nested] 12+ messages in thread