From eaec2b0bd30690575c581eebffae64bfb7f684ac Mon Sep 17 00:00:00 2001 From: Zhiqiang Liu Date: Mon, 30 Mar 2020 10:18:33 +0800 Subject: [PATCH 1/5] signal: check sig before setting info in kill_pid_usb_asyncio In kill_pid_usb_asyncio, if signal is not valid, we do not need to set info struct. Signed-off-by: Zhiqiang Liu Acked-by: Christian Brauner Link: https://lore.kernel.org/r/f525fd08-1cf7-fb09-d20c-4359145eb940@huawei.com Signed-off-by: Christian Brauner --- kernel/signal.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/kernel/signal.c b/kernel/signal.c index e58a6c619824..3f94894d1253 100644 --- a/kernel/signal.c +++ b/kernel/signal.c @@ -1510,15 +1510,15 @@ int kill_pid_usb_asyncio(int sig, int errno, sigval_t addr, unsigned long flags; int ret = -EINVAL; + if (!valid_signal(sig)) + return ret; + clear_siginfo(&info); info.si_signo = sig; info.si_errno = errno; info.si_code = SI_ASYNCIO; *((sigval_t *)&info.si_pid) = addr; - if (!valid_signal(sig)) - return ret; - rcu_read_lock(); p = pid_task(pid, PIDTYPE_PID); if (!p) { From 3075afdf15b89a063f8d31c0db08a50472bb7faf Mon Sep 17 00:00:00 2001 From: Zhiqiang Liu Date: Mon, 30 Mar 2020 10:44:43 +0800 Subject: [PATCH 2/5] signal: use kill_proc_info instead of kill_pid_info in kill_something_info signal.c provides kill_proc_info, we can use it instead of kill_pid_info in kill_something_info func gracefully. Signed-off-by: Zhiqiang Liu Acked-by: Oleg Nesterov Acked-by: Christian Brauner Link: https://lore.kernel.org/r/80236965-f0b5-c888-95ff-855bdec75bb3@huawei.com Signed-off-by: Christian Brauner --- kernel/signal.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/kernel/signal.c b/kernel/signal.c index 3f94894d1253..713104884414 100644 --- a/kernel/signal.c +++ b/kernel/signal.c @@ -1557,12 +1557,8 @@ static int kill_something_info(int sig, struct kernel_siginfo *info, pid_t pid) { int ret; - if (pid > 0) { - rcu_read_lock(); - ret = kill_pid_info(sig, info, find_vpid(pid)); - rcu_read_unlock(); - return ret; - } + if (pid > 0) + return kill_proc_info(sig, info, pid); /* -INT_MIN is undefined. Exclude this case to avoid a UBSAN warning */ if (pid == INT_MIN) From e82a118f57b89bbb437ce70780fc2678d5c281e5 Mon Sep 17 00:00:00 2001 From: Eugene Syromiatnikov Date: Sun, 12 Apr 2020 22:25:33 +0200 Subject: [PATCH 3/5] clone3: fix cgroup argument sanity check Checking that cgroup field value of struct clone_args is less than 0 is useless, as it is defined as unsigned 64-bit integer. Moreover, it doesn't catch the situations where its higher bits are lost during the assignment to the cgroup field of the cgroup field of the internal struct kernel_clone_args (where it is declared as signed 32-bit integer), so it is still possible to pass garbage there. A check against INT_MAX solves both these issues. Fixes: ef2c41cf38a7559b ("clone3: allow spawning processes into cgroups") Signed-off-by: Eugene Syromiatnikov Acked-by: Christian Brauner Link: https://lore.kernel.org/r/20200412202533.GA29554@asgard.redhat.com Signed-off-by: Christian Brauner --- kernel/fork.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/fork.c b/kernel/fork.c index 4385f3d639f2..b4f7775623c8 100644 --- a/kernel/fork.c +++ b/kernel/fork.c @@ -2631,7 +2631,7 @@ noinline static int copy_clone_args_from_user(struct kernel_clone_args *kargs, !valid_signal(args.exit_signal))) return -EINVAL; - if ((args.flags & CLONE_INTO_CGROUP) && args.cgroup < 0) + if ((args.flags & CLONE_INTO_CGROUP) && args.cgroup > INT_MAX) return -EINVAL; *kargs = (struct kernel_clone_args){ From 62173872ca65767c586217dec0a32485da8a2f07 Mon Sep 17 00:00:00 2001 From: Eugene Syromiatnikov Date: Sun, 12 Apr 2020 22:31:23 +0200 Subject: [PATCH 4/5] clone3: add a check for the user struct size if CLONE_INTO_CGROUP is set Passing CLONE_INTO_CGROUP with an under-sized structure (that doesn't properly contain cgroup field) seems like garbage input, especially considering the fact that fd 0 is a valid descriptor. Signed-off-by: Eugene Syromiatnikov Acked-by: Christian Brauner Link: https://lore.kernel.org/r/20200412203123.GA5869@asgard.redhat.com Signed-off-by: Christian Brauner --- kernel/fork.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/kernel/fork.c b/kernel/fork.c index b4f7775623c8..3ab7cf88e455 100644 --- a/kernel/fork.c +++ b/kernel/fork.c @@ -2631,7 +2631,8 @@ noinline static int copy_clone_args_from_user(struct kernel_clone_args *kargs, !valid_signal(args.exit_signal))) return -EINVAL; - if ((args.flags & CLONE_INTO_CGROUP) && args.cgroup > INT_MAX) + if ((args.flags & CLONE_INTO_CGROUP) && + (args.cgroup > INT_MAX || usize < CLONE_ARGS_SIZE_VER2)) return -EINVAL; *kargs = (struct kernel_clone_args){ From a966dcfe153ab0a3d8d79cd971a079411a489be7 Mon Sep 17 00:00:00 2001 From: Eugene Syromiatnikov Date: Sun, 12 Apr 2020 22:26:58 +0200 Subject: [PATCH 5/5] clone3: add build-time CLONE_ARGS_SIZE_VER* validity checks CLONE_ARGS_SIZE_VER* macros are defined explicitly and not via the offsets of the relevant struct clone_args fields, which makes it rather error-prone, so it probably makes sense to add some compile-time checks for them (including the one that breaks on struct clone_args extension as a reminder to add a relevant size macro and a similar check). Function copy_clone_args_from_user seems to be a good place for such checks. Signed-off-by: Eugene Syromiatnikov Acked-by: Christian Brauner Link: https://lore.kernel.org/r/20200412202658.GA31499@asgard.redhat.com Signed-off-by: Christian Brauner --- kernel/fork.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/kernel/fork.c b/kernel/fork.c index 3ab7cf88e455..8c700f881d92 100644 --- a/kernel/fork.c +++ b/kernel/fork.c @@ -2605,6 +2605,14 @@ noinline static int copy_clone_args_from_user(struct kernel_clone_args *kargs, struct clone_args args; pid_t *kset_tid = kargs->set_tid; + BUILD_BUG_ON(offsetofend(struct clone_args, tls) != + CLONE_ARGS_SIZE_VER0); + BUILD_BUG_ON(offsetofend(struct clone_args, set_tid_size) != + CLONE_ARGS_SIZE_VER1); + BUILD_BUG_ON(offsetofend(struct clone_args, cgroup) != + CLONE_ARGS_SIZE_VER2); + BUILD_BUG_ON(sizeof(struct clone_args) != CLONE_ARGS_SIZE_VER2); + if (unlikely(usize > PAGE_SIZE)) return -E2BIG; if (unlikely(usize < CLONE_ARGS_SIZE_VER0))