Re: [PATCH v2] fs/binfmt_elf_fdpic.c: provide NOMMU loader for regular ELF binaries
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-embedded" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] fs/binfmt_elf_fdpic.c: provide NOMMU loader for regular ELF binaries
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 >>> >>> 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 >> >> I have no problem with this, so from me: >> >> Acked-by: Greg Ungerer > > 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-embedded" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] fs/binfmt_elf_fdpic.c: provide NOMMU loader for regular ELF binaries
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 > > > >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 > > I have no problem with this, so from me: > > Acked-by: Greg Ungerer 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-embedded" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2] fs/binfmt_elf_fdpic.c: provide NOMMU loader for regular ELF binaries
Hi Rich, On 09/10/15 02:38, Rich Felker wrote: From: Rich Felker 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 I have no problem with this, so from me: Acked-by: Greg Ungerer --- 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 + +++ fs/binfmt_elf_fdpic.c 2015-10-07 05:11:33.702236056 + @@ -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 */
[PATCH v2] fs/binfmt_elf_fdpic.c: provide NOMMU loader for regular ELF binaries
From: Rich Felker 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 --- 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. 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 + +++ fs/binfmt_elf_fdpic.c 2015-10-07 05:11:33.702236056 + @@ -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; i