Commit 26bc6721 authored Nov 05, 2019 by Linus Torvalds

linux

Pull clone3 stack argument update from Christian Brauner:
"This changes clone3() to do basic stack validation and to set up the
stack depending on whether or not it is growing up or down.

With clone3() the expectation is now very simply that the .stack
argument points to the lowest address of the stack and that
.stack_size specifies the initial stack size. This is diferent from
legacy clone() where the "stack" argument had to point to the lowest
or highest address of the stack depending on the architecture.

clone3() was released with 5.3. Currently, it is not documented and
very unclear to userspace how the stack and stack_size argument have
to be passed. After talking to glibc folks we concluded that changing
clone3() to determine stack direction and doing basic validation is
the right course of action.

Note, this is a potentially user visible change. In the very unlikely
case, that it breaks someone's use-case we will revert. (And then e.g.
place the new behavior under an appropriate flag.)

Note that passing an empty stack will continue working just as before.
Breaking someone's use-case is very unlikely. Neither glibc nor musl
currently expose a wrapper for clone3(). There is currently also no
real motivation for anyone to use clone3() directly. First, because
using clone{3}() with stacks requires some assembly (see glibc and
musl). Second, because it does not provide features that legacy
clone() doesn't. New features for clone3() will first happen in v5.5
which is why v5.4 is still a good time to try and make that change now
and backport it to v5.3.

I did a codesearch on https://codesearch.debian.net, github, and
gitlab and could not find any software currently relying directly on
clone3(). I expect this to change once we land CLONE_CLEAR_SIGHAND
which was a request coming from glibc at which point they'll likely
start using it"

* tag 'for-linus-2019-11-05' of git://git.kernel.org/pub/scm/linux/kernel/git/brauner/linux:
clone3: validate stack arguments

parents 7111fa11 fa729c4d

include/uapi/linux/sched.h

+4 −0

Original line number	Diff line number	Diff line
		@@ -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.

kernel/fork.c

+32 −1

Original line number	Diff line number	Diff line
		@@ -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
		}

		return true;
		}

		static bool clone3_args_valid(struct kernel_clone_args *kargs)
		{
		/*
		* All lower bits of the flag word are taken.
		@@ -2581,6 +2609,9 @@ static bool clone3_args_valid(const struct kernel_clone_args *kargs)
		kargs->exit_signal)
		return false;

		if (!clone3_stack_valid(kargs))
		return false;

		return true;
		}

Admin message