Re: exec: Promised cleanups after introducing exec_update_mutex

[Greg Ungerer <gerg@linux-m68k.org>]

Hi Eric,

On 6/5/20 5:39 am, Eric W. Biederman wrote:
> In the patchset that introduced exec_update_mutex there were a few last
> minute discoveries and fixes that left the code in a state that can
> be very easily be improved.
> 
> During the merge window we discussed the first three of these patches
> and I promised I would resend them.
> 
> What the first patch does is it makes the the calls in the binfmts:
> 	flush_old_exec();
>          /* set the personality */
>          setup_new_exec();
>          install_exec_creds();
> 
> With no sleeps or anything in between.
> 
> At the conclusion of this set of changes the the calls in the binfmts
> are:
> 	begin_new_exec();
>          /* set the personality */
>          setup_new_exec();
> 
> The intent is to make the code easier to follow and easier to change.
> 
> Eric W. Biederman (7):
>        binfmt: Move install_exec_creds after setup_new_exec to match binfmt_elf
>        exec: Make unlocking exec_update_mutex explict
>        exec: Rename the flag called_exec_mmap point_of_no_return
>        exec: Merge install_exec_creds into setup_new_exec
>        exec: In setup_new_exec cache current in the local variable me
>        exec: Move most of setup_new_exec into flush_old_exec
>        exec: Rename flush_old_exec begin_new_exec
> 
>   Documentation/trace/ftrace.rst |   2 +-
>   arch/x86/ia32/ia32_aout.c      |   4 +-
>   fs/binfmt_aout.c               |   3 +-
>   fs/binfmt_elf.c                |   3 +-
>   fs/binfmt_elf_fdpic.c          |   3 +-
>   fs/binfmt_flat.c               |   4 +-
>   fs/exec.c                      | 162 ++++++++++++++++++++---------------------
>   include/linux/binfmts.h        |  10 +--
>   kernel/events/core.c           |   2 +-
>   9 files changed, 92 insertions(+), 101 deletions(-)

I tested the the whole series on non-MMU m68k and non-MMU arm
(exercising binfmt_flat) and it all tested out with no problems,
so for the binfmt_flat changes:

Tested-by: Greg Ungerer <gerg@linux-m68k.org>

I reviewed the whole series too, and looks good to me:

Reviewed-by: Greg Ungerer <gerg@linux-m68k.org>

Regards
Greg


> ---
> 
> These changes are against v5.7-rc3.
> 
> My intention once everything passes code reveiw is to place these
> changes in a topic branch in my tree and then into linux-next, and
> eventually to send Linus a pull when the next merge window opens.
> Unless someone has a better idea.
> 
> I am a little concerned that I might conflict with the ongoing coredump
> cleanups.
> 
> I have several follow up sets of changes with additional cleanups as
> well but I am trying to keep everything small enough that the code can
> be reviewed.
> 
> After enough cleanups I hope to reopen the conversation of dealing with
> the livelock situation with cred_guard_mutex.  As I think figuring out
> what to do becomes much easier once several of my planned
> cleanups/improvements have been made.
> 
> But ultimately I just want to get exec to the point where when
> we have disucssions on how to make exec better the code is in good
> enough shape we can actually address the issues we see.
> 
> Eric
>