Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Fri, 8 May 2020 22:31:06 -0700
From:   Kees Cook <keescook@chromium.org>
To:     "Eric W. Biederman" <ebiederm@xmission.com>
Cc:     linux-kernel@vger.kernel.org,
        Linus Torvalds <torvalds@linux-foundation.org>,
        Oleg Nesterov <oleg@redhat.com>, Jann Horn <jannh@google.com>,
        Greg Ungerer <gerg@linux-m68k.org>,
        Rob Landley <rob@landley.net>,
        Bernd Edlinger <bernd.edlinger@hotmail.de>,
        linux-fsdevel@vger.kernel.org, Al Viro <viro@ZenIV.linux.org.uk>,
        Alexey Dobriyan <adobriyan@gmail.com>,
        Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [PATCH 5/6] exec: Move handling of the point of no return to the
 top level
Message-ID: <202005082228.5C0E44CC6@keescook>
References: <87h7wujhmz.fsf@x220.int.ebiederm.org>
 <87sgga6ze4.fsf@x220.int.ebiederm.org>
 <87y2q25knl.fsf_-_@x220.int.ebiederm.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <87y2q25knl.fsf_-_@x220.int.ebiederm.org>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Fri, May 08, 2020 at 01:47:10PM -0500, Eric W. Biederman wrote:
> 
> Move the handing of the point of no return from search_binary_handler
> into __do_execve_file so that it is easier to find, and to keep
> things robust in the face of change.
> 
> Make it clear that an existing fatal signal will take precedence over
> a forced SIGSEGV by not forcing SIGSEGV if a fatal signal is already
> pending.  This does not change the behavior but it saves a reader
> of the code the tedium of reading and understanding force_sig
> and the signal delivery code.
> 
> Update the comment in begin_new_exec about where SIGSEGV is forced.
> 
> Keep point_of_no_return from being a mystery by documenting
> what the code is doing where it forces SIGSEGV if the
> code is past the point of no return.
> 
> Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>

I had to read the code around these changes a bit carefully, but yeah,
this looks like a safe cleanup. It is a behavioral change, though (in
that in unmasks non-SEGV fatal signals), so I do wonder if something
somewhere might notice this, but I'd agree that it's the more robust
behavior.

Reviewed-by: Kees Cook <keescook@chromium.org>

-- 
Kees Cook