Linux-Fsdevel Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Greg Ungerer <firstname.lastname@example.org>
To: Linus Torvalds <email@example.com>
Cc: Russell King - ARM Linux admin <firstname.lastname@example.org>,
Jann Horn <email@example.com>, Nicolas Pitre <firstname.lastname@example.org>,
Andrew Morton <email@example.com>,
Christoph Hellwig <firstname.lastname@example.org>,
Linux Kernel Mailing List <email@example.com>,
Alexander Viro <firstname.lastname@example.org>,
"Eric W . Biederman" <email@example.com>,
Oleg Nesterov <firstname.lastname@example.org>,
Linux ARM <email@example.com>,
Mark Salter <firstname.lastname@example.org>,
Aurelien Jacquiot <email@example.com>,
Yoshinori Sato <firstname.lastname@example.org>,
Rich Felker <email@example.com>,
Linux-sh list <firstname.lastname@example.org>
Subject: Re: [PATCH v2 0/5] Fix ELF / FDPIC ELF core dumping, and use mmap_sem properly in there
Date: Fri, 1 May 2020 17:14:43 +1000 [thread overview]
Message-ID: <email@example.com> (raw)
On 1/5/20 2:54 am, Linus Torvalds wrote:
> On Thu, Apr 30, 2020 at 7:10 AM Greg Ungerer <firstname.lastname@example.org> wrote:
>>> in load_flat_file() - which is also used to loading _libraries_. Where
>>> it makes no sense at all.
>> I haven't looked at the shared lib support in there for a long time,
>> but I thought that "id" is only 0 for the actual final program.
>> Libraries have a slot or id number associated with them.
> Yes, that was my assumption, but looking at the code, it really isn't
> obvious that that is the case at all.
> 'id' gets calculated from fields that very much look like they could
> be zero (eg by taking the top bits from another random field).
>>> 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.
>> 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".
> So to some degree, that's the important one.
> But that flush_old_exec() does look odd in load_flat_file(). It's not
> like anything but executing a binary should flush the old exec.
> Certainly not loading a library, however odd that flat library code
> My _guess_ is that the reason for this is that "load_flat_file()" also
> does a lot of verification of the file and does that whole "return
> -ENOEXEC if the file format isn't right". So we don't want to flush
> the old exec before that is done, but we obviously also don't want to
> flush the old exec after we've actually loaded the new one into
Yeah, that is what it looks like. Looking at the history, the introduction
of setup_new_exec() [in commit 221af7f87b974] was probably just
added where the the existing flush_old_exec() was.
> So the location of flush_old_exec() makes that kind of sense, but it
> would have made it better if that flat file support had a clear
> separation of "check the file" from "load the file".
> Oh well. As mentioned, the whole "at least put setup_new_exec() and
> install_exec_creds() together" is the bigger thing.
> But if it's true that nobody really uses the odd flat library support
> any more and there are no testers, maybe we should consider ripping it
I am for that. If nobody pipes up and complains I'll look at taking it out.
next prev parent reply other threads:[~2020-05-01 7:14 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-04-29 21:49 Jann Horn
2020-04-29 21:49 ` [PATCH v2 1/5] binfmt_elf_fdpic: Stop using dump_emit() on user pointers on !MMU Jann Horn
2020-05-05 10:48 ` Christoph Hellwig
2020-05-05 11:42 ` Jann Horn
2020-05-05 12:15 ` Christoph Hellwig
2020-08-11 3:05 ` Jann Horn
2020-04-29 21:49 ` [PATCH v2 2/5] coredump: Let dump_emit() bail out on short writes Jann Horn
2020-04-29 21:49 ` [PATCH v2 3/5] coredump: Refactor page range dumping into common helper Jann Horn
2020-05-05 10:50 ` Christoph Hellwig
2020-05-05 11:44 ` Jann Horn
2020-04-29 21:49 ` [PATCH v2 4/5] binfmt_elf, binfmt_elf_fdpic: Use a VMA list snapshot Jann Horn
2020-05-05 11:03 ` Christoph Hellwig
2020-05-05 12:11 ` Jann Horn
2020-04-29 21:49 ` [PATCH v2 5/5] mm/gup: Take mmap_sem in get_dump_page() Jann Horn
2020-04-29 21:56 ` [PATCH v2 0/5] Fix ELF / FDPIC ELF core dumping, and use mmap_sem properly in there Russell King - ARM Linux admin
2020-04-29 23:03 ` Linus Torvalds
2020-04-30 1:27 ` Nicolas Pitre
2020-04-30 14:10 ` Greg Ungerer
2020-04-30 14:51 ` Rich Felker
2020-04-30 21:13 ` Rob Landley
2020-05-01 6:00 ` Greg Ungerer
2020-05-01 19:09 ` Rob Landley
2020-04-30 16:54 ` Linus Torvalds
2020-04-30 19:07 ` Eric W. Biederman
2020-05-01 5:44 ` Greg Ungerer
2020-05-01 11:13 ` Eric W. Biederman
2020-05-01 7:14 ` Greg Ungerer [this message]
2020-04-30 1:59 ` Nicolas Pitre
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--subject='Re: [PATCH v2 0/5] Fix ELF / FDPIC ELF core dumping, and use mmap_sem properly in there' \
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).