| Message ID | 20151008163810.GX8645@brightrain.aerifal.cx (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Rich, On 09/10/15 02:38, Rich Felker wrote: > From: Rich Felker <dalias@libc.org> > > The ELF binary loader in binfmt_elf.c requires an MMU, making it > impossible to use regular ELF binaries on NOMMU archs. However, the > FDPIC ELF loader in binfmt_elf_fdpic.c is fully capable as a loader > for plain ELF, which requires constant displacements between LOAD > segments, since it already supports FDPIC ELF files flagged as needing > constant displacement. > > This patch adjusts the FDPIC ELF loader to accept non-FDPIC ELF files > on NOMMU archs. They are treated identically to FDPIC ELF files with > the constant-displacement flag bit set, except for personality, which > must match the ABI of the program being loaded; the PER_LINUX_FDPIC > personality controls how the kernel interprets function pointers > passed to sigaction. > > Files that do not set a stack size requirement explicitly are given a > default stack size (matching the amount of committed stack the normal > ELF loader for MMU archs would give them) rather than being rejected; > this is necessary because plain ELF files generally do not declare > stack requirements in theit program headers. > > Only ET_DYN (PIE) format ELF files are supported, since loading at a > fixed virtual address is not possible on NOMMU. > > Signed-off-by: Rich Felker <dalias@libc.org> I have no problem with this, so from me: Acked-by: Greg Ungerer <gerg@uclinux.org> > --- > > This patch was developed and tested on J2 (SH2-compatible) but should > be usable immediately on all archs where binfmt_elf_fdpic is > available. Moreover, by providing dummy definitions of the > elf_check_fdpic() and elf_check_const_displacement() macros for archs > which lack an FDPIC ABI, it should be possible to enable building of > binfmt_elf_fdpic on all other NOMMU archs and thereby give them ELF > binary support, but I have not yet tested this. There is a couple of other details that will currently stop this from working on other arches too. . kernel/ptrace.c has some fdpic specific code (wanting PTRACE_GETFDPIC) . arch specific mm_context_t may not have members ‘interp_fdpic_loadmap' or 'exec_fdpic_loadmap' Should be easy to fix those. It would be good to get some testing and verification on other fdpic supported arches (frv or blackfin or microblaze for example). Regards Greg > The motivation for using binfmt_elf_fdpic.c rather than adapting > binfmt_elf.c to NOMMU is that the former already has all the necessary > code to work properly on NOMMU and has already received widespread > real-world use and testing. I hope this is not controversial. > > I'm not really happy with having to unset the FDPIC_FUNCPTRS > personality bit when loading non-FDPIC ELF. This bit should really > reset automatically on execve, since otherwise, executing non-ELF > binaries (e.g. bFLT) from an FDPIC process will leave the personality > in the wrong state and severely break signal handling. But that's a > separate, existing bug and I don't know the right place to fix it. > > The previous version of this patch had a bug which broke loading of > non-ET_DYN FDPIC binaries that slipped through my testing. This > version fixes the regression and has been tested with both FDPIC and > non-FDPIC ELF files. > > > --- fs/binfmt_elf_fdpic.c.orig 2015-09-29 22:13:06.716412478 +0000 > +++ fs/binfmt_elf_fdpic.c 2015-10-07 05:11:33.702236056 +0000 > @@ -103,19 +103,36 @@ > core_initcall(init_elf_fdpic_binfmt); > module_exit(exit_elf_fdpic_binfmt); > > -static int is_elf_fdpic(struct elfhdr *hdr, struct file *file) > +static int is_elf(struct elfhdr *hdr, struct file *file) > { > if (memcmp(hdr->e_ident, ELFMAG, SELFMAG) != 0) > return 0; > if (hdr->e_type != ET_EXEC && hdr->e_type != ET_DYN) > return 0; > - if (!elf_check_arch(hdr) || !elf_check_fdpic(hdr)) > + if (!elf_check_arch(hdr)) > return 0; > if (!file->f_op->mmap) > return 0; > return 1; > } > > +#ifndef elf_check_fdpic > +#define elf_check_fdpic(x) 0 > +#endif > + > +#ifndef elf_check_const_displacement > +#define elf_check_const_displacement(x) 0 > +#endif > + > +static int is_constdisp(struct elfhdr *hdr) > +{ > + if (!elf_check_fdpic(hdr)) > + return 1; > + if (elf_check_const_displacement(hdr)) > + return 1; > + return 0; > +} > + > /*****************************************************************************/ > /* > * read the program headers table into memory > @@ -191,8 +208,18 @@ > > /* check that this is a binary we know how to deal with */ > retval = -ENOEXEC; > - if (!is_elf_fdpic(&exec_params.hdr, bprm->file)) > + if (!is_elf(&exec_params.hdr, bprm->file)) > goto error; > + if (!elf_check_fdpic(&exec_params.hdr)) { > +#ifdef CONFIG_MMU > + /* binfmt_elf handles non-fdpic elf except on nommu */ > + goto error; > +#else > + /* nommu can only load ET_DYN (PIE) ELF */ > + if (exec_params.hdr.e_type != ET_DYN) > + goto error; > +#endif > + } > > /* read the program header table */ > retval = elf_fdpic_fetch_phdrs(&exec_params, bprm->file); > @@ -269,13 +296,13 @@ > > } > > - if (elf_check_const_displacement(&exec_params.hdr)) > + if (is_constdisp(&exec_params.hdr)) > exec_params.flags |= ELF_FDPIC_FLAG_CONSTDISP; > > /* perform insanity checks on the interpreter */ > if (interpreter_name) { > retval = -ELIBBAD; > - if (!is_elf_fdpic(&interp_params.hdr, interpreter)) > + if (!is_elf(&interp_params.hdr, interpreter)) > goto error; > > interp_params.flags = ELF_FDPIC_FLAG_PRESENT; > @@ -306,9 +333,9 @@ > > retval = -ENOEXEC; > if (stack_size == 0) > - goto error; > + stack_size = 131072UL; /* same as exec.c's default commit */ > > - if (elf_check_const_displacement(&interp_params.hdr)) > + if (is_constdisp(&interp_params.hdr)) > interp_params.flags |= ELF_FDPIC_FLAG_CONSTDISP; > > /* flush all traces of the currently running executable */ > @@ -319,7 +346,10 @@ > /* there's now no turning back... the old userspace image is dead, > * defunct, deceased, etc. > */ > - set_personality(PER_LINUX_FDPIC); > + if (elf_check_fdpic(&exec_params.hdr)) > + set_personality(PER_LINUX_FDPIC); > + else > + set_personality(PER_LINUX); > if (elf_read_implies_exec(&exec_params.hdr, executable_stack)) > current->personality |= READ_IMPLIES_EXEC; > > -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Oct 13, 2015 at 10:55:45PM +1000, Greg Ungerer wrote: > Hi Rich, > > On 09/10/15 02:38, Rich Felker wrote: > >From: Rich Felker <dalias@libc.org> > > > >The ELF binary loader in binfmt_elf.c requires an MMU, making it > >impossible to use regular ELF binaries on NOMMU archs. However, the > >FDPIC ELF loader in binfmt_elf_fdpic.c is fully capable as a loader > >for plain ELF, which requires constant displacements between LOAD > >segments, since it already supports FDPIC ELF files flagged as needing > >constant displacement. > > > >This patch adjusts the FDPIC ELF loader to accept non-FDPIC ELF files > >on NOMMU archs. They are treated identically to FDPIC ELF files with > >the constant-displacement flag bit set, except for personality, which > >must match the ABI of the program being loaded; the PER_LINUX_FDPIC > >personality controls how the kernel interprets function pointers > >passed to sigaction. > > > >Files that do not set a stack size requirement explicitly are given a > >default stack size (matching the amount of committed stack the normal > >ELF loader for MMU archs would give them) rather than being rejected; > >this is necessary because plain ELF files generally do not declare > >stack requirements in theit program headers. > > > >Only ET_DYN (PIE) format ELF files are supported, since loading at a > >fixed virtual address is not possible on NOMMU. > > > >Signed-off-by: Rich Felker <dalias@libc.org> > > I have no problem with this, so from me: > > Acked-by: Greg Ungerer <gerg@uclinux.org> Thanks! > >--- > > > >This patch was developed and tested on J2 (SH2-compatible) but should > >be usable immediately on all archs where binfmt_elf_fdpic is > >available. Moreover, by providing dummy definitions of the > >elf_check_fdpic() and elf_check_const_displacement() macros for archs > >which lack an FDPIC ABI, it should be possible to enable building of > >binfmt_elf_fdpic on all other NOMMU archs and thereby give them ELF > >binary support, but I have not yet tested this. > > There is a couple of other details that will currently stop this from > working on other arches too. > > .. kernel/ptrace.c has some fdpic specific code (wanting PTRACE_GETFDPIC) > .. arch specific mm_context_t may not have members > ‘interp_fdpic_loadmap' or 'exec_fdpic_loadmap' > > Should be easy to fix those. I see. For archs that lack an FDPIC ABI, I'm not sure it makes sense to add these things unless/until someone developes an FDPIC ABI. Would it instead make sense to add a new kconfig switch CONFIG_BINFMT_ELF_NOMMU ("NOMMU ELF loader") that's implied-on by CONFIG_BINFMT_ELF_FDPIC but that can also be enabled independently on archs where CONFIG_BINFMT_ELF_FDPIC is not available? Right now these are just ideas. Unless there's a quick and easy decision to be made, I'd like it if we could move forward with the current patch (which only offers the feature on archs where CONFIG_BINFMT_ELF_FDPIC is already available) first and continue to explore options for making this available to other archs separately. > It would be good to get some testing and verification on other > fdpic supported arches (frv or blackfin or microblaze for example). I wasn't aware Microblaze had an FDPIC ABI; are you sure it does? Testing to make sure these aren't broken by the patch shouldn't be hard to do; I'll start looking into getting a setup for it or finding someone who has one. If you want to also test non-FDPIC ELF binaries, I think just using the ELF output of a bFLT toolchain without running elf2flt may work as a test case, but I'm not sure. Alternatively, any FDPIC binary linked with -pie that doesn't use signals can run as a non-FDPIC one just by clearing the FDPIC bit in the header. Rich -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Rich, On 14/10/15 01:49, Rich Felker wrote: > On Tue, Oct 13, 2015 at 10:55:45PM +1000, Greg Ungerer wrote: >> Hi Rich, >> >> On 09/10/15 02:38, Rich Felker wrote: >>> From: Rich Felker <dalias@libc.org> >>> >>> The ELF binary loader in binfmt_elf.c requires an MMU, making it >>> impossible to use regular ELF binaries on NOMMU archs. However, the >>> FDPIC ELF loader in binfmt_elf_fdpic.c is fully capable as a loader >>> for plain ELF, which requires constant displacements between LOAD >>> segments, since it already supports FDPIC ELF files flagged as needing >>> constant displacement. >>> >>> This patch adjusts the FDPIC ELF loader to accept non-FDPIC ELF files >>> on NOMMU archs. They are treated identically to FDPIC ELF files with >>> the constant-displacement flag bit set, except for personality, which >>> must match the ABI of the program being loaded; the PER_LINUX_FDPIC >>> personality controls how the kernel interprets function pointers >>> passed to sigaction. >>> >>> Files that do not set a stack size requirement explicitly are given a >>> default stack size (matching the amount of committed stack the normal >>> ELF loader for MMU archs would give them) rather than being rejected; >>> this is necessary because plain ELF files generally do not declare >>> stack requirements in theit program headers. >>> >>> Only ET_DYN (PIE) format ELF files are supported, since loading at a >>> fixed virtual address is not possible on NOMMU. >>> >>> Signed-off-by: Rich Felker <dalias@libc.org> >> >> I have no problem with this, so from me: >> >> Acked-by: Greg Ungerer <gerg@uclinux.org> > > Thanks! > >>> --- >>> >>> This patch was developed and tested on J2 (SH2-compatible) but should >>> be usable immediately on all archs where binfmt_elf_fdpic is >>> available. Moreover, by providing dummy definitions of the >>> elf_check_fdpic() and elf_check_const_displacement() macros for archs >>> which lack an FDPIC ABI, it should be possible to enable building of >>> binfmt_elf_fdpic on all other NOMMU archs and thereby give them ELF >>> binary support, but I have not yet tested this. >> >> There is a couple of other details that will currently stop this from >> working on other arches too. >> >> .. kernel/ptrace.c has some fdpic specific code (wanting PTRACE_GETFDPIC) >> .. arch specific mm_context_t may not have members >> ‘interp_fdpic_loadmap' or 'exec_fdpic_loadmap' >> >> Should be easy to fix those. > > I see. For archs that lack an FDPIC ABI, I'm not sure it makes sense > to add these things unless/until someone developes an FDPIC ABI. Would Ok. I was looking at it from the point of view of supoporting ELF on m68k/coldfire, that doesn't currently support FDPIC. So bypassing FDPIC support completely. > it instead make sense to add a new kconfig switch > CONFIG_BINFMT_ELF_NOMMU ("NOMMU ELF loader") that's implied-on by > CONFIG_BINFMT_ELF_FDPIC but that can also be enabled independently on > archs where CONFIG_BINFMT_ELF_FDPIC is not available? Yes that may be the only answer here. > Right now these are just ideas. Unless there's a quick and easy > decision to be made, I'd like it if we could move forward with the > current patch (which only offers the feature on archs where > CONFIG_BINFMT_ELF_FDPIC is already available) first and continue to > explore options for making this available to other archs separately. Oh, yes. No problem with that. Looks like Andrew has picked it up. So all good there. >> It would be good to get some testing and verification on other >> fdpic supported arches (frv or blackfin or microblaze for example). > > I wasn't aware Microblaze had an FDPIC ABI; are you sure it does? Sorry, my mistake. No microblaze FDPIC as far as I know. Regards Greg > Testing to make sure these aren't broken by the patch shouldn't be > hard to do; I'll start looking into getting a setup for it or finding > someone who has one. If you want to also test non-FDPIC ELF binaries, > I think just using the ELF output of a bFLT toolchain without running > elf2flt may work as a test case, but I'm not sure. Alternatively, any > FDPIC binary linked with -pie that doesn't use signals can run as a > non-FDPIC one just by clearing the FDPIC bit in the header. > > Rich > -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 10/14/15 22:09, Greg Ungerer wrote: > Hi Rich, > > On 14/10/15 01:49, Rich Felker wrote: >> On Tue, Oct 13, 2015 at 10:55:45PM +1000, Greg Ungerer wrote: >>> Hi Rich, >>> >>> On 09/10/15 02:38, Rich Felker wrote: >>> It would be good to get some testing and verification on other >>> fdpic supported arches (frv or blackfin or microblaze for example). >> I wasn't aware Microblaze had an FDPIC ABI; are you sure it does? What about ARM Cortex-M? https://sfo15.pathable.com/mobile/meetings/303078 -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
--- fs/binfmt_elf_fdpic.c.orig 2015-09-29 22:13:06.716412478 +0000 +++ fs/binfmt_elf_fdpic.c 2015-10-07 05:11:33.702236056 +0000 @@ -103,19 +103,36 @@ core_initcall(init_elf_fdpic_binfmt); module_exit(exit_elf_fdpic_binfmt); -static int is_elf_fdpic(struct elfhdr *hdr, struct file *file) +static int is_elf(struct elfhdr *hdr, struct file *file) { if (memcmp(hdr->e_ident, ELFMAG, SELFMAG) != 0) return 0; if (hdr->e_type != ET_EXEC && hdr->e_type != ET_DYN) return 0; - if (!elf_check_arch(hdr) || !elf_check_fdpic(hdr)) + if (!elf_check_arch(hdr)) return 0; if (!file->f_op->mmap) return 0; return 1; } +#ifndef elf_check_fdpic +#define elf_check_fdpic(x) 0 +#endif + +#ifndef elf_check_const_displacement +#define elf_check_const_displacement(x) 0 +#endif + +static int is_constdisp(struct elfhdr *hdr) +{ + if (!elf_check_fdpic(hdr)) + return 1; + if (elf_check_const_displacement(hdr)) + return 1; + return 0; +} + /*****************************************************************************/ /* * read the program headers table into memory @@ -191,8 +208,18 @@ /* check that this is a binary we know how to deal with */ retval = -ENOEXEC; - if (!is_elf_fdpic(&exec_params.hdr, bprm->file)) + if (!is_elf(&exec_params.hdr, bprm->file)) goto error; + if (!elf_check_fdpic(&exec_params.hdr)) { +#ifdef CONFIG_MMU + /* binfmt_elf handles non-fdpic elf except on nommu */ + goto error; +#else + /* nommu can only load ET_DYN (PIE) ELF */ + if (exec_params.hdr.e_type != ET_DYN) + goto error; +#endif + } /* read the program header table */ retval = elf_fdpic_fetch_phdrs(&exec_params, bprm->file); @@ -269,13 +296,13 @@ } - if (elf_check_const_displacement(&exec_params.hdr)) + if (is_constdisp(&exec_params.hdr)) exec_params.flags |= ELF_FDPIC_FLAG_CONSTDISP; /* perform insanity checks on the interpreter */ if (interpreter_name) { retval = -ELIBBAD; - if (!is_elf_fdpic(&interp_params.hdr, interpreter)) + if (!is_elf(&interp_params.hdr, interpreter)) goto error; interp_params.flags = ELF_FDPIC_FLAG_PRESENT; @@ -306,9 +333,9 @@ retval = -ENOEXEC; if (stack_size == 0) - goto error; + stack_size = 131072UL; /* same as exec.c's default commit */ - if (elf_check_const_displacement(&interp_params.hdr)) + if (is_constdisp(&interp_params.hdr)) interp_params.flags |= ELF_FDPIC_FLAG_CONSTDISP; /* flush all traces of the currently running executable */ @@ -319,7 +346,10 @@ /* there's now no turning back... the old userspace image is dead, * defunct, deceased, etc. */ - set_personality(PER_LINUX_FDPIC); + if (elf_check_fdpic(&exec_params.hdr)) + set_personality(PER_LINUX_FDPIC); + else + set_personality(PER_LINUX); if (elf_read_implies_exec(&exec_params.hdr, executable_stack)) current->personality |= READ_IMPLIES_EXEC;