From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933478AbdC3IOn (ORCPT ); Thu, 30 Mar 2017 04:14:43 -0400 Received: from out02.mta.xmission.com ([166.70.13.232]:33855 "EHLO out02.mta.xmission.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933526AbdC3IM5 (ORCPT ); Thu, 30 Mar 2017 04:12:57 -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> Date: Thu, 30 Mar 2017 03:07:42 -0500 In-Reply-To: <20170304170312.GB13131@redhat.com> (Oleg Nesterov's message of "Sat, 4 Mar 2017 18:03:13 +0100") Message-ID: <8760ir192p.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=1ctVCO-0007or-Ne;;;mid=<8760ir192p.fsf@xmission.com>;;;hst=in01.mta.xmission.com;;;ip=67.3.234.240;;;frm=ebiederm@xmission.com;;;spf=neutral X-XM-AID: U2FsdGVkX192hatHobmeOvbzvyYYIDSVPuoN7S5mTp4= 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.7 XMSubLong Long Subject * 1.5 TR_Symld_Words too many words that have symbols inside * 0.0 TVD_RCVD_IP Message was received from an IP address * 0.0 T_TM2_M_HEADER_IN_MSG BODY: No description available. * 0.8 BAYES_50 BODY: Bayes spam probability is 40 to 60% * [score: 0.5000] * -0.0 DCC_CHECK_NEGATIVE Not listed in DCC * [sa01 1397; Body=1 Fuz1=1 Fuz2=1] * 1.0 T_XMHurry_00 Hurry and Do Something * 0.0 T_TooManySym_01 4+ unique symbols in subject X-Spam-DCC: XMission; sa01 1397; Body=1 Fuz1=1 Fuz2=1 X-Spam-Combo: ***;Oleg Nesterov X-Spam-Relay-Country: X-Spam-Timing: total 1741 ms - load_scoreonly_sql: 0.06 (0.0%), signal_user_changed: 3.3 (0.2%), b_tie_ro: 2.2 (0.1%), parse: 1.37 (0.1%), extract_message_metadata: 6 (0.3%), get_uri_detail_list: 2.9 (0.2%), tests_pri_-1000: 7 (0.4%), tests_pri_-950: 2.0 (0.1%), tests_pri_-900: 1.76 (0.1%), tests_pri_-400: 34 (2.0%), check_bayes: 32 (1.9%), b_tokenize: 13 (0.7%), b_tok_get_all: 8 (0.5%), b_comp_prob: 4.8 (0.3%), b_tok_touch_all: 2.8 (0.2%), b_finish: 0.82 (0.0%), tests_pri_0: 1657 (95.2%), check_dkim_signature: 0.92 (0.1%), check_dkim_adsp: 4.9 (0.3%), tests_pri_500: 7 (0.4%), rewrite_mail: 0.00 (0.0%) Subject: Re: [RFC][PATCH] exec: Don't wait for ptraced threads to be reaped. 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 in01.mta.xmission.com) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Oleg Nesterov writes: > On 03/03, Eric W. Biederman wrote: >> @@ -1065,11 +1065,8 @@ static int de_thread(struct task_struct *tsk) >> } >> >> sig->group_exit_task = tsk; >> - sig->notify_count = zap_other_threads(tsk); >> - if (!thread_group_leader(tsk)) >> - sig->notify_count--; >> - >> - while (sig->notify_count) { >> + zap_other_threads(tsk); >> + while (atomic_read(&sig->live) > 1) { >> __set_current_state(TASK_KILLABLE); >> spin_unlock_irq(lock); >> schedule(); > > Very nice. So de_thread() returns as soon as all other threads decrement > signal->live in do_exit(). Before they do, say, exit_mm(). This is already > wrong, for example this breaks OOM. Plus a lot more problems afaics, but > lets ignore this. Which means that we need to keep sig->notify_count. > Note that de_thread() also unshares ->sighand before return. So in the > case of mt exec it will likely see oldsighand->count != 1 and alloc the > new sighand_struct and this breaks the locking. > > Because the execing thread will use newsighand->siglock to protect its > signal_struct while the zombie threads will use oldsighand->siglock to > protect the same signal struct. Yes, tasklist_lock + the fact irq_disable > implies rcu_lock mostly save us but not entirely, say, a foreign process > doing __send_signal() can take the right or the wrong lock depending on > /dev/random. Which leads to the question how can we get back tot he 2.4 behavior of freeing sighand_struct in do_exit? At which point as soon as we free sighand_struct if we are the last to dying thread notify de_thread and everything works. There are only two uses of sighand->siglock that I can see in release_task: __ptrace_unlink and __exit_signal. For what __ptrace_unlink is doing we should just be able to skip acquiring of siglock if PF_EXITING is set. __exit_signal is a little more interesting but half of what it is doing looks like it was pulled out of do_exit and just needs to be put back. Which probably adds up to 4 or 5 small carefully written patches to sort out that part of the exit path, but I think it leads to a very simple and straight forward result. With the only side effect that we can exec a process and still have a debugger reaping zombie threads. Oleg does that sound reasonable to you? Eric