Greg Ungerer <gerg@xxxxxxxxxxxxxx> writes: > On 1/5/20 5:07 am, Eric W. Biederman wrote: >> Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> writes: >> >>> On Thu, Apr 30, 2020 at 7:10 AM Greg Ungerer <gerg@xxxxxxxxxxxxxx> wrote: >> >>>>> Most of that file goes back to pre-git days. And most of the commits >>>>> since are not so much about binfmt_flat, as they are about cleanups or >>>>> changes elsewhere where binfmt_flat was just a victim. >>>> >>>> I'll have a look at this. >>> >>> Thanks. >>> >>>> Quick hack test shows moving setup_new_exec(bprm) to be just before >>>> install_exec_creds(bprm) works fine for the static binaries case. >>>> Doing the flush_old_exec(bprm) there too crashed out - I'll need to >>>> dig into that to see why. >>> >>> Just moving setup_new_exec() would at least allow us to then join the >>> two together, and just say "setup_new_exec() does the credential >>> installation too". >> >> But it is only half a help if we allow failure points between >> flush_old_exec and install_exec_creds. >> >> Greg do things work acceptably if install_exec_creds is moved to right >> after setup_new_exec? (patch below) > > Yes, confirmed. Worked fine with that patch applied. Good. Thank you. That is what we need for other cleanups. All three of those together. >> This is what I was thinking about applying. >> >> diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c >> index 831a2b25ba79..1a1d1fcb893f 100644 >> --- a/fs/binfmt_flat.c >> +++ b/fs/binfmt_flat.c >> @@ -541,6 +541,7 @@ static int load_flat_file(struct linux_binprm *bprm, >> /* OK, This is the point of no return */ >> set_personality(PER_LINUX_32BIT); >> setup_new_exec(bprm); >> + install_exec_creds(bprm); >> } >> /* >> @@ -963,8 +964,6 @@ static int load_flat_binary(struct linux_binprm *bprm) >> } >> } >> - install_exec_creds(bprm); >> - >> set_binfmt(&flat_format); >> #ifdef CONFIG_MMU Eric