From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751440AbdGRNUk (ORCPT ); Tue, 18 Jul 2017 09:20:40 -0400 Received: from out03.mta.xmission.com ([166.70.13.233]:56005 "EHLO out03.mta.xmission.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751399AbdGRNUh (ORCPT ); Tue, 18 Jul 2017 09:20:37 -0400 From: ebiederm@xmission.com (Eric W. Biederman) To: Kees Cook Cc: Linus Torvalds , Andy Lutomirski , David Howells , Serge Hallyn , John Johansen , Casey Schaufler , Alexander Viro , Michal Hocko , Ben Hutchings , Hugh Dickins , Oleg Nesterov , "Jason A. Donenfeld" , Rik van Riel , James Morris , Greg Ungerer , Ingo Molnar , Nicolas Pitre , Stephen Smalley , Paul Moore , Vivek Goyal , =?utf-8?Q?Micka=C3=ABl_Sala=C3=BCn?= , Tetsuo Handa , "linux-fsdevel\@vger.kernel.org" , LKML , "\" , SE Linux References: <1499673451-66160-1-git-send-email-keescook@chromium.org> <1499673451-66160-2-git-send-email-keescook@chromium.org> <87van0r86d.fsf@xmission.com> <87pod8mdad.fsf@xmission.com> Date: Tue, 18 Jul 2017 08:12:22 -0500 In-Reply-To: (Kees Cook's message of "Mon, 17 Jul 2017 23:39:43 -0700") Message-ID: <87lgnlkhxl.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=1dXSQH-0004jk-H4;;;mid=<87lgnlkhxl.fsf@xmission.com>;;;hst=in02.mta.xmission.com;;;ip=67.3.213.87;;;frm=ebiederm@xmission.com;;;spf=neutral X-XM-AID: U2FsdGVkX1/0YKQ1p+RfDcyXj4yPG5atSzTRwgHYR0o= X-SA-Exim-Connect-IP: 67.3.213.87 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 * 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 * [sa06 1397; Body=1 Fuz1=1 Fuz2=1] * 0.2 T_XMDrugObfuBody_14 obfuscated drug references * 0.1 XMSolicitRefs_0 Weightloss drug X-Spam-DCC: XMission; sa06 1397; Body=1 Fuz1=1 Fuz2=1 X-Spam-Combo: ;Kees Cook X-Spam-Relay-Country: X-Spam-Timing: total 12590 ms - load_scoreonly_sql: 0.04 (0.0%), signal_user_changed: 2.6 (0.0%), b_tie_ro: 1.77 (0.0%), parse: 0.89 (0.0%), extract_message_metadata: 14 (0.1%), get_uri_detail_list: 1.93 (0.0%), tests_pri_-1000: 7 (0.1%), tests_pri_-950: 1.10 (0.0%), tests_pri_-900: 0.98 (0.0%), tests_pri_-400: 29 (0.2%), check_bayes: 28 (0.2%), b_tokenize: 10 (0.1%), b_tok_get_all: 10 (0.1%), b_comp_prob: 2.7 (0.0%), b_tok_touch_all: 3.7 (0.0%), b_finish: 0.59 (0.0%), tests_pri_0: 217 (1.7%), check_dkim_signature: 0.49 (0.0%), check_dkim_adsp: 2.8 (0.0%), tests_pri_500: 12315 (97.8%), poll_dns_idle: 12306 (97.7%), rewrite_mail: 0.00 (0.0%) Subject: Re: [PATCH v2 1/8] exec: Correct comments about "point of no return" 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 Kees Cook writes: > On Mon, Jul 10, 2017 at 10:07 AM, Eric W. Biederman > wrote: >> Kees Cook writes: >> >>> On Mon, Jul 10, 2017 at 1:46 AM, Eric W. Biederman >>> wrote: >>>> >>>> But you miss it. >>>> >>>> The "point of no return" is the call to de_thread. Or aguably anything in >>>> flush_old_exec. Once anything in the current task is modified you can't >>>> return an error. >>>> >>>> It very much does not have anything to do with brpm. It has >>>> everything to do with current. >>> >>> Yes, but the thing that actually enforces this is the test of bprm->mm >>> and the SIGSEGV in search_binary_handlers(). >> >> So what. Calling that the point of no return is wrong. >> >> The point of no return is when we kill change anyting in signal_struct >> or task_struct. AKA killing the first thread in de_thread. > > Well, okay, I think this is a semantic difference. Prior to bprm->mm > being NULL, there is still an error return path (yes?), though there > may have been side-effects (like de_thread(), as you say). But after > going NULL, the exec either succeeds or SEGVs. It is literally the > point of no "return". Nope. The only exits out of de_thread without de_thread completing successfully are when we know the processes is already exiting (signal_group_exit) or when a fatal signal is pending (__fatal_signal_pending). With a process exit already pending there is no need to send a separate signal. Quite seriously after exec starts having side effects on the process we may not return to userspace. >> It is more than just the SIGSEGV in search_binary_handlers that enforces >> this. de_thread only returns (with a failure code) after having killed >> some threads if those threads are dead. > > This would still result in the exec-ing thread returning with that > error, yes? Nope. The process dies before it gets to see the failure code. >> Similarly exec_mmap only returns with failure if we know that a core >> dump is pending, and as such the process will be killed before returning >> to userspace. > > Yeah, I had looked at this code and mostly decided it wasn't possible > for exec_mmap() to actually get its return value back to userspace. > >> I am a little worried that we may fail to dump some threads if a core >> dump races with exec, but that is a quality of implementation issue, and >> the window is very small so I don't know that it matters. >> >> The point of no return very much comes a while before clearing brpm->mm. > > I'm happy to re-write the comments, but I was just trying to document > the SEGV case, which is what that comment was originally trying to do > (and got lost in the various shuffles). My objection is you are misdocumenting what is going on. If we are going to correct the comment let's correct the comment. The start of flush_old_exec is the point of no return. Any errors after that point the process will never see. Eric