linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Rob Landley <rob@landley.net>
To: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Oleg Nesterov <oleg@redhat.com>, Jann Horn <jannh@google.com>,
	Kees Cook <keescook@chromium.org>,
	Greg Ungerer <gerg@linux-m68k.org>,
	Bernd Edlinger <bernd.edlinger@hotmail.de>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>,
	Al Viro <viro@zeniv.linux.org.uk>,
	Alexey Dobriyan <adobriyan@gmail.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Casey Schaufler <casey@schaufler-ca.com>,
	LSM List <linux-security-module@vger.kernel.org>,
	James Morris <jmorris@namei.org>,
	"Serge E. Hallyn" <serge@hallyn.com>,
	Andy Lutomirski <luto@amacapital.net>,
	dalias@libc.org
Subject: Re: [PATCH 3/5] exec: Remove recursion from search_binary_handler
Date: Thu, 14 May 2020 13:46:30 -0500	[thread overview]
Message-ID: <705e28f9-b8ec-4986-bf00-e2892f0272bc@landley.net> (raw)
In-Reply-To: <87ftc3lcmw.fsf@x220.int.ebiederm.org>

On 5/13/20 4:59 PM, Eric W. Biederman wrote:
> Careful with your terminology.  ELF sections are for .o's For
> executables ELF have segments.  And reading through the code it is the
> program segments that are independently relocatable.

Sorry, I have trouble keeping this stuff straight when it's not in front of me.
(I have a paperback copy of the old "linkers and loaders" book and it was the
driest thing I have _ever_ slogged through. Back before the Linux Foundation ate
the FSG I was pushing https://refspecs.linuxbase.org/ to include missing ABI
supplement, I have copies of ones it doesn't collected from now long-dead sites...)

But more recently I've just made puppy eyes at Rich Felker to have him fix this
stuff for me, because I do _not_ retain the terminology here. REL vs RELA vs
PLT, can you have a PLT without a GOT...?

> There is a flag but it is defined per architecture and I don't think one
> of the architectures define it.

They all check for one, but I don't remember there being a #define.

I have a todo item to check more architectures' fdpic binaries, this was from
sh2eb (ala j-core):

  https://github.com/landley/toybox/commit/d61aeaf9e#diff-4442ddbb8949R65

There was the out of tree arm fdpic toolchain from the french guys for cortex-m,
and the original frv paper, and in theory blackfin but nothing they touched ever
got merged upstream anywhere:

In _theory_ you could do fdpic for x86, but as with u-boot for x86 nobody ever
bothers because it's got an x86-only solution. (And then the x86 version of
stuff gets pushed to other platforms because all our device tree files were
GPLed so of course acpi for arm became a thing. Sigh...)

> I looked at ARM and apparently with an MMU ARM turns fdpic binaries into
> PIE executables.  I am not certain why.

Falling back to a more widely tested codepath, I expect. Also maybe it saves 3
registers if all 4 are using the same base register? Map them linearly and it
becomes "single base + offset"? Which of course looses the extra ASLR benefits
the security people wanted, but "undoing what the security people want in the
name of an unmeasurable microbenchmark optimization" is a proud tradition.

Just because the 4 segments are compiled as independently relocatable doesn't
mean they HAVE to be. (You'd think the code would be using different register
numbers to index stuff so you'd STILL be using 4 registers, but I haven't looked
at what arm's doing...)

> The registers passed to the entry point are also different for both
> cases.

From the same machine code chunks? I boggle at what the ld.so fixup is doing then...

> I think it would have been nice if the fdpic support had used a
> different ELF type, instead of a different depending on using a
> different architecture.

This is what you get when a blackfin developer talks to the gnu/binutils developers:

  https://sourceware.org/legacy-ml/binutils/2008-04/msg00350.html

> All that aside the core dumping code looks to be essentially the same
> between binfmt_elf.c and binfmt_elf_fdpic.c.  Do you think people would
> be interested in refactoring binfmt_elf.c and binfmt_elf_fdpic.c so that
> they could share the same core dumping code?

I think merging the two of them together entirely would be a good idea, and
anything that can collapse together I'm happy to regression test on sh2.

I also note that qemu-sh4eb can run these binaries, maybe I can whip up a
qemu-system-sh4eb that runs a nommu fdpic userspace...

[hours later]

Ok, here's me asking Rich Felker a question:

>>> So fdpic binaries run under qemu-sh2eb and there's a qemu-system-sh2eb that
>>> SHOULD also be able to run them under the r2d board emulation, and the kernel
>>> builds fine under the sh2eb compiler but I can't enable fdpic support without
>>> CONFIG_NOMMU, and if I yank that dependency from Kconfig (which only sh2 has,
>>> arm and such do fdpic with or without mmu) the build breaks with:
>>>
>>> /home/landley/toybox/clean/ccc/sh2eb-linux-muslfdpic-cross/bin/sh2eb-linux-muslfdpic-ld:
>>> fs/binfmt_elf_fdpic.o: in function `load_elf_fdpic_binary':
>>> binfmt_elf_fdpic.c:(.text+0x1734): undefined reference to
>>> `elf_fdpic_arch_lay_out_mm'
>>>
>>> The problem is if I switch off CONFIG_MMU in the kernel, buckets of stuff in the
>>> r2d board kernel config changes and suddenly I don't get serial output from the
>>> qemu-system-sh2eb -M r2d boot anymore. Before it was running the kernel but just
>>> failing to run init...

And his response:

>> I don't think qemu-system-sh4eb can boot a nommu kernel. But you don't
>> need to in order to do userspace-only testing. Just build a normal
>> sh4eb kernel. It doesn't need CONFIG_BINFMT_ELF_FDPIC. The normal ELF
>> loader can load FDPIC just fine, because a valid FDPIC ELF file is a
>> valid ELF file, just with more constraints (in same sense a square is
>> a rectangle). The normal ELF loader won't independently float the text
>> and data segments, but that's okay because your emulated system has an
>> MMU and can just map them adjacently like they show up in the ELF file
>> with their untransformed addresses.
>> 
>> Now that I think about it, it's possible that the ARM folks broke this
>> when adding support for enabling CONFIG_BINFMT_ELF_FDPIC with MMU. If
>> so, and you find you really do need the FDPIC loader now because they
>> made the normal ELF loader refuse to do it, I think it will suffice to
>> copy the ARM version of elf_fdpic_arch_lay_out_mm from
>> arch/arm/kernel/elf.c to somewhere it will be compiled on SH.

I.E. testing the kernel fdpic loader under qemu is NOT EASY (because the fdpic
loader refuses to build in a with-mmu context, and the relevant board emulations
refuse to build without), but it can fall back to the conventional ELF loader
which collates the segments and treats fdpic as PIE? (Which... is how qemu-sh2eb
application emulation is loading them...?)

Which was news to me...

> Eric

Rob

  reply	other threads:[~2020-05-14 18:39 UTC|newest]

Thread overview: 149+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-05 19:39 exec: Promised cleanups after introducing exec_update_mutex Eric W. Biederman
2020-05-05 19:41 ` [PATCH 1/7] binfmt: Move install_exec_creds after setup_new_exec to match binfmt_elf Eric W. Biederman
2020-05-05 20:45   ` Kees Cook
2020-05-06 12:42   ` Greg Ungerer
2020-05-06 12:56     ` Eric W. Biederman
2020-05-05 19:41 ` [PATCH 2/7] exec: Make unlocking exec_update_mutex explict Eric W. Biederman
2020-05-05 20:46   ` Kees Cook
2020-05-05 19:42 ` [PATCH 3/7] exec: Rename the flag called_exec_mmap point_of_no_return Eric W. Biederman
2020-05-05 20:49   ` Kees Cook
2020-05-05 19:43 ` [PATCH 4/7] exec: Merge install_exec_creds into setup_new_exec Eric W. Biederman
2020-05-05 20:50   ` Kees Cook
2020-05-05 19:44 ` [PATCH 5/7] exec: In setup_new_exec cache current in the local variable me Eric W. Biederman
2020-05-05 20:51   ` Kees Cook
2020-05-05 19:45 ` [PATCH 6/7] exec: Move most of setup_new_exec into flush_old_exec Eric W. Biederman
2020-05-05 21:29   ` Kees Cook
2020-05-06 14:57     ` Eric W. Biederman
2020-05-06 15:30       ` Kees Cook
2020-05-07 19:51         ` Eric W. Biederman
2020-05-07 21:51     ` Eric W. Biederman
2020-05-08  5:50       ` Kees Cook
2020-05-05 19:46 ` [PATCH 7/7] exec: Rename flush_old_exec begin_new_exec Eric W. Biederman
2020-05-05 21:30   ` Kees Cook
2020-05-06 12:41 ` exec: Promised cleanups after introducing exec_update_mutex Greg Ungerer
2020-05-08 18:43 ` [PATCH 0/6] exec: Trivial cleanups for exec Eric W. Biederman
2020-05-08 18:44   ` [PATCH 1/6] exec: Move the comment from above de_thread to above unshare_sighand Eric W. Biederman
2020-05-09  5:02     ` Kees Cook
2020-05-08 18:44   ` [PATCH 2/6] exec: Fix spelling of search_binary_handler in a comment Eric W. Biederman
2020-05-09  5:03     ` Kees Cook
2020-05-08 18:45   ` [PATCH 3/6] exec: Stop open coding mutex_lock_killable of cred_guard_mutex Eric W. Biederman
2020-05-09  5:08     ` Kees Cook
2020-05-09 19:18     ` Linus Torvalds
2020-05-09 19:57       ` Eric W. Biederman
2020-05-10 20:33       ` Kees Cook
2020-05-08 18:45   ` [PATCH 4/6] exec: Run sync_mm_rss before taking exec_update_mutex Eric W. Biederman
2020-05-09  5:15     ` Kees Cook
2020-05-09 14:17       ` Eric W. Biederman
2020-05-08 18:47   ` [PATCH 5/6] exec: Move handling of the point of no return to the top level Eric W. Biederman
2020-05-09  5:31     ` Kees Cook
2020-05-09 13:39       ` Eric W. Biederman
2020-05-08 18:48   ` [PATCH 6/6] exec: Set the point of no return sooner Eric W. Biederman
2020-05-09  5:33     ` Kees Cook
2020-05-09 19:40   ` [PATCH 0/5] exec: Control flow simplifications Eric W. Biederman
2020-05-09 19:40     ` [PATCH 1/5] exec: Call cap_bprm_set_creds directly from prepare_binprm Eric W. Biederman
2020-05-09 20:04       ` Linus Torvalds
2020-05-09 19:41     ` [PATCH 2/5] exec: Directly call security_bprm_set_creds from __do_execve_file Eric W. Biederman
2020-05-09 20:07       ` Linus Torvalds
2020-05-09 20:12         ` Eric W. Biederman
2020-05-09 20:19           ` Linus Torvalds
2020-05-11  3:15       ` Kees Cook
2020-05-11 16:52         ` Eric W. Biederman
2020-05-11 21:18           ` Kees Cook
2020-05-09 19:41     ` [PATCH 3/5] exec: Remove recursion from search_binary_handler Eric W. Biederman
2020-05-09 20:16       ` Linus Torvalds
2020-05-10  4:22       ` Tetsuo Handa
2020-05-10 19:38         ` Linus Torvalds
2020-05-11 14:33           ` Eric W. Biederman
2020-05-11 19:10             ` Rob Landley
2020-05-13 21:59               ` Eric W. Biederman
2020-05-14 18:46                 ` Rob Landley [this message]
2020-05-11 21:55             ` Kees Cook
2020-05-12 18:42               ` Eric W. Biederman
2020-05-12 19:25                 ` Kees Cook
2020-05-12 20:31                   ` Eric W. Biederman
2020-05-12 23:08                     ` Kees Cook
2020-05-12 23:47                       ` Kees Cook
2020-05-12 23:51                         ` Kees Cook
2020-05-14 14:56                           ` Eric W. Biederman
2020-05-14 16:56                             ` Casey Schaufler
2020-05-14 17:02                               ` Eric W. Biederman
2020-05-13  0:20                 ` Linus Torvalds
2020-05-13  2:39                   ` Rob Landley
2020-05-13 19:51                     ` Linus Torvalds
2020-05-14 16:49                   ` Eric W. Biederman
2020-05-09 19:42     ` [PATCH 4/5] exec: Allow load_misc_binary to call prepare_binfmt unconditionally Eric W. Biederman
2020-05-11 22:09       ` Kees Cook
2020-05-09 19:42     ` [PATCH 5/5] exec: Move the call of prepare_binprm into search_binary_handler Eric W. Biederman
2020-05-11 22:24       ` Kees Cook
2020-05-19  0:29     ` [PATCH v2 0/8] exec: Control flow simplifications Eric W. Biederman
2020-05-19  0:29       ` [PATCH v2 1/8] exec: Teach prepare_exec_creds how exec treats uids & gids Eric W. Biederman
2020-05-19 18:03         ` Kees Cook
2020-05-19 18:28           ` Linus Torvalds
2020-05-19 18:57             ` Eric W. Biederman
2020-05-19  0:30       ` [PATCH v2 2/8] exec: Factor security_bprm_creds_for_exec out of security_bprm_set_creds Eric W. Biederman
2020-05-19 15:34         ` Casey Schaufler
2020-05-19 18:10         ` Kees Cook
2020-05-19 21:28           ` James Morris
2020-05-19  0:31       ` [PATCH v2 3/8] exec: Convert security_bprm_set_creds into security_bprm_repopulate_creds Eric W. Biederman
2020-05-19 18:21         ` Kees Cook
2020-05-19 19:03           ` Eric W. Biederman
2020-05-19 19:14             ` Kees Cook
2020-05-20 20:22               ` Eric W. Biederman
2020-05-20 20:53                 ` Kees Cook
2020-05-19 21:52         ` James Morris
2020-05-20 12:40           ` Eric W. Biederman
2020-05-19  0:31       ` [PATCH v2 4/8] exec: Allow load_misc_binary to call prepare_binfmt unconditionally Eric W. Biederman
2020-05-19 18:27         ` Kees Cook
2020-05-19 19:08           ` Eric W. Biederman
2020-05-19 19:17             ` Kees Cook
2020-05-19  0:32       ` [PATCH v2 5/8] exec: Move the call of prepare_binprm into search_binary_handler Eric W. Biederman
2020-05-19 18:27         ` Kees Cook
2020-05-19 21:30         ` James Morris
2020-05-19  0:33       ` [PATCH v2 6/8] exec/binfmt_script: Don't modify bprm->buf and then return -ENOEXEC Eric W. Biederman
2020-05-19 19:08         ` Kees Cook
2020-05-19 19:19           ` Eric W. Biederman
2020-05-19  0:33       ` [PATCH v2 7/8] exec: Generic execfd support Eric W. Biederman
2020-05-19 19:46         ` Kees Cook
2020-05-19 19:54           ` Linus Torvalds
2020-05-19 20:20             ` Eric W. Biederman
2020-05-19 21:59         ` Rob Landley
2020-05-20 16:05           ` Eric W. Biederman
2020-05-21 22:50             ` Rob Landley
2020-05-22  3:28               ` Eric W. Biederman
2020-05-22  4:51                 ` Rob Landley
2020-05-22 13:35                   ` Eric W. Biederman
2020-05-19  0:34       ` [PATCH v2 8/8] exec: Remove recursion from search_binary_handler Eric W. Biederman
2020-05-19 20:37         ` Kees Cook
2020-05-19  1:25       ` [PATCH v2 0/8] exec: Control flow simplifications Linus Torvalds
2020-05-19 21:55       ` Kees Cook
2020-05-20 13:02         ` Eric W. Biederman
2020-05-20 22:12       ` Eric W. Biederman
2020-05-20 23:43         ` Kees Cook
2020-05-21 11:53           ` Eric W. Biederman
2020-05-28 15:38       ` [PATCH 0/11] exec: cred calculation simplifications Eric W. Biederman
2020-05-28 15:41         ` [PATCH 01/11] exec: Reduce bprm->per_clear to a single bit Eric W. Biederman
2020-05-28 19:04           ` Linus Torvalds
2020-05-28 19:17             ` Eric W. Biederman
2020-05-28 15:42         ` [PATCH 02/11] exec: Introduce active_per_clear the per file version of per_clear Eric W. Biederman
2020-05-28 19:05           ` Linus Torvalds
2020-05-28 15:42         ` [PATCH 03/11] exec: Compute file based creds only once Eric W. Biederman
2020-05-28 15:43         ` [PATCH 04/11] exec: Move uid/gid handling from creds_from_file into bprm_fill_uid Eric W. Biederman
2020-05-28 15:44         ` Eric W. Biederman
2020-05-28 15:44         ` [PATCH 05/11] exec: In bprm_fill_uid use CAP_SETGID to see if a gid change is safe Eric W. Biederman
2020-05-28 15:48         ` [PATCH 06/11] exec: Don't set secureexec when the uid or gid changes are abandoned Eric W. Biederman
2020-05-28 15:48         ` [PATCH 07/11] exec: Set saved, fs, and effective ids together in bprm_fill_uid Eric W. Biederman
2020-05-28 15:49         ` [PATCH 08/11] exec: In bprm_fill_uid remove unnecessary no new privs check Eric W. Biederman
2020-05-28 15:49         ` [PATCH 09/11] exec: In bprm_fill_uid only set per_clear when honoring suid or sgid Eric W. Biederman
2020-05-28 19:08           ` Linus Torvalds
2020-05-28 19:21             ` Eric W. Biederman
2020-05-28 15:50         ` [PATCH 10/11] exec: In bprm_fill_uid set secureexec at same time as per_clear Eric W. Biederman
2020-05-28 15:50         ` [PATCH 11/11] exec: Remove the label after_setid from bprm_fill_uid Eric W. Biederman
2020-05-29 16:45         ` [PATCH 0/2] exec: Remove the computation of bprm->cred Eric W. Biederman
2020-05-29 16:46           ` [PATCH 1/2] exec: Add a per bprm->file version of per_clear Eric W. Biederman
2020-05-29 21:06             ` Kees Cook
2020-05-30  3:23               ` Eric W. Biederman
2020-05-30  5:14                 ` Kees Cook
2020-05-29 16:47           ` [PATCH 2/2] exec: Compute file based creds only once Eric W. Biederman
2020-05-29 21:24             ` Kees Cook
2020-05-30  3:28               ` Eric W. Biederman
2020-05-30  5:18                 ` Kees Cook

Reply instructions:

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:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=705e28f9-b8ec-4986-bf00-e2892f0272bc@landley.net \
    --to=rob@landley.net \
    --cc=adobriyan@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=bernd.edlinger@hotmail.de \
    --cc=casey@schaufler-ca.com \
    --cc=dalias@libc.org \
    --cc=ebiederm@xmission.com \
    --cc=gerg@linux-m68k.org \
    --cc=jannh@google.com \
    --cc=jmorris@namei.org \
    --cc=keescook@chromium.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=luto@amacapital.net \
    --cc=oleg@redhat.com \
    --cc=penguin-kernel@i-love.sakura.ne.jp \
    --cc=serge@hallyn.com \
    --cc=torvalds@linux-foundation.org \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).