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:   Thu, 7 May 2020 22:50:53 -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 6/7] exec: Move most of setup_new_exec into flush_old_exec
Message-ID: <202005072249.81159F29@keescook>
References: <87h7wujhmz.fsf@x220.int.ebiederm.org>
 <87ftcei2si.fsf@x220.int.ebiederm.org>
 <202005051354.C7E2278688@keescook>
 <87blmz8lda.fsf@x220.int.ebiederm.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <87blmz8lda.fsf@x220.int.ebiederm.org>
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Thu, May 07, 2020 at 04:51:13PM -0500, Eric W. Biederman wrote:
> I intend to the following text to the changelog.  At this point I
> believe I have read through everything and nothing raises any concerns
> for me:
> 
> --- text begin ---
> 
> To see why it is safe to move this code please note that effectively
> this change moves the personality setting in the binfmt and the following
> three lines of code after everything except unlocking the mutexes:
>         arch_pick_mmap_layout
>         arch_setup_new_exec
>         mm->task_size = TASK_SIZE
> 
> The function arch_pick_mmap_layout at most sets:
>         mm->get_unmapped_area
>         mm->mmap_base
>         mm->mmap_legacy_base
>         mm->mmap_compat_base
>         mm->mmap_compat_legacy_base
> which nothing in flush_old_exec or setup_new_exec depends on.
> 
> The function arch_setup_new_exec only sets architecture specific
> state and the rest of the functions only deal in state that applies
> to all architectures.
> 
> The last line just sets mm->task_size and again nothing in flush_old_exec
> or setup_new_exec depend on task_size.
> 
> --- text end ---
> [...]
> > So, with a bit larger changelog discussing what's moving "earlier",
> > I think this looks good:
> 
> Please see above.

Awesome! Thanks for checking my checking of your checking. ;)

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

-- 
Kees Cook