From: Christian Brauner <christian.brauner@ubuntu.com>
To: Szabolcs Nagy <Szabolcs.Nagy@arm.com>,
Florian Weimer <fweimer@redhat.com>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
GNU C Library <libc-alpha@sourceware.org>, nd <nd@arm.com>,
Arnd Bergmann <arnd@arndb.de>, Kees Cook <keescook@chromium.org>,
Jann Horn <jannh@google.com>, David Howells <dhowells@redhat.com>,
Ingo Molnar <mingo@redhat.com>, Oleg Nesterov <oleg@redhat.com>,
Linus Torvalds <torvalds@linux-foundation.org>,
Peter Zijlstra <peterz@infradead.org>,
"linux-api@vger.kernel.org" <linux-api@vger.kernel.org>,
"stable@vger.kernel.org" <stable@vger.kernel.org>
Subject: Re: [PATCH] clone3: validate stack arguments
Date: Fri, 1 Nov 2019 16:10:14 +0100 [thread overview]
Message-ID: <20191101151014.75jfcdc3hrwt6ssv@wittgenstein> (raw)
In-Reply-To: <f51d97a2-8130-0ba2-c591-638b7fd6b14d@arm.com>
On Fri, Nov 01, 2019 at 02:57:10PM +0000, Szabolcs Nagy wrote:
> On 31/10/2019 11:36, Christian Brauner wrote:
> > diff --git a/include/uapi/linux/sched.h b/include/uapi/linux/sched.h
> > index 99335e1f4a27..25b4fa00bad1 100644
> > --- a/include/uapi/linux/sched.h
> > +++ b/include/uapi/linux/sched.h
> > @@ -51,6 +51,10 @@
> > * sent when the child exits.
> > * @stack: Specify the location of the stack for the
> > * child process.
> > + * Note, @stack is expected to point to the
> > + * lowest address. The stack direction will be
> > + * determined by the kernel and set up
> > + * appropriately based on @stack_size.
> > * @stack_size: The size of the stack for the child process.
> > * @tls: If CLONE_SETTLS is set, the tls descriptor
> > * is set to tls.
> > diff --git a/kernel/fork.c b/kernel/fork.c
> > index bcdf53125210..55af6931c6ec 100644
> > --- a/kernel/fork.c
> > +++ b/kernel/fork.c
> > @@ -2561,7 +2561,35 @@ noinline static int copy_clone_args_from_user(struct kernel_clone_args *kargs,
> > return 0;
> > }
> >
> > -static bool clone3_args_valid(const struct kernel_clone_args *kargs)
> > +/**
> > + * clone3_stack_valid - check and prepare stack
> > + * @kargs: kernel clone args
> > + *
> > + * Verify that the stack arguments userspace gave us are sane.
> > + * In addition, set the stack direction for userspace since it's easy for us to
> > + * determine.
> > + */
> > +static inline bool clone3_stack_valid(struct kernel_clone_args *kargs)
> > +{
> > + if (kargs->stack == 0) {
> > + if (kargs->stack_size > 0)
> > + return false;
> > + } else {
> > + if (kargs->stack_size == 0)
> > + return false;
> > +
> > + if (!access_ok((void __user *)kargs->stack, kargs->stack_size))
> > + return false;
> > +
> > +#if !defined(CONFIG_STACK_GROWSUP) && !defined(CONFIG_IA64)
> > + kargs->stack += kargs->stack_size;
> > +#endif
> > + }
>
> from the description it is not clear whose
> responsibility it is to guarantee the alignment
> of sp on entry.
Userspace.
>
> i think 0 stack size may work if signals are
> blocked and then prohibiting it might not be
> the right thing.
Note that stack size 0 is allowed:
struct clone_args args = {
.exit_signal = SIGCHLD,
};
clone3(&args, sizeof(args));
will just work fine.
>
> it's not clear how libc should deal with v5.3
> kernels which don't have the stack+=stack_size
> logic.
stable is already Cced and the change will be backported to v5.3.
Nearly all distros track pull in stable updates.
Florian, thoughts on this?
Christian
next prev parent reply other threads:[~2019-11-01 15:10 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-10-31 11:36 [PATCH] clone3: validate stack arguments Christian Brauner
2019-10-31 11:41 ` David Laight
2019-10-31 13:59 ` Aleksa Sarai
2019-10-31 14:27 ` David Laight
2019-10-31 16:46 ` Oleg Nesterov
2019-11-01 11:06 ` Christian Brauner
2019-11-01 12:32 ` Oleg Nesterov
2019-11-01 14:40 ` Christian Brauner
2019-11-01 14:57 ` Szabolcs Nagy
2019-11-01 15:10 ` Christian Brauner [this message]
2019-11-05 14:23 ` Arnd Bergmann
2019-11-05 20:23 ` Michael Kerrisk (man-pages)
[not found] ` <20191031124037.E26AF20650@mail.kernel.org>
2019-11-06 2:18 ` Christian Brauner
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20191101151014.75jfcdc3hrwt6ssv@wittgenstein \
--to=christian.brauner@ubuntu.com \
--cc=Szabolcs.Nagy@arm.com \
--cc=arnd@arndb.de \
--cc=dhowells@redhat.com \
--cc=fweimer@redhat.com \
--cc=jannh@google.com \
--cc=keescook@chromium.org \
--cc=libc-alpha@sourceware.org \
--cc=linux-api@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=nd@arm.com \
--cc=oleg@redhat.com \
--cc=peterz@infradead.org \
--cc=stable@vger.kernel.org \
--cc=torvalds@linux-foundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).