Hi Adrian,
On 7/31/19 5:12 PM, Adrian Reber wrote:
[..]
> @@ -2530,14 +2530,12 @@ noinline static int copy_clone_args_from_user(struct
> kernel_clone_args *kargs,
> struct clone_args __user *uargs,
> size_t size)
> {
> + struct pid_namespace *pid_ns = task_active_pid_ns(current);
> struct clone_args args;
>
> if (unlikely(size > PAGE_SIZE))
> return -E2BIG;
>
> - if (unlikely(size < sizeof(struct clone_args)))
> - return -EINVAL;
> -
It might be better to validate it still somehow, but I don't insist.
[..]
> @@ -2578,11 +2580,16 @@ noinline static int copy_clone_args_from_user(struct
> kernel_clone_args *kargs,
>
> static bool clone3_args_valid(const struct kernel_clone_args *kargs)
> {
> - /*
> - * All lower bits of the flag word are taken.
> - * Verify that no other unknown flags are passed along.
> - */
> - if (kargs->flags & ~CLONE_LEGACY_FLAGS)
> + /* Verify that no other unknown flags are passed along. */
> + if (kargs->flags & ~(CLONE_LEGACY_FLAGS | CLONE_SET_TID))
> + return false;
> +
> + /* Fail if set_tid is set without CLONE_SET_TID */
> + if (kargs->set_tid && !(kargs->flags & CLONE_SET_TID))
> + return false;
> +
> + /* Also fail if set_tid is invalid */
> + if ((kargs->set_tid <= 0) && (kargs->flags & CLONE_SET_TID))
> return false;
Sorry for not mentioning it on v1, but I've noticed it only now:
you check kargs->set_tid even with the legacy-sized kernel_clone_args,
which is probably some random value on a task's stack?
[..]
Thanks much,
Dmitry
