| From: "Eric W. Biederman" <ebiederm@xmission.com> |
| Date: Mon, 30 Mar 2020 19:01:04 -0500 |
| Subject: signal: Extend exec_id to 64bits |
| |
| commit d1e7fd6462ca9fc76650fbe6ca800e35b24267da upstream. |
| |
| Replace the 32bit exec_id with a 64bit exec_id to make it impossible |
| to wrap the exec_id counter. With care an attacker can cause exec_id |
| wrap and send arbitrary signals to a newly exec'd parent. This |
| bypasses the signal sending checks if the parent changes their |
| credentials during exec. |
| |
| The severity of this problem can been seen that in my limited testing |
| of a 32bit exec_id it can take as little as 19s to exec 65536 times. |
| Which means that it can take as little as 14 days to wrap a 32bit |
| exec_id. Adam Zabrocki has succeeded wrapping the self_exe_id in 7 |
| days. Even my slower timing is in the uptime of a typical server. |
| Which means self_exec_id is simply a speed bump today, and if exec |
| gets noticably faster self_exec_id won't even be a speed bump. |
| |
| Extending self_exec_id to 64bits introduces a problem on 32bit |
| architectures where reading self_exec_id is no longer atomic and can |
| take two read instructions. Which means that is is possible to hit |
| a window where the read value of exec_id does not match the written |
| value. So with very lucky timing after this change this still |
| remains expoiltable. |
| |
| I have updated the update of exec_id on exec to use WRITE_ONCE |
| and the read of exec_id in do_notify_parent to use READ_ONCE |
| to make it clear that there is no locking between these two |
| locations. |
| |
| Link: https://lore.kernel.org/kernel-hardening/20200324215049.GA3710@pi3.com.pl |
| Fixes: 2.3.23pre2 |
| Cc: stable@vger.kernel.org |
| Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com> |
| [bwh: Backported to 3.16: |
| - Use ACCESS_ONCE() |
| - Adjust context] |
| Signed-off-by: Ben Hutchings <ben@decadent.org.uk> |
| --- |
| fs/exec.c | 2 +- |
| include/linux/sched.h | 4 ++-- |
| kernel/signal.c | 2 +- |
| 3 files changed, 4 insertions(+), 4 deletions(-) |
| |
| --- a/fs/exec.c |
| +++ b/fs/exec.c |
| @@ -1182,7 +1182,7 @@ void setup_new_exec(struct linux_binprm |
| |
| /* An exec changes our domain. We are no longer part of the thread |
| group */ |
| - current->self_exec_id++; |
| + ACCESS_ONCE(current->self_exec_id) = current->self_exec_id + 1; |
| flush_signal_handlers(current, 0); |
| } |
| EXPORT_SYMBOL(setup_new_exec); |
| --- a/include/linux/sched.h |
| +++ b/include/linux/sched.h |
| @@ -1427,8 +1427,8 @@ struct task_struct { |
| struct seccomp seccomp; |
| |
| /* Thread group tracking */ |
| - u32 parent_exec_id; |
| - u32 self_exec_id; |
| + u64 parent_exec_id; |
| + u64 self_exec_id; |
| /* Protection of (de-)allocation: mm, files, fs, tty, keyrings, mems_allowed, |
| * mempolicy */ |
| spinlock_t alloc_lock; |
| --- a/kernel/signal.c |
| +++ b/kernel/signal.c |
| @@ -1679,7 +1679,7 @@ bool do_notify_parent(struct task_struct |
| * This is only possible if parent == real_parent. |
| * Check if it has changed security domain. |
| */ |
| - if (tsk->parent_exec_id != tsk->parent->self_exec_id) |
| + if (tsk->parent_exec_id != ACCESS_ONCE(tsk->parent->self_exec_id)) |
| sig = SIGCHLD; |
| } |
| |