From mboxrd@z Thu Jan 1 00:00:00 1970 From: ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org (Eric W. Biederman) Subject: [RFC][PATCH v2 5/5] signal: Don't allow accessing signal_struct by old threads after exec Date: Sun, 02 Apr 2017 17:57:46 -0500 Message-ID: <87zify76z9.fsf_-_@xmission.com> References: <20170213141452.GA30203@redhat.com> <20170224160354.GA845@redhat.com> <87shmv6ufl.fsf@xmission.com> <20170303173326.GA17899@redhat.com> <87tw7axlr0.fsf@xmission.com> <87d1dyw5iw.fsf@xmission.com> <87tw7aunuh.fsf@xmission.com> <87lgsmunmj.fsf_-_@xmission.com> <20170304170312.GB13131@redhat.com> <8760ir192p.fsf@xmission.com> <878tnkpv8h.fsf_-_@xmission.com> <874ly6a0h1.fsf_-_@xmission.com> Mime-Version: 1.0 Content-Type: text/plain Return-path: In-Reply-To: <874ly6a0h1.fsf_-_-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org> (Eric W. Biederman's message of "Sun, 02 Apr 2017 17:50:02 -0500") Sender: linux-api-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: Oleg Nesterov Cc: Andrew Morton , Aleksa Sarai , Andy Lutomirski , Attila Fazekas , Jann Horn , Kees Cook , Michal Hocko , Ulrich Obergfell , linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org List-Id: linux-api@vger.kernel.org Add exec_id to signal_struct and compare it at a few choice moments. I believe this closes the security holes that letting the zombie threads linger after exec opens up. The problem is that old threads may have different creds after a setuid exec, and then formerly shared resources may change. So signal sending and accesses by proc need to be blocked. Signed-off-by: "Eric W. Biederman" --- fs/exec.c | 1 + include/linux/sched/signal.h | 1 + kernel/fork.c | 1 + kernel/ptrace.c | 4 ++++ kernel/signal.c | 7 ++++++- 5 files changed, 13 insertions(+), 1 deletion(-) diff --git a/fs/exec.c b/fs/exec.c index 303a114b00ce..730dee8bb2f8 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -1323,6 +1323,7 @@ void setup_new_exec(struct linux_binprm * bprm) /* An exec changes our domain. We are no longer part of the thread group */ current->self_exec_id++; + current->signal->exec_id = current->self_exec_id; flush_signal_handlers(current, 0); } EXPORT_SYMBOL(setup_new_exec); diff --git a/include/linux/sched/signal.h b/include/linux/sched/signal.h index 2cf446704cd4..63ae951ee330 100644 --- a/include/linux/sched/signal.h +++ b/include/linux/sched/signal.h @@ -80,6 +80,7 @@ struct signal_struct { atomic_t live; int nr_threads; struct list_head thread_head; + u32 exec_id; wait_queue_head_t wait_chldexit; /* for wait4() */ diff --git a/kernel/fork.c b/kernel/fork.c index 0632ac1180be..a442fa099842 100644 --- a/kernel/fork.c +++ b/kernel/fork.c @@ -1387,6 +1387,7 @@ static int copy_signal(unsigned long clone_flags, struct task_struct *tsk) sig->oom_score_adj = current->signal->oom_score_adj; sig->oom_score_adj_min = current->signal->oom_score_adj_min; + sig->exec_id = current->self_exec_id; mutex_init(&sig->cred_guard_mutex); diff --git a/kernel/ptrace.c b/kernel/ptrace.c index 0af928712174..cc6b10b1ffbe 100644 --- a/kernel/ptrace.c +++ b/kernel/ptrace.c @@ -277,6 +277,10 @@ static int __ptrace_may_access(struct task_struct *task, unsigned int mode) * or halting the specified task is impossible. */ + /* Don't allow inspecting a thread after exec */ + if (task->self_exec_id != task->signal->exec_id) + return 1; + /* Don't let security modules deny introspection */ if (same_thread_group(task, current)) return 0; diff --git a/kernel/signal.c b/kernel/signal.c index fd75ba33ee3d..fe8dcdb622f5 100644 --- a/kernel/signal.c +++ b/kernel/signal.c @@ -995,6 +995,10 @@ static int __send_signal(int sig, struct siginfo *info, struct task_struct *t, from_ancestor_ns || (info == SEND_SIG_FORCED))) goto ret; + /* Don't allow thread group signals after exec */ + if (group && (t->signal->exec_id != t->self_exec_id)) + goto ret; + pending = group ? &t->signal->shared_pending : &t->pending; /* * Short-circuit ignored signals and support queuing @@ -1247,7 +1251,8 @@ struct sighand_struct *__lock_task_sighand(struct task_struct *tsk, * must see ->sighand == NULL. */ spin_lock(&sighand->siglock); - if (likely(sighand == tsk->sighand)) { + if (likely((sighand == tsk->sighand) && + (tsk->self_exec_id == tsk->signal->exec_id))) { rcu_read_unlock(); break; } -- 2.10.1 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751480AbdDBXDJ (ORCPT ); Sun, 2 Apr 2017 19:03:09 -0400 Received: from out03.mta.xmission.com ([166.70.13.233]:49502 "EHLO out03.mta.xmission.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750983AbdDBXDH (ORCPT ); Sun, 2 Apr 2017 19:03:07 -0400 From: ebiederm@xmission.com (Eric W. Biederman) To: Oleg Nesterov Cc: Andrew Morton , Aleksa Sarai , Andy Lutomirski , Attila Fazekas , Jann Horn , Kees Cook , Michal Hocko , Ulrich Obergfell , linux-kernel@vger.kernel.org, linux-api@vger.kernel.org References: <20170213141452.GA30203@redhat.com> <20170224160354.GA845@redhat.com> <87shmv6ufl.fsf@xmission.com> <20170303173326.GA17899@redhat.com> <87tw7axlr0.fsf@xmission.com> <87d1dyw5iw.fsf@xmission.com> <87tw7aunuh.fsf@xmission.com> <87lgsmunmj.fsf_-_@xmission.com> <20170304170312.GB13131@redhat.com> <8760ir192p.fsf@xmission.com> <878tnkpv8h.fsf_-_@xmission.com> <874ly6a0h1.fsf_-_@xmission.com> Date: Sun, 02 Apr 2017 17:57:46 -0500 In-Reply-To: <874ly6a0h1.fsf_-_@xmission.com> (Eric W. Biederman's message of "Sun, 02 Apr 2017 17:50:02 -0500") Message-ID: <87zify76z9.fsf_-_@xmission.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-XM-SPF: eid=1cuoWS-0002Ov-6w;;;mid=<87zify76z9.fsf_-_@xmission.com>;;;hst=in02.mta.xmission.com;;;ip=67.3.234.240;;;frm=ebiederm@xmission.com;;;spf=neutral X-XM-AID: U2FsdGVkX19w2DC6HQ6C+icl9fuBkdy5AmJiu74dvDw= X-SA-Exim-Connect-IP: 67.3.234.240 X-SA-Exim-Mail-From: ebiederm@xmission.com X-Spam-Report: * -1.0 ALL_TRUSTED Passed through trusted hosts only via SMTP * 0.0 TVD_RCVD_IP Message was received from an IP address * 1.5 TR_Symld_Words too many words that have symbols inside * 0.7 XMSubLong Long Subject * 0.8 BAYES_50 BODY: Bayes spam probability is 40 to 60% * [score: 0.5000] * -0.0 DCC_CHECK_NEGATIVE Not listed in DCC * [sa06 1397; Body=1 Fuz1=1 Fuz2=1] * 0.0 T_TooManySym_02 5+ unique symbols in subject * 1.0 XMSubMetaSx_00 1+ Sexy Words * 0.0 T_TooManySym_01 4+ unique symbols in subject X-Spam-DCC: XMission; sa06 1397; Body=1 Fuz1=1 Fuz2=1 X-Spam-Combo: ***;Oleg Nesterov X-Spam-Relay-Country: X-Spam-Timing: total 5546 ms - load_scoreonly_sql: 0.08 (0.0%), signal_user_changed: 3.4 (0.1%), b_tie_ro: 2.3 (0.0%), parse: 1.37 (0.0%), extract_message_metadata: 13 (0.2%), get_uri_detail_list: 1.95 (0.0%), tests_pri_-1000: 6 (0.1%), tests_pri_-950: 1.23 (0.0%), tests_pri_-900: 1.04 (0.0%), tests_pri_-400: 29 (0.5%), check_bayes: 28 (0.5%), b_tokenize: 12 (0.2%), b_tok_get_all: 8 (0.1%), b_comp_prob: 2.3 (0.0%), b_tok_touch_all: 3.4 (0.1%), b_finish: 0.64 (0.0%), tests_pri_0: 226 (4.1%), check_dkim_signature: 0.62 (0.0%), check_dkim_adsp: 3.2 (0.1%), tests_pri_500: 5263 (94.9%), poll_dns_idle: 5253 (94.7%), rewrite_mail: 0.00 (0.0%) Subject: [RFC][PATCH v2 5/5] signal: Don't allow accessing signal_struct by old threads after exec X-Spam-Flag: No X-SA-Exim-Version: 4.2.1 (built Thu, 05 May 2016 13:38:54 -0600) X-SA-Exim-Scanned: Yes (on in02.mta.xmission.com) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Add exec_id to signal_struct and compare it at a few choice moments. I believe this closes the security holes that letting the zombie threads linger after exec opens up. The problem is that old threads may have different creds after a setuid exec, and then formerly shared resources may change. So signal sending and accesses by proc need to be blocked. Signed-off-by: "Eric W. Biederman" --- fs/exec.c | 1 + include/linux/sched/signal.h | 1 + kernel/fork.c | 1 + kernel/ptrace.c | 4 ++++ kernel/signal.c | 7 ++++++- 5 files changed, 13 insertions(+), 1 deletion(-) diff --git a/fs/exec.c b/fs/exec.c index 303a114b00ce..730dee8bb2f8 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -1323,6 +1323,7 @@ void setup_new_exec(struct linux_binprm * bprm) /* An exec changes our domain. We are no longer part of the thread group */ current->self_exec_id++; + current->signal->exec_id = current->self_exec_id; flush_signal_handlers(current, 0); } EXPORT_SYMBOL(setup_new_exec); diff --git a/include/linux/sched/signal.h b/include/linux/sched/signal.h index 2cf446704cd4..63ae951ee330 100644 --- a/include/linux/sched/signal.h +++ b/include/linux/sched/signal.h @@ -80,6 +80,7 @@ struct signal_struct { atomic_t live; int nr_threads; struct list_head thread_head; + u32 exec_id; wait_queue_head_t wait_chldexit; /* for wait4() */ diff --git a/kernel/fork.c b/kernel/fork.c index 0632ac1180be..a442fa099842 100644 --- a/kernel/fork.c +++ b/kernel/fork.c @@ -1387,6 +1387,7 @@ static int copy_signal(unsigned long clone_flags, struct task_struct *tsk) sig->oom_score_adj = current->signal->oom_score_adj; sig->oom_score_adj_min = current->signal->oom_score_adj_min; + sig->exec_id = current->self_exec_id; mutex_init(&sig->cred_guard_mutex); diff --git a/kernel/ptrace.c b/kernel/ptrace.c index 0af928712174..cc6b10b1ffbe 100644 --- a/kernel/ptrace.c +++ b/kernel/ptrace.c @@ -277,6 +277,10 @@ static int __ptrace_may_access(struct task_struct *task, unsigned int mode) * or halting the specified task is impossible. */ + /* Don't allow inspecting a thread after exec */ + if (task->self_exec_id != task->signal->exec_id) + return 1; + /* Don't let security modules deny introspection */ if (same_thread_group(task, current)) return 0; diff --git a/kernel/signal.c b/kernel/signal.c index fd75ba33ee3d..fe8dcdb622f5 100644 --- a/kernel/signal.c +++ b/kernel/signal.c @@ -995,6 +995,10 @@ static int __send_signal(int sig, struct siginfo *info, struct task_struct *t, from_ancestor_ns || (info == SEND_SIG_FORCED))) goto ret; + /* Don't allow thread group signals after exec */ + if (group && (t->signal->exec_id != t->self_exec_id)) + goto ret; + pending = group ? &t->signal->shared_pending : &t->pending; /* * Short-circuit ignored signals and support queuing @@ -1247,7 +1251,8 @@ struct sighand_struct *__lock_task_sighand(struct task_struct *tsk, * must see ->sighand == NULL. */ spin_lock(&sighand->siglock); - if (likely(sighand == tsk->sighand)) { + if (likely((sighand == tsk->sighand) && + (tsk->self_exec_id == tsk->signal->exec_id))) { rcu_read_unlock(); break; } -- 2.10.1