* [LTP] [PATCH] syscalls/clone302: drop CLONE_CHILD_SETTID and CLONE_PARENT_SETTID @ 2020-08-07 10:58 Jan Stancek 2020-08-19 5:00 ` Viresh Kumar 2020-08-19 8:02 ` Christian Brauner 0 siblings, 2 replies; 4+ messages in thread From: Jan Stancek @ 2020-08-07 10:58 UTC (permalink / raw) To: ltp Per https://lore.kernel.org/linux-m68k/20200627122332.ki2otaiw3v7wndbl@wittgenstein/T/#u EFAULT isn't propagated back to userspace so these will always appear to succeed. Also issue is that multiple flags are tested together and some arguments persisted between calls, because they were set only when argument != NULL. Cc: Christian Brauner <christian.brauner@ubuntu.com> Reported-by: Naresh Kamboju <naresh.kamboju@linaro.org> Signed-off-by: Jan Stancek <jstancek@redhat.com> --- testcases/kernel/syscalls/clone3/clone302.c | 42 +++++++++++---------- 1 file changed, 23 insertions(+), 19 deletions(-) diff --git a/testcases/kernel/syscalls/clone3/clone302.c b/testcases/kernel/syscalls/clone3/clone302.c index a30df6f8286e..54d59a1f571a 100644 --- a/testcases/kernel/syscalls/clone3/clone302.c +++ b/testcases/kernel/syscalls/clone3/clone302.c @@ -21,27 +21,33 @@ static struct tcase { size_t size; uint64_t flags; int **pidfd; - int **child_tid; - int **parent_tid; int exit_signal; unsigned long stack; unsigned long stack_size; unsigned long tls; int exp_errno; } tcases[] = { - {"invalid args", &invalid_args, sizeof(*valid_args), 0, NULL, NULL, NULL, SIGCHLD, 0, 0, 0, EFAULT}, - {"zero size", &valid_args, 0, 0, NULL, NULL, NULL, SIGCHLD, 0, 0, 0, EINVAL}, - {"short size", &valid_args, sizeof(*valid_args) - 1, 0, NULL, NULL, NULL, SIGCHLD, 0, 0, 0, EINVAL}, - {"extra size", &valid_args, sizeof(*valid_args) + 1, 0, NULL, NULL, NULL, SIGCHLD, 0, 0, 0, EFAULT}, - {"sighand-no-VM", &valid_args, sizeof(*valid_args), CLONE_SIGHAND, NULL, NULL, NULL, SIGCHLD, 0, 0, 0, EINVAL}, - {"thread-no-sighand", &valid_args, sizeof(*valid_args), CLONE_THREAD, NULL, NULL, NULL, SIGCHLD, 0, 0, 0, EINVAL}, - {"fs-newns", &valid_args, sizeof(*valid_args), CLONE_FS | CLONE_NEWNS, NULL, NULL, NULL, SIGCHLD, 0, 0, 0, EINVAL}, - {"invalid pidfd", &valid_args, sizeof(*valid_args), CLONE_PARENT_SETTID | CLONE_CHILD_SETTID | CLONE_PIDFD, &invalid_address, NULL, NULL, SIGCHLD, 0, 0, 0, EFAULT}, - {"invalid childtid", &valid_args, sizeof(*valid_args), CLONE_PARENT_SETTID | CLONE_CHILD_SETTID | CLONE_PIDFD, NULL, &invalid_address, NULL, SIGCHLD, 0, 0, 0, EFAULT}, - {"invalid parenttid", &valid_args, sizeof(*valid_args), CLONE_PARENT_SETTID | CLONE_CHILD_SETTID | CLONE_PIDFD, NULL, NULL, &invalid_address, SIGCHLD, 0, 0, 0, EFAULT}, - {"invalid signal", &valid_args, sizeof(*valid_args), 0, NULL, NULL, NULL, CSIGNAL + 1, 0, 0, 0, EINVAL}, - {"zero-stack-size", &valid_args, sizeof(*valid_args), 0, NULL, NULL, NULL, SIGCHLD, (unsigned long)&stack, 0, 0, EINVAL}, - {"invalid-stack", &valid_args, sizeof(*valid_args), 0, NULL, NULL, NULL, SIGCHLD, 0, 4, 0, EINVAL}, + {"invalid args", &invalid_args, sizeof(*valid_args), 0, NULL, SIGCHLD, 0, 0, 0, EFAULT}, + {"zero size", &valid_args, 0, 0, NULL, SIGCHLD, 0, 0, 0, EINVAL}, + {"short size", &valid_args, sizeof(*valid_args) - 1, 0, NULL, SIGCHLD, 0, 0, 0, EINVAL}, + {"extra size", &valid_args, sizeof(*valid_args) + 1, 0, NULL, SIGCHLD, 0, 0, 0, EFAULT}, + {"sighand-no-VM", &valid_args, sizeof(*valid_args), CLONE_SIGHAND, NULL, SIGCHLD, 0, 0, 0, EINVAL}, + {"thread-no-sighand", &valid_args, sizeof(*valid_args), CLONE_THREAD, NULL, SIGCHLD, 0, 0, 0, EINVAL}, + {"fs-newns", &valid_args, sizeof(*valid_args), CLONE_FS | CLONE_NEWNS, NULL, SIGCHLD, 0, 0, 0, EINVAL}, + {"invalid pidfd", &valid_args, sizeof(*valid_args), CLONE_PIDFD, &invalid_address, SIGCHLD, 0, 0, 0, EFAULT}, + {"invalid signal", &valid_args, sizeof(*valid_args), 0, NULL, CSIGNAL + 1, 0, 0, 0, EINVAL}, + {"zero-stack-size", &valid_args, sizeof(*valid_args), 0, NULL, SIGCHLD, (unsigned long)&stack, 0, 0, EINVAL}, + {"invalid-stack", &valid_args, sizeof(*valid_args), 0, NULL, SIGCHLD, 0, 4, 0, EINVAL}, + /* + * Don't test CLONE_CHILD_SETTID and CLONE_PARENT_SETTID: + * When the parent tid is written to the memory location for + * CLONE_PARENT_SETTID we're past the point of no return of process + * creation, i.e. the return value from put_user() isn't checked and + * can't be checked anymore so you'd never receive EFAULT for a bogus + * parent_tid memory address. + * + * https://lore.kernel.org/linux-m68k/20200627122332.ki2otaiw3v7wndbl@wittgenstein/T/#u + */ }; static void setup(void) @@ -63,10 +69,8 @@ static void run(unsigned int n) args->flags = tc->flags; if (tc->pidfd) args->pidfd = (uint64_t)(*tc->pidfd); - if (tc->child_tid) - args->child_tid = (uint64_t)(*tc->child_tid); - if (tc->parent_tid) - args->parent_tid = (uint64_t)(*tc->parent_tid); + else + args->pidfd = 0; args->exit_signal = tc->exit_signal; args->stack = tc->stack; args->stack_size = tc->stack_size; -- 2.18.1 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* [LTP] [PATCH] syscalls/clone302: drop CLONE_CHILD_SETTID and CLONE_PARENT_SETTID 2020-08-07 10:58 [LTP] [PATCH] syscalls/clone302: drop CLONE_CHILD_SETTID and CLONE_PARENT_SETTID Jan Stancek @ 2020-08-19 5:00 ` Viresh Kumar 2020-08-19 8:02 ` Christian Brauner 1 sibling, 0 replies; 4+ messages in thread From: Viresh Kumar @ 2020-08-19 5:00 UTC (permalink / raw) To: ltp On 07-08-20, 12:58, Jan Stancek wrote: > Per https://lore.kernel.org/linux-m68k/20200627122332.ki2otaiw3v7wndbl@wittgenstein/T/#u > EFAULT isn't propagated back to userspace so these will always appear > to succeed. Also issue is that multiple flags are tested together > and some arguments persisted between calls, because they were set > only when argument != NULL. > > Cc: Christian Brauner <christian.brauner@ubuntu.com> > Reported-by: Naresh Kamboju <naresh.kamboju@linaro.org> > Signed-off-by: Jan Stancek <jstancek@redhat.com> > --- > testcases/kernel/syscalls/clone3/clone302.c | 42 +++++++++++---------- > 1 file changed, 23 insertions(+), 19 deletions(-) Acked-by: Viresh Kumar <viresh.kumar@linaro.org> -- viresh ^ permalink raw reply [flat|nested] 4+ messages in thread
* [LTP] [PATCH] syscalls/clone302: drop CLONE_CHILD_SETTID and CLONE_PARENT_SETTID 2020-08-07 10:58 [LTP] [PATCH] syscalls/clone302: drop CLONE_CHILD_SETTID and CLONE_PARENT_SETTID Jan Stancek 2020-08-19 5:00 ` Viresh Kumar @ 2020-08-19 8:02 ` Christian Brauner 2020-08-19 12:36 ` Jan Stancek 1 sibling, 1 reply; 4+ messages in thread From: Christian Brauner @ 2020-08-19 8:02 UTC (permalink / raw) To: ltp On Fri, Aug 07, 2020 at 12:58:44PM +0200, Jan Stancek wrote: > Per https://lore.kernel.org/linux-m68k/20200627122332.ki2otaiw3v7wndbl@wittgenstein/T/#u > EFAULT isn't propagated back to userspace so these will always appear > to succeed. Also issue is that multiple flags are tested together > and some arguments persisted between calls, because they were set > only when argument != NULL. > > Cc: Christian Brauner <christian.brauner@ubuntu.com> > Reported-by: Naresh Kamboju <naresh.kamboju@linaro.org> > Signed-off-by: Jan Stancek <jstancek@redhat.com> > --- Thanks! Acked-by: Christian Brauner <christian.brauner@ubuntu.com> > testcases/kernel/syscalls/clone3/clone302.c | 42 +++++++++++---------- > 1 file changed, 23 insertions(+), 19 deletions(-) > > diff --git a/testcases/kernel/syscalls/clone3/clone302.c b/testcases/kernel/syscalls/clone3/clone302.c > index a30df6f8286e..54d59a1f571a 100644 > --- a/testcases/kernel/syscalls/clone3/clone302.c > +++ b/testcases/kernel/syscalls/clone3/clone302.c > @@ -21,27 +21,33 @@ static struct tcase { > size_t size; > uint64_t flags; > int **pidfd; > - int **child_tid; > - int **parent_tid; > int exit_signal; > unsigned long stack; > unsigned long stack_size; > unsigned long tls; > int exp_errno; > } tcases[] = { > - {"invalid args", &invalid_args, sizeof(*valid_args), 0, NULL, NULL, NULL, SIGCHLD, 0, 0, 0, EFAULT}, > - {"zero size", &valid_args, 0, 0, NULL, NULL, NULL, SIGCHLD, 0, 0, 0, EINVAL}, > - {"short size", &valid_args, sizeof(*valid_args) - 1, 0, NULL, NULL, NULL, SIGCHLD, 0, 0, 0, EINVAL}, > - {"extra size", &valid_args, sizeof(*valid_args) + 1, 0, NULL, NULL, NULL, SIGCHLD, 0, 0, 0, EFAULT}, > - {"sighand-no-VM", &valid_args, sizeof(*valid_args), CLONE_SIGHAND, NULL, NULL, NULL, SIGCHLD, 0, 0, 0, EINVAL}, > - {"thread-no-sighand", &valid_args, sizeof(*valid_args), CLONE_THREAD, NULL, NULL, NULL, SIGCHLD, 0, 0, 0, EINVAL}, > - {"fs-newns", &valid_args, sizeof(*valid_args), CLONE_FS | CLONE_NEWNS, NULL, NULL, NULL, SIGCHLD, 0, 0, 0, EINVAL}, > - {"invalid pidfd", &valid_args, sizeof(*valid_args), CLONE_PARENT_SETTID | CLONE_CHILD_SETTID | CLONE_PIDFD, &invalid_address, NULL, NULL, SIGCHLD, 0, 0, 0, EFAULT}, > - {"invalid childtid", &valid_args, sizeof(*valid_args), CLONE_PARENT_SETTID | CLONE_CHILD_SETTID | CLONE_PIDFD, NULL, &invalid_address, NULL, SIGCHLD, 0, 0, 0, EFAULT}, > - {"invalid parenttid", &valid_args, sizeof(*valid_args), CLONE_PARENT_SETTID | CLONE_CHILD_SETTID | CLONE_PIDFD, NULL, NULL, &invalid_address, SIGCHLD, 0, 0, 0, EFAULT}, > - {"invalid signal", &valid_args, sizeof(*valid_args), 0, NULL, NULL, NULL, CSIGNAL + 1, 0, 0, 0, EINVAL}, > - {"zero-stack-size", &valid_args, sizeof(*valid_args), 0, NULL, NULL, NULL, SIGCHLD, (unsigned long)&stack, 0, 0, EINVAL}, > - {"invalid-stack", &valid_args, sizeof(*valid_args), 0, NULL, NULL, NULL, SIGCHLD, 0, 4, 0, EINVAL}, > + {"invalid args", &invalid_args, sizeof(*valid_args), 0, NULL, SIGCHLD, 0, 0, 0, EFAULT}, > + {"zero size", &valid_args, 0, 0, NULL, SIGCHLD, 0, 0, 0, EINVAL}, > + {"short size", &valid_args, sizeof(*valid_args) - 1, 0, NULL, SIGCHLD, 0, 0, 0, EINVAL}, > + {"extra size", &valid_args, sizeof(*valid_args) + 1, 0, NULL, SIGCHLD, 0, 0, 0, EFAULT}, > + {"sighand-no-VM", &valid_args, sizeof(*valid_args), CLONE_SIGHAND, NULL, SIGCHLD, 0, 0, 0, EINVAL}, > + {"thread-no-sighand", &valid_args, sizeof(*valid_args), CLONE_THREAD, NULL, SIGCHLD, 0, 0, 0, EINVAL}, > + {"fs-newns", &valid_args, sizeof(*valid_args), CLONE_FS | CLONE_NEWNS, NULL, SIGCHLD, 0, 0, 0, EINVAL}, > + {"invalid pidfd", &valid_args, sizeof(*valid_args), CLONE_PIDFD, &invalid_address, SIGCHLD, 0, 0, 0, EFAULT}, > + {"invalid signal", &valid_args, sizeof(*valid_args), 0, NULL, CSIGNAL + 1, 0, 0, 0, EINVAL}, > + {"zero-stack-size", &valid_args, sizeof(*valid_args), 0, NULL, SIGCHLD, (unsigned long)&stack, 0, 0, EINVAL}, > + {"invalid-stack", &valid_args, sizeof(*valid_args), 0, NULL, SIGCHLD, 0, 4, 0, EINVAL}, > + /* > + * Don't test CLONE_CHILD_SETTID and CLONE_PARENT_SETTID: > + * When the parent tid is written to the memory location for > + * CLONE_PARENT_SETTID we're past the point of no return of process > + * creation, i.e. the return value from put_user() isn't checked and > + * can't be checked anymore so you'd never receive EFAULT for a bogus > + * parent_tid memory address. > + * > + * https://lore.kernel.org/linux-m68k/20200627122332.ki2otaiw3v7wndbl@wittgenstein/T/#u > + */ > }; > > static void setup(void) > @@ -63,10 +69,8 @@ static void run(unsigned int n) > args->flags = tc->flags; > if (tc->pidfd) > args->pidfd = (uint64_t)(*tc->pidfd); > - if (tc->child_tid) > - args->child_tid = (uint64_t)(*tc->child_tid); > - if (tc->parent_tid) > - args->parent_tid = (uint64_t)(*tc->parent_tid); > + else > + args->pidfd = 0; > args->exit_signal = tc->exit_signal; > args->stack = tc->stack; > args->stack_size = tc->stack_size; > -- > 2.18.1 > ^ permalink raw reply [flat|nested] 4+ messages in thread
* [LTP] [PATCH] syscalls/clone302: drop CLONE_CHILD_SETTID and CLONE_PARENT_SETTID 2020-08-19 8:02 ` Christian Brauner @ 2020-08-19 12:36 ` Jan Stancek 0 siblings, 0 replies; 4+ messages in thread From: Jan Stancek @ 2020-08-19 12:36 UTC (permalink / raw) To: ltp ----- Original Message ----- > On Fri, Aug 07, 2020 at 12:58:44PM +0200, Jan Stancek wrote: > > Per > > https://lore.kernel.org/linux-m68k/20200627122332.ki2otaiw3v7wndbl@wittgenstein/T/#u > > EFAULT isn't propagated back to userspace so these will always appear > > to succeed. Also issue is that multiple flags are tested together > > and some arguments persisted between calls, because they were set > > only when argument != NULL. > > > > Cc: Christian Brauner <christian.brauner@ubuntu.com> > > Reported-by: Naresh Kamboju <naresh.kamboju@linaro.org> > > Signed-off-by: Jan Stancek <jstancek@redhat.com> > > --- > > Thanks! > Acked-by: Christian Brauner <christian.brauner@ubuntu.com> Pushed. ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-08-19 12:36 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-08-07 10:58 [LTP] [PATCH] syscalls/clone302: drop CLONE_CHILD_SETTID and CLONE_PARENT_SETTID Jan Stancek 2020-08-19 5:00 ` Viresh Kumar 2020-08-19 8:02 ` Christian Brauner 2020-08-19 12:36 ` Jan Stancek
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox