| Message ID | 87levzzts4.fsf_-_@email.froward.int.ebiederm.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | binfmt_flat: Remove shared library support | expand |
On Wed, 20 Apr 2022 07:58:03 PDT (-0700), ebiederm@xmission.com wrote: > > In a recent discussion[1] it was reported that the binfmt_flat library > support was only ever used on m68k and even on m68k has not been used > in a very long time. > > The structure of binfmt_flat is different from all of the other binfmt > implementations becasue of this shared library support and it made > life and code review more effort when I refactored the code in fs/exec.c. > > Since in practice the code is dead remove the binfmt_flat shared libarary > support and make maintenance of the code easier. > > [1] https://lkml.kernel.org/r/81788b56-5b15-7308-38c7-c7f2502c4e15@linux-m68k.org > Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com> > --- > > Can the binfmt_flat folks please verify that the shared library support > really isn't used? I don't actually know follow the RISC-V flat support, last I heard it was still sort of just in limbo (some toolchain/userspace bugs th at needed to be sorted out). Damien would know better, though, he's already on the thread. I'll leave it up to him to ack this one, if you were even looking for anything from the RISC-V folks at all (we don't have this in any defconfigs). > Was binfmt_flat being enabled on arm and sh the mistake it looks like? > > arch/arm/configs/lpc18xx_defconfig | 1 - > arch/arm/configs/mps2_defconfig | 1 - > arch/arm/configs/stm32_defconfig | 1 - > arch/arm/configs/vf610m4_defconfig | 1 - > arch/sh/configs/rsk7201_defconfig | 1 - > arch/sh/configs/rsk7203_defconfig | 1 - > arch/sh/configs/se7206_defconfig | 1 - > fs/Kconfig.binfmt | 6 - > fs/binfmt_flat.c | 190 ++++++----------------------- > 9 files changed, 40 insertions(+), 163 deletions(-) > > diff --git a/arch/arm/configs/lpc18xx_defconfig b/arch/arm/configs/lpc18xx_defconfig > index be882ea0eee4..688c9849eec8 100644 > --- a/arch/arm/configs/lpc18xx_defconfig > +++ b/arch/arm/configs/lpc18xx_defconfig > @@ -30,7 +30,6 @@ CONFIG_ARM_APPENDED_DTB=y > # CONFIG_BLK_DEV_BSG is not set > CONFIG_BINFMT_FLAT=y > CONFIG_BINFMT_ZFLAT=y > -CONFIG_BINFMT_SHARED_FLAT=y > # CONFIG_COREDUMP is not set > CONFIG_NET=y > CONFIG_PACKET=y > diff --git a/arch/arm/configs/mps2_defconfig b/arch/arm/configs/mps2_defconfig > index 89f4a6ff30bd..c1e98e33a348 100644 > --- a/arch/arm/configs/mps2_defconfig > +++ b/arch/arm/configs/mps2_defconfig > @@ -23,7 +23,6 @@ CONFIG_PREEMPT_VOLUNTARY=y > CONFIG_ZBOOT_ROM_TEXT=0x0 > CONFIG_ZBOOT_ROM_BSS=0x0 > CONFIG_BINFMT_FLAT=y > -CONFIG_BINFMT_SHARED_FLAT=y > # CONFIG_COREDUMP is not set > # CONFIG_SUSPEND is not set > CONFIG_NET=y > diff --git a/arch/arm/configs/stm32_defconfig b/arch/arm/configs/stm32_defconfig > index 551db328009d..71d6bfcf4551 100644 > --- a/arch/arm/configs/stm32_defconfig > +++ b/arch/arm/configs/stm32_defconfig > @@ -28,7 +28,6 @@ CONFIG_ZBOOT_ROM_BSS=0x0 > CONFIG_XIP_KERNEL=y > CONFIG_XIP_PHYS_ADDR=0x08008000 > CONFIG_BINFMT_FLAT=y > -CONFIG_BINFMT_SHARED_FLAT=y > # CONFIG_COREDUMP is not set > CONFIG_DEVTMPFS=y > CONFIG_DEVTMPFS_MOUNT=y > diff --git a/arch/arm/configs/vf610m4_defconfig b/arch/arm/configs/vf610m4_defconfig > index a89f035c3b01..70fdbfd83484 100644 > --- a/arch/arm/configs/vf610m4_defconfig > +++ b/arch/arm/configs/vf610m4_defconfig > @@ -18,7 +18,6 @@ CONFIG_XIP_KERNEL=y > CONFIG_XIP_PHYS_ADDR=0x0f000080 > CONFIG_BINFMT_FLAT=y > CONFIG_BINFMT_ZFLAT=y > -CONFIG_BINFMT_SHARED_FLAT=y > # CONFIG_SUSPEND is not set > # CONFIG_UEVENT_HELPER is not set > # CONFIG_STANDALONE is not set > diff --git a/arch/sh/configs/rsk7201_defconfig b/arch/sh/configs/rsk7201_defconfig > index e41526120be1..619c18699459 100644 > --- a/arch/sh/configs/rsk7201_defconfig > +++ b/arch/sh/configs/rsk7201_defconfig > @@ -25,7 +25,6 @@ CONFIG_CMDLINE_OVERWRITE=y > CONFIG_CMDLINE="console=ttySC0,115200 earlyprintk=serial ignore_loglevel" > CONFIG_BINFMT_FLAT=y > CONFIG_BINFMT_ZFLAT=y > -CONFIG_BINFMT_SHARED_FLAT=y > CONFIG_PM=y > CONFIG_CPU_IDLE=y > # CONFIG_STANDALONE is not set > diff --git a/arch/sh/configs/rsk7203_defconfig b/arch/sh/configs/rsk7203_defconfig > index 6af08fa1ddf8..5a54e2b883f0 100644 > --- a/arch/sh/configs/rsk7203_defconfig > +++ b/arch/sh/configs/rsk7203_defconfig > @@ -30,7 +30,6 @@ CONFIG_CMDLINE_OVERWRITE=y > CONFIG_CMDLINE="console=ttySC0,115200 earlyprintk=serial ignore_loglevel" > CONFIG_BINFMT_FLAT=y > CONFIG_BINFMT_ZFLAT=y > -CONFIG_BINFMT_SHARED_FLAT=y > CONFIG_PM=y > CONFIG_CPU_IDLE=y > CONFIG_NET=y > diff --git a/arch/sh/configs/se7206_defconfig b/arch/sh/configs/se7206_defconfig > index 601d062250d1..122216123e63 100644 > --- a/arch/sh/configs/se7206_defconfig > +++ b/arch/sh/configs/se7206_defconfig > @@ -40,7 +40,6 @@ CONFIG_CMDLINE_OVERWRITE=y > CONFIG_CMDLINE="console=ttySC3,115200 ignore_loglevel earlyprintk=serial" > CONFIG_BINFMT_FLAT=y > CONFIG_BINFMT_ZFLAT=y > -CONFIG_BINFMT_SHARED_FLAT=y > CONFIG_BINFMT_MISC=y > CONFIG_NET=y > CONFIG_PACKET=y > diff --git a/fs/Kconfig.binfmt b/fs/Kconfig.binfmt > index 21c6332fa785..32dff7ba3dda 100644 > --- a/fs/Kconfig.binfmt > +++ b/fs/Kconfig.binfmt > @@ -142,12 +142,6 @@ config BINFMT_ZFLAT > help > Support FLAT format compressed binaries > > -config BINFMT_SHARED_FLAT > - bool "Enable shared FLAT support" > - depends on BINFMT_FLAT > - help > - Support FLAT shared libraries > - > config HAVE_AOUT > def_bool n > > diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c > index 0ad2c7bbaddd..82e4412a9665 100644 > --- a/fs/binfmt_flat.c > +++ b/fs/binfmt_flat.c > @@ -68,11 +68,7 @@ > #define RELOC_FAILED 0xff00ff01 /* Relocation incorrect somewhere */ > #define UNLOADED_LIB 0x7ff000ff /* Placeholder for unused library */ > > -#ifdef CONFIG_BINFMT_SHARED_FLAT > -#define MAX_SHARED_LIBS (4) > -#else > -#define MAX_SHARED_LIBS (1) > -#endif > +#define MAX_SHARED_LIBS (1) > > #ifdef CONFIG_BINFMT_FLAT_NO_DATA_START_OFFSET > #define DATA_START_OFFSET_WORDS (0) > @@ -92,10 +88,6 @@ struct lib_info { > } lib_list[MAX_SHARED_LIBS]; > }; > > -#ifdef CONFIG_BINFMT_SHARED_FLAT > -static int load_flat_shared_library(int id, struct lib_info *p); > -#endif > - > static int load_flat_binary(struct linux_binprm *); > > static struct linux_binfmt flat_format = { > @@ -307,51 +299,18 @@ static int decompress_exec(struct linux_binprm *bprm, loff_t fpos, char *dst, > /****************************************************************************/ > > static unsigned long > -calc_reloc(unsigned long r, struct lib_info *p, int curid, int internalp) > +calc_reloc(unsigned long r, struct lib_info *p) > { > unsigned long addr; > - int id; > unsigned long start_brk; > unsigned long start_data; > unsigned long text_len; > unsigned long start_code; > > -#ifdef CONFIG_BINFMT_SHARED_FLAT > - if (r == 0) > - id = curid; /* Relocs of 0 are always self referring */ > - else { > - id = (r >> 24) & 0xff; /* Find ID for this reloc */ > - r &= 0x00ffffff; /* Trim ID off here */ > - } > - if (id >= MAX_SHARED_LIBS) { > - pr_err("reference 0x%lx to shared library %d", r, id); > - goto failed; > - } > - if (curid != id) { > - if (internalp) { > - pr_err("reloc address 0x%lx not in same module " > - "(%d != %d)", r, curid, id); > - goto failed; > - } else if (!p->lib_list[id].loaded && > - load_flat_shared_library(id, p) < 0) { > - pr_err("failed to load library %d", id); > - goto failed; > - } > - /* Check versioning information (i.e. time stamps) */ > - if (p->lib_list[id].build_date && p->lib_list[curid].build_date && > - p->lib_list[curid].build_date < p->lib_list[id].build_date) { > - pr_err("library %d is younger than %d", id, curid); > - goto failed; > - } > - } > -#else > - id = 0; > -#endif > - > - start_brk = p->lib_list[id].start_brk; > - start_data = p->lib_list[id].start_data; > - start_code = p->lib_list[id].start_code; > - text_len = p->lib_list[id].text_len; > + start_brk = p->lib_list[0].start_brk; > + start_data = p->lib_list[0].start_data; > + start_code = p->lib_list[0].start_code; > + text_len = p->lib_list[0].text_len; > > if (r > start_brk - start_data + text_len) { > pr_err("reloc outside program 0x%lx (0 - 0x%lx/0x%lx)", > @@ -419,7 +378,7 @@ static void old_reloc(unsigned long rl) > /****************************************************************************/ > > static int load_flat_file(struct linux_binprm *bprm, > - struct lib_info *libinfo, int id, unsigned long *extra_stack) > + struct lib_info *libinfo, unsigned long *extra_stack) > { > struct flat_hdr *hdr; > unsigned long textpos, datapos, realdatastart; > @@ -471,14 +430,6 @@ static int load_flat_file(struct linux_binprm *bprm, > goto err; > } > > - /* Don't allow old format executables to use shared libraries */ > - if (rev == OLD_FLAT_VERSION && id != 0) { > - pr_err("shared libraries are not available before rev 0x%lx\n", > - FLAT_VERSION); > - ret = -ENOEXEC; > - goto err; > - } > - > /* > * fix up the flags for the older format, there were all kinds > * of endian hacks, this only works for the simple cases > @@ -529,15 +480,13 @@ static int load_flat_file(struct linux_binprm *bprm, > } > > /* Flush all traces of the currently running executable */ > - if (id == 0) { > - ret = begin_new_exec(bprm); > - if (ret) > - goto err; > + ret = begin_new_exec(bprm); > + if (ret) > + goto err; > > - /* OK, This is the point of no return */ > - set_personality(PER_LINUX_32BIT); > - setup_new_exec(bprm); > - } > + /* OK, This is the point of no return */ > + set_personality(PER_LINUX_32BIT); > + setup_new_exec(bprm); > > /* > * calculate the extra space we need to map in > @@ -717,42 +666,40 @@ static int load_flat_file(struct linux_binprm *bprm, > text_len -= sizeof(struct flat_hdr); /* the real code len */ > > /* The main program needs a little extra setup in the task structure */ > - if (id == 0) { > - current->mm->start_code = start_code; > - current->mm->end_code = end_code; > - current->mm->start_data = datapos; > - current->mm->end_data = datapos + data_len; > - /* > - * set up the brk stuff, uses any slack left in data/bss/stack > - * allocation. We put the brk after the bss (between the bss > - * and stack) like other platforms. > - * Userspace code relies on the stack pointer starting out at > - * an address right at the end of a page. > - */ > - current->mm->start_brk = datapos + data_len + bss_len; > - current->mm->brk = (current->mm->start_brk + 3) & ~3; > + current->mm->start_code = start_code; > + current->mm->end_code = end_code; > + current->mm->start_data = datapos; > + current->mm->end_data = datapos + data_len; > + /* > + * set up the brk stuff, uses any slack left in data/bss/stack > + * allocation. We put the brk after the bss (between the bss > + * and stack) like other platforms. > + * Userspace code relies on the stack pointer starting out at > + * an address right at the end of a page. > + */ > + current->mm->start_brk = datapos + data_len + bss_len; > + current->mm->brk = (current->mm->start_brk + 3) & ~3; > #ifndef CONFIG_MMU > - current->mm->context.end_brk = memp + memp_size - stack_len; > + current->mm->context.end_brk = memp + memp_size - stack_len; > #endif > - } > > if (flags & FLAT_FLAG_KTRACE) { > pr_info("Mapping is %lx, Entry point is %x, data_start is %x\n", > textpos, 0x00ffffff&ntohl(hdr->entry), ntohl(hdr->data_start)); > pr_info("%s %s: TEXT=%lx-%lx DATA=%lx-%lx BSS=%lx-%lx\n", > - id ? "Lib" : "Load", bprm->filename, > + "Load", bprm->filename, > start_code, end_code, datapos, datapos + data_len, > datapos + data_len, (datapos + data_len + bss_len + 3) & ~3); > } > > /* Store the current module values into the global library structure */ > - libinfo->lib_list[id].start_code = start_code; > - libinfo->lib_list[id].start_data = datapos; > - libinfo->lib_list[id].start_brk = datapos + data_len + bss_len; > - libinfo->lib_list[id].text_len = text_len; > - libinfo->lib_list[id].loaded = 1; > - libinfo->lib_list[id].entry = (0x00ffffff & ntohl(hdr->entry)) + textpos; > - libinfo->lib_list[id].build_date = ntohl(hdr->build_date); > + libinfo->lib_list[0].start_code = start_code; > + libinfo->lib_list[0].start_data = datapos; > + libinfo->lib_list[0].start_brk = datapos + data_len + bss_len; > + libinfo->lib_list[0].text_len = text_len; > + libinfo->lib_list[0].loaded = 1; > + libinfo->lib_list[0].entry = (0x00ffffff & ntohl(hdr->entry)) + textpos; > + libinfo->lib_list[0].build_date = ntohl(hdr->build_date); > > /* > * We just load the allocations into some temporary memory to > @@ -774,7 +721,7 @@ static int load_flat_file(struct linux_binprm *bprm, > if (rp_val == 0xffffffff) > break; > if (rp_val) { > - addr = calc_reloc(rp_val, libinfo, id, 0); > + addr = calc_reloc(rp_val, libinfo); > if (addr == RELOC_FAILED) { > ret = -ENOEXEC; > goto err; > @@ -810,7 +757,7 @@ static int load_flat_file(struct linux_binprm *bprm, > return -EFAULT; > relval = ntohl(tmp); > addr = flat_get_relocate_addr(relval); > - rp = (u32 __user *)calc_reloc(addr, libinfo, id, 1); > + rp = (u32 __user *)calc_reloc(addr, libinfo); > if (rp == (u32 __user *)RELOC_FAILED) { > ret = -ENOEXEC; > goto err; > @@ -833,7 +780,7 @@ static int load_flat_file(struct linux_binprm *bprm, > */ > addr = ntohl((__force __be32)addr); > } > - addr = calc_reloc(addr, libinfo, id, 0); > + addr = calc_reloc(addr, libinfo); > if (addr == RELOC_FAILED) { > ret = -ENOEXEC; > goto err; > @@ -861,7 +808,7 @@ static int load_flat_file(struct linux_binprm *bprm, > /* zero the BSS, BRK and stack areas */ > if (clear_user((void __user *)(datapos + data_len), bss_len + > (memp + memp_size - stack_len - /* end brk */ > - libinfo->lib_list[id].start_brk) + /* start brk */ > + libinfo->lib_list[0].start_brk) + /* start brk */ > stack_len)) > return -EFAULT; > > @@ -871,49 +818,6 @@ static int load_flat_file(struct linux_binprm *bprm, > } > > > -/****************************************************************************/ > -#ifdef CONFIG_BINFMT_SHARED_FLAT > - > -/* > - * Load a shared library into memory. The library gets its own data > - * segment (including bss) but not argv/argc/environ. > - */ > - > -static int load_flat_shared_library(int id, struct lib_info *libs) > -{ > - /* > - * This is a fake bprm struct; only the members "buf", "file" and > - * "filename" are actually used. > - */ > - struct linux_binprm bprm; > - int res; > - char buf[16]; > - loff_t pos = 0; > - > - memset(&bprm, 0, sizeof(bprm)); > - > - /* Create the file name */ > - sprintf(buf, "/lib/lib%d.so", id); > - > - /* Open the file up */ > - bprm.filename = buf; > - bprm.file = open_exec(bprm.filename); > - res = PTR_ERR(bprm.file); > - if (IS_ERR(bprm.file)) > - return res; > - > - res = kernel_read(bprm.file, bprm.buf, BINPRM_BUF_SIZE, &pos); > - > - if (res >= 0) > - res = load_flat_file(&bprm, libs, id, NULL); > - > - allow_write_access(bprm.file); > - fput(bprm.file); > - > - return res; > -} > - > -#endif /* CONFIG_BINFMT_SHARED_FLAT */ > /****************************************************************************/ > > /* > @@ -946,7 +850,7 @@ static int load_flat_binary(struct linux_binprm *bprm) > stack_len += (bprm->envc + 1) * sizeof(char *); /* the envp array */ > stack_len = ALIGN(stack_len, FLAT_STACK_ALIGN); > > - res = load_flat_file(bprm, &libinfo, 0, &stack_len); > + res = load_flat_file(bprm, &libinfo, &stack_len); > if (res < 0) > return res; > > @@ -991,20 +895,6 @@ static int load_flat_binary(struct linux_binprm *bprm) > */ > start_addr = libinfo.lib_list[0].entry; > > -#ifdef CONFIG_BINFMT_SHARED_FLAT > - for (i = MAX_SHARED_LIBS-1; i > 0; i--) { > - if (libinfo.lib_list[i].loaded) { > - /* Push previos first to call address */ > - unsigned long __user *sp; > - current->mm->start_stack -= sizeof(unsigned long); > - sp = (unsigned long __user *)current->mm->start_stack; > - if (put_user(start_addr, sp)) > - return -EFAULT; > - start_addr = libinfo.lib_list[i].entry; > - } > - } > -#endif > - > #ifdef FLAT_PLAT_INIT > FLAT_PLAT_INIT(regs); > #endif
On Wed, Apr 20, 2022 at 09:17:22AM -0700, Palmer Dabbelt wrote: > On Wed, 20 Apr 2022 07:58:03 PDT (-0700), ebiederm@xmission.com wrote: > > > >In a recent discussion[1] it was reported that the binfmt_flat library > >support was only ever used on m68k and even on m68k has not been used > >in a very long time. > > > >The structure of binfmt_flat is different from all of the other binfmt > >implementations becasue of this shared library support and it made > >life and code review more effort when I refactored the code in fs/exec.c. > > > >Since in practice the code is dead remove the binfmt_flat shared libarary > >support and make maintenance of the code easier. > > > >[1] https://lkml.kernel.org/r/81788b56-5b15-7308-38c7-c7f2502c4e15@linux-m68k.org > >Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com> > >--- > > > >Can the binfmt_flat folks please verify that the shared library support > >really isn't used? > > I don't actually know follow the RISC-V flat support, last I heard it was still > sort of just in limbo (some toolchain/userspace bugs th at needed to be sorted > out). Damien would know better, though, he's already on the thread. I'll > leave it up to him to ack this one, if you were even looking for anything from > the RISC-V folks at all (we don't have this in any defconfigs). For what it's worth, bimfmt_flat (with or without shared library support) should be simple to implement as a binfmt_misc handler if anyone needs the old shared library support (or if kernel wanted to drop it entirely, which I would be in favor of). That's how I handled old aout binaries I wanted to run after aout was removed: trivial binfmt_misc loader. Rich
On Wed, Apr 20, 2022 at 12:59:37PM -0400, Rich Felker wrote: > On Wed, Apr 20, 2022 at 09:17:22AM -0700, Palmer Dabbelt wrote: > > On Wed, 20 Apr 2022 07:58:03 PDT (-0700), ebiederm@xmission.com wrote: > > > > > >In a recent discussion[1] it was reported that the binfmt_flat library > > >support was only ever used on m68k and even on m68k has not been used > > >in a very long time. > > > > > >The structure of binfmt_flat is different from all of the other binfmt > > >implementations becasue of this shared library support and it made > > >life and code review more effort when I refactored the code in fs/exec.c. > > > > > >Since in practice the code is dead remove the binfmt_flat shared libarary > > >support and make maintenance of the code easier. > > > > > >[1] https://lkml.kernel.org/r/81788b56-5b15-7308-38c7-c7f2502c4e15@linux-m68k.org > > >Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com> > > >--- > > > > > >Can the binfmt_flat folks please verify that the shared library support > > >really isn't used? > > > > I don't actually know follow the RISC-V flat support, last I heard it was still > > sort of just in limbo (some toolchain/userspace bugs th at needed to be sorted > > out). Damien would know better, though, he's already on the thread. I'll > > leave it up to him to ack this one, if you were even looking for anything from > > the RISC-V folks at all (we don't have this in any defconfigs). > > For what it's worth, bimfmt_flat (with or without shared library > support) should be simple to implement as a binfmt_misc handler if > anyone needs the old shared library support (or if kernel wanted to > drop it entirely, which I would be in favor of). That's how I handled > old aout binaries I wanted to run after aout was removed: trivial > binfmt_misc loader. Yeah, I was trying to understand why systems were using binfmt_flat and not binfmt_elf, given the mention of elf2flat -- is there really such a large kernel memory footprint savings to be had from removing binfmt_elf? But regardless, yes, it seems like if you're doing anything remotely needing shared libraries with binfmt_flat, such a system could just use ELF instead.
On Wed, Apr 20, 2022 at 7:47 PM Kees Cook <keescook@chromium.org> wrote: > > Yeah, I was trying to understand why systems were using binfmt_flat and > not binfmt_elf, given the mention of elf2flat -- is there really such a > large kernel memory footprint savings to be had from removing > binfmt_elf? I think the main reason for using flat binaries is nommu support on m68k, xtensa and risc-v. The regular binfmt_elf support requires an MMU, and the elf-fdpic variant is only available for arm and sh at this point (the other nommu architectures got removed over time). Arnd
On Wed, Apr 20, 2022 at 10:04:32PM +0200, Arnd Bergmann wrote: > On Wed, Apr 20, 2022 at 7:47 PM Kees Cook <keescook@chromium.org> wrote: > > > > Yeah, I was trying to understand why systems were using binfmt_flat and > > not binfmt_elf, given the mention of elf2flat -- is there really such a > > large kernel memory footprint savings to be had from removing > > binfmt_elf? > > I think the main reason for using flat binaries is nommu support on > m68k, xtensa and risc-v. The regular binfmt_elf support requires > an MMU, and the elf-fdpic variant is only available for arm and sh > at this point (the other nommu architectures got removed over time). I believe I made the elf-fdpic loader so that it's capable of loading normal non-fdpic elf files on nommu (1bde925d23), unless somebody broke that. I also seem to recall that capability being added to the main elf loader later. Rich
On 4/21/22 05:23, Rich Felker wrote: > On Wed, Apr 20, 2022 at 10:04:32PM +0200, Arnd Bergmann wrote: >> On Wed, Apr 20, 2022 at 7:47 PM Kees Cook <keescook@chromium.org> wrote: >>> >>> Yeah, I was trying to understand why systems were using binfmt_flat and >>> not binfmt_elf, given the mention of elf2flat -- is there really such a >>> large kernel memory footprint savings to be had from removing >>> binfmt_elf? >> >> I think the main reason for using flat binaries is nommu support on >> m68k, xtensa and risc-v. The regular binfmt_elf support requires >> an MMU, and the elf-fdpic variant is only available for arm and sh >> at this point (the other nommu architectures got removed over time). > > I believe I made the elf-fdpic loader so that it's capable of loading > normal non-fdpic elf files on nommu (1bde925d23), unless somebody > broke that. I also seem to recall that capability being added to the > main elf loader later. Last time I checked, building shared libraries usable with nommu riscv required gcc/ld options that were not supported for riscv (PIE related stuff). So removing the kernel support for shared flat libs is fine with me.
On 4/20/22 23:58, Eric W. Biederman wrote: > Nit: extra blank line here. > In a recent discussion[1] it was reported that the binfmt_flat library > support was only ever used on m68k and even on m68k has not been used > in a very long time. > > The structure of binfmt_flat is different from all of the other binfmt > implementations becasue of this shared library support and it made s/becasue/because > life and code review more effort when I refactored the code in fs/exec.c. > > Since in practice the code is dead remove the binfmt_flat shared libarary s/libarary/library > support and make maintenance of the code easier. > > [1] https://lkml.kernel.org/r/81788b56-5b15-7308-38c7-c7f2502c4e15@linux-m68k.org > Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com> > --- > > Can the binfmt_flat folks please verify that the shared library support > really isn't used? As mentioned in a different email, it is not supported by the toolchains for riscv. > > Was binfmt_flat being enabled on arm and sh the mistake it looks like? > > arch/arm/configs/lpc18xx_defconfig | 1 - > arch/arm/configs/mps2_defconfig | 1 - > arch/arm/configs/stm32_defconfig | 1 - > arch/arm/configs/vf610m4_defconfig | 1 - > arch/sh/configs/rsk7201_defconfig | 1 - > arch/sh/configs/rsk7203_defconfig | 1 - > arch/sh/configs/se7206_defconfig | 1 - > fs/Kconfig.binfmt | 6 - > fs/binfmt_flat.c | 190 ++++++----------------------- > 9 files changed, 40 insertions(+), 163 deletions(-) > > diff --git a/arch/arm/configs/lpc18xx_defconfig b/arch/arm/configs/lpc18xx_defconfig > index be882ea0eee4..688c9849eec8 100644 > --- a/arch/arm/configs/lpc18xx_defconfig > +++ b/arch/arm/configs/lpc18xx_defconfig > @@ -30,7 +30,6 @@ CONFIG_ARM_APPENDED_DTB=y > # CONFIG_BLK_DEV_BSG is not set > CONFIG_BINFMT_FLAT=y > CONFIG_BINFMT_ZFLAT=y > -CONFIG_BINFMT_SHARED_FLAT=y > # CONFIG_COREDUMP is not set > CONFIG_NET=y > CONFIG_PACKET=y > diff --git a/arch/arm/configs/mps2_defconfig b/arch/arm/configs/mps2_defconfig > index 89f4a6ff30bd..c1e98e33a348 100644 > --- a/arch/arm/configs/mps2_defconfig > +++ b/arch/arm/configs/mps2_defconfig > @@ -23,7 +23,6 @@ CONFIG_PREEMPT_VOLUNTARY=y > CONFIG_ZBOOT_ROM_TEXT=0x0 > CONFIG_ZBOOT_ROM_BSS=0x0 > CONFIG_BINFMT_FLAT=y > -CONFIG_BINFMT_SHARED_FLAT=y > # CONFIG_COREDUMP is not set > # CONFIG_SUSPEND is not set > CONFIG_NET=y > diff --git a/arch/arm/configs/stm32_defconfig b/arch/arm/configs/stm32_defconfig > index 551db328009d..71d6bfcf4551 100644 > --- a/arch/arm/configs/stm32_defconfig > +++ b/arch/arm/configs/stm32_defconfig > @@ -28,7 +28,6 @@ CONFIG_ZBOOT_ROM_BSS=0x0 > CONFIG_XIP_KERNEL=y > CONFIG_XIP_PHYS_ADDR=0x08008000 > CONFIG_BINFMT_FLAT=y > -CONFIG_BINFMT_SHARED_FLAT=y > # CONFIG_COREDUMP is not set > CONFIG_DEVTMPFS=y > CONFIG_DEVTMPFS_MOUNT=y > diff --git a/arch/arm/configs/vf610m4_defconfig b/arch/arm/configs/vf610m4_defconfig > index a89f035c3b01..70fdbfd83484 100644 > --- a/arch/arm/configs/vf610m4_defconfig > +++ b/arch/arm/configs/vf610m4_defconfig > @@ -18,7 +18,6 @@ CONFIG_XIP_KERNEL=y > CONFIG_XIP_PHYS_ADDR=0x0f000080 > CONFIG_BINFMT_FLAT=y > CONFIG_BINFMT_ZFLAT=y > -CONFIG_BINFMT_SHARED_FLAT=y > # CONFIG_SUSPEND is not set > # CONFIG_UEVENT_HELPER is not set > # CONFIG_STANDALONE is not set > diff --git a/arch/sh/configs/rsk7201_defconfig b/arch/sh/configs/rsk7201_defconfig > index e41526120be1..619c18699459 100644 > --- a/arch/sh/configs/rsk7201_defconfig > +++ b/arch/sh/configs/rsk7201_defconfig > @@ -25,7 +25,6 @@ CONFIG_CMDLINE_OVERWRITE=y > CONFIG_CMDLINE="console=ttySC0,115200 earlyprintk=serial ignore_loglevel" > CONFIG_BINFMT_FLAT=y > CONFIG_BINFMT_ZFLAT=y > -CONFIG_BINFMT_SHARED_FLAT=y > CONFIG_PM=y > CONFIG_CPU_IDLE=y > # CONFIG_STANDALONE is not set > diff --git a/arch/sh/configs/rsk7203_defconfig b/arch/sh/configs/rsk7203_defconfig > index 6af08fa1ddf8..5a54e2b883f0 100644 > --- a/arch/sh/configs/rsk7203_defconfig > +++ b/arch/sh/configs/rsk7203_defconfig > @@ -30,7 +30,6 @@ CONFIG_CMDLINE_OVERWRITE=y > CONFIG_CMDLINE="console=ttySC0,115200 earlyprintk=serial ignore_loglevel" > CONFIG_BINFMT_FLAT=y > CONFIG_BINFMT_ZFLAT=y > -CONFIG_BINFMT_SHARED_FLAT=y > CONFIG_PM=y > CONFIG_CPU_IDLE=y > CONFIG_NET=y > diff --git a/arch/sh/configs/se7206_defconfig b/arch/sh/configs/se7206_defconfig > index 601d062250d1..122216123e63 100644 > --- a/arch/sh/configs/se7206_defconfig > +++ b/arch/sh/configs/se7206_defconfig > @@ -40,7 +40,6 @@ CONFIG_CMDLINE_OVERWRITE=y > CONFIG_CMDLINE="console=ttySC3,115200 ignore_loglevel earlyprintk=serial" > CONFIG_BINFMT_FLAT=y > CONFIG_BINFMT_ZFLAT=y > -CONFIG_BINFMT_SHARED_FLAT=y > CONFIG_BINFMT_MISC=y > CONFIG_NET=y > CONFIG_PACKET=y > diff --git a/fs/Kconfig.binfmt b/fs/Kconfig.binfmt > index 21c6332fa785..32dff7ba3dda 100644 > --- a/fs/Kconfig.binfmt > +++ b/fs/Kconfig.binfmt > @@ -142,12 +142,6 @@ config BINFMT_ZFLAT > help > Support FLAT format compressed binaries > > -config BINFMT_SHARED_FLAT > - bool "Enable shared FLAT support" > - depends on BINFMT_FLAT > - help > - Support FLAT shared libraries > - > config HAVE_AOUT > def_bool n > > diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c > index 0ad2c7bbaddd..82e4412a9665 100644 > --- a/fs/binfmt_flat.c > +++ b/fs/binfmt_flat.c > @@ -68,11 +68,7 @@ > #define RELOC_FAILED 0xff00ff01 /* Relocation incorrect somewhere */ > #define UNLOADED_LIB 0x7ff000ff /* Placeholder for unused library */ > > -#ifdef CONFIG_BINFMT_SHARED_FLAT > -#define MAX_SHARED_LIBS (4) > -#else > -#define MAX_SHARED_LIBS (1) > -#endif > +#define MAX_SHARED_LIBS (1) > > #ifdef CONFIG_BINFMT_FLAT_NO_DATA_START_OFFSET > #define DATA_START_OFFSET_WORDS (0) > @@ -92,10 +88,6 @@ struct lib_info { > } lib_list[MAX_SHARED_LIBS]; > }; > > -#ifdef CONFIG_BINFMT_SHARED_FLAT > -static int load_flat_shared_library(int id, struct lib_info *p); > -#endif > - > static int load_flat_binary(struct linux_binprm *); > > static struct linux_binfmt flat_format = { > @@ -307,51 +299,18 @@ static int decompress_exec(struct linux_binprm *bprm, loff_t fpos, char *dst, > /****************************************************************************/ > > static unsigned long > -calc_reloc(unsigned long r, struct lib_info *p, int curid, int internalp) > +calc_reloc(unsigned long r, struct lib_info *p) > { > unsigned long addr; > - int id; > unsigned long start_brk; > unsigned long start_data; > unsigned long text_len; > unsigned long start_code; > > -#ifdef CONFIG_BINFMT_SHARED_FLAT > - if (r == 0) > - id = curid; /* Relocs of 0 are always self referring */ > - else { > - id = (r >> 24) & 0xff; /* Find ID for this reloc */ > - r &= 0x00ffffff; /* Trim ID off here */ > - } > - if (id >= MAX_SHARED_LIBS) { > - pr_err("reference 0x%lx to shared library %d", r, id); > - goto failed; > - } > - if (curid != id) { > - if (internalp) { > - pr_err("reloc address 0x%lx not in same module " > - "(%d != %d)", r, curid, id); > - goto failed; > - } else if (!p->lib_list[id].loaded && > - load_flat_shared_library(id, p) < 0) { > - pr_err("failed to load library %d", id); > - goto failed; > - } > - /* Check versioning information (i.e. time stamps) */ > - if (p->lib_list[id].build_date && p->lib_list[curid].build_date && > - p->lib_list[curid].build_date < p->lib_list[id].build_date) { > - pr_err("library %d is younger than %d", id, curid); > - goto failed; > - } > - } > -#else > - id = 0; > -#endif > - > - start_brk = p->lib_list[id].start_brk; > - start_data = p->lib_list[id].start_data; > - start_code = p->lib_list[id].start_code; > - text_len = p->lib_list[id].text_len; > + start_brk = p->lib_list[0].start_brk; > + start_data = p->lib_list[0].start_data; > + start_code = p->lib_list[0].start_code; > + text_len = p->lib_list[0].text_len; > > if (r > start_brk - start_data + text_len) { > pr_err("reloc outside program 0x%lx (0 - 0x%lx/0x%lx)", > @@ -419,7 +378,7 @@ static void old_reloc(unsigned long rl) > /****************************************************************************/ > > static int load_flat_file(struct linux_binprm *bprm, > - struct lib_info *libinfo, int id, unsigned long *extra_stack) > + struct lib_info *libinfo, unsigned long *extra_stack) > { > struct flat_hdr *hdr; > unsigned long textpos, datapos, realdatastart; > @@ -471,14 +430,6 @@ static int load_flat_file(struct linux_binprm *bprm, > goto err; > } > > - /* Don't allow old format executables to use shared libraries */ > - if (rev == OLD_FLAT_VERSION && id != 0) { > - pr_err("shared libraries are not available before rev 0x%lx\n", > - FLAT_VERSION); > - ret = -ENOEXEC; > - goto err; > - } > - > /* > * fix up the flags for the older format, there were all kinds > * of endian hacks, this only works for the simple cases > @@ -529,15 +480,13 @@ static int load_flat_file(struct linux_binprm *bprm, > } > > /* Flush all traces of the currently running executable */ > - if (id == 0) { > - ret = begin_new_exec(bprm); > - if (ret) > - goto err; > + ret = begin_new_exec(bprm); > + if (ret) > + goto err; > > - /* OK, This is the point of no return */ > - set_personality(PER_LINUX_32BIT); > - setup_new_exec(bprm); > - } > + /* OK, This is the point of no return */ > + set_personality(PER_LINUX_32BIT); > + setup_new_exec(bprm); > > /* > * calculate the extra space we need to map in > @@ -717,42 +666,40 @@ static int load_flat_file(struct linux_binprm *bprm, > text_len -= sizeof(struct flat_hdr); /* the real code len */ > > /* The main program needs a little extra setup in the task structure */ > - if (id == 0) { > - current->mm->start_code = start_code; > - current->mm->end_code = end_code; > - current->mm->start_data = datapos; > - current->mm->end_data = datapos + data_len; > - /* > - * set up the brk stuff, uses any slack left in data/bss/stack > - * allocation. We put the brk after the bss (between the bss > - * and stack) like other platforms. > - * Userspace code relies on the stack pointer starting out at > - * an address right at the end of a page. > - */ > - current->mm->start_brk = datapos + data_len + bss_len; > - current->mm->brk = (current->mm->start_brk + 3) & ~3; > + current->mm->start_code = start_code; > + current->mm->end_code = end_code; > + current->mm->start_data = datapos; > + current->mm->end_data = datapos + data_len; > + /* > + * set up the brk stuff, uses any slack left in data/bss/stack > + * allocation. We put the brk after the bss (between the bss > + * and stack) like other platforms. > + * Userspace code relies on the stack pointer starting out at > + * an address right at the end of a page. > + */ > + current->mm->start_brk = datapos + data_len + bss_len; > + current->mm->brk = (current->mm->start_brk + 3) & ~3; > #ifndef CONFIG_MMU > - current->mm->context.end_brk = memp + memp_size - stack_len; > + current->mm->context.end_brk = memp + memp_size - stack_len; > #endif > - } > > if (flags & FLAT_FLAG_KTRACE) { > pr_info("Mapping is %lx, Entry point is %x, data_start is %x\n", > textpos, 0x00ffffff&ntohl(hdr->entry), ntohl(hdr->data_start)); > pr_info("%s %s: TEXT=%lx-%lx DATA=%lx-%lx BSS=%lx-%lx\n", > - id ? "Lib" : "Load", bprm->filename, > + "Load", bprm->filename, > start_code, end_code, datapos, datapos + data_len, > datapos + data_len, (datapos + data_len + bss_len + 3) & ~3); > } > > /* Store the current module values into the global library structure */ > - libinfo->lib_list[id].start_code = start_code; > - libinfo->lib_list[id].start_data = datapos; > - libinfo->lib_list[id].start_brk = datapos + data_len + bss_len; > - libinfo->lib_list[id].text_len = text_len; > - libinfo->lib_list[id].loaded = 1; > - libinfo->lib_list[id].entry = (0x00ffffff & ntohl(hdr->entry)) + textpos; > - libinfo->lib_list[id].build_date = ntohl(hdr->build_date); > + libinfo->lib_list[0].start_code = start_code; > + libinfo->lib_list[0].start_data = datapos; > + libinfo->lib_list[0].start_brk = datapos + data_len + bss_len; > + libinfo->lib_list[0].text_len = text_len; > + libinfo->lib_list[0].loaded = 1; > + libinfo->lib_list[0].entry = (0x00ffffff & ntohl(hdr->entry)) + textpos; > + libinfo->lib_list[0].build_date = ntohl(hdr->build_date); > > /* > * We just load the allocations into some temporary memory to > @@ -774,7 +721,7 @@ static int load_flat_file(struct linux_binprm *bprm, > if (rp_val == 0xffffffff) > break; > if (rp_val) { > - addr = calc_reloc(rp_val, libinfo, id, 0); > + addr = calc_reloc(rp_val, libinfo); > if (addr == RELOC_FAILED) { > ret = -ENOEXEC; > goto err; > @@ -810,7 +757,7 @@ static int load_flat_file(struct linux_binprm *bprm, > return -EFAULT; > relval = ntohl(tmp); > addr = flat_get_relocate_addr(relval); > - rp = (u32 __user *)calc_reloc(addr, libinfo, id, 1); > + rp = (u32 __user *)calc_reloc(addr, libinfo); > if (rp == (u32 __user *)RELOC_FAILED) { > ret = -ENOEXEC; > goto err; > @@ -833,7 +780,7 @@ static int load_flat_file(struct linux_binprm *bprm, > */ > addr = ntohl((__force __be32)addr); > } > - addr = calc_reloc(addr, libinfo, id, 0); > + addr = calc_reloc(addr, libinfo); > if (addr == RELOC_FAILED) { > ret = -ENOEXEC; > goto err; > @@ -861,7 +808,7 @@ static int load_flat_file(struct linux_binprm *bprm, > /* zero the BSS, BRK and stack areas */ > if (clear_user((void __user *)(datapos + data_len), bss_len + > (memp + memp_size - stack_len - /* end brk */ > - libinfo->lib_list[id].start_brk) + /* start brk */ > + libinfo->lib_list[0].start_brk) + /* start brk */ > stack_len)) > return -EFAULT; > > @@ -871,49 +818,6 @@ static int load_flat_file(struct linux_binprm *bprm, > } > > > -/****************************************************************************/ > -#ifdef CONFIG_BINFMT_SHARED_FLAT > - > -/* > - * Load a shared library into memory. The library gets its own data > - * segment (including bss) but not argv/argc/environ. > - */ > - > -static int load_flat_shared_library(int id, struct lib_info *libs) > -{ > - /* > - * This is a fake bprm struct; only the members "buf", "file" and > - * "filename" are actually used. > - */ > - struct linux_binprm bprm; > - int res; > - char buf[16]; > - loff_t pos = 0; > - > - memset(&bprm, 0, sizeof(bprm)); > - > - /* Create the file name */ > - sprintf(buf, "/lib/lib%d.so", id); > - > - /* Open the file up */ > - bprm.filename = buf; > - bprm.file = open_exec(bprm.filename); > - res = PTR_ERR(bprm.file); > - if (IS_ERR(bprm.file)) > - return res; > - > - res = kernel_read(bprm.file, bprm.buf, BINPRM_BUF_SIZE, &pos); > - > - if (res >= 0) > - res = load_flat_file(&bprm, libs, id, NULL); > - > - allow_write_access(bprm.file); > - fput(bprm.file); > - > - return res; > -} > - > -#endif /* CONFIG_BINFMT_SHARED_FLAT */ > /****************************************************************************/ > > /* > @@ -946,7 +850,7 @@ static int load_flat_binary(struct linux_binprm *bprm) > stack_len += (bprm->envc + 1) * sizeof(char *); /* the envp array */ > stack_len = ALIGN(stack_len, FLAT_STACK_ALIGN); > > - res = load_flat_file(bprm, &libinfo, 0, &stack_len); > + res = load_flat_file(bprm, &libinfo, &stack_len); > if (res < 0) > return res; > > @@ -991,20 +895,6 @@ static int load_flat_binary(struct linux_binprm *bprm) > */ > start_addr = libinfo.lib_list[0].entry; > > -#ifdef CONFIG_BINFMT_SHARED_FLAT > - for (i = MAX_SHARED_LIBS-1; i > 0; i--) { > - if (libinfo.lib_list[i].loaded) { > - /* Push previos first to call address */ > - unsigned long __user *sp; > - current->mm->start_stack -= sizeof(unsigned long); > - sp = (unsigned long __user *)current->mm->start_stack; > - if (put_user(start_addr, sp)) > - return -EFAULT; > - start_addr = libinfo.lib_list[i].entry; > - } > - } > -#endif > - > #ifdef FLAT_PLAT_INIT > FLAT_PLAT_INIT(regs); > #endif Apart from the typos mentioned above, looks OK to me. Reviewed-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Hi Eric, On 21/4/22 00:58, Eric W. Biederman wrote: > In a recent discussion[1] it was reported that the binfmt_flat library > support was only ever used on m68k and even on m68k has not been used > in a very long time. > > The structure of binfmt_flat is different from all of the other binfmt > implementations becasue of this shared library support and it made > life and code review more effort when I refactored the code in fs/exec.c. > > Since in practice the code is dead remove the binfmt_flat shared libarary > support and make maintenance of the code easier. > > [1] https://lkml.kernel.org/r/81788b56-5b15-7308-38c7-c7f2502c4e15@linux-m68k.org > Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com> > --- > > Can the binfmt_flat folks please verify that the shared library support > really isn't used? I can definitely confirm I don't use it on m68k. And I don't know of anyone that has used it in many years. > Was binfmt_flat being enabled on arm and sh the mistake it looks like? > > arch/arm/configs/lpc18xx_defconfig | 1 - > arch/arm/configs/mps2_defconfig | 1 - > arch/arm/configs/stm32_defconfig | 1 - > arch/arm/configs/vf610m4_defconfig | 1 - binfmt_flat works on ARM. I use it all the time. According to those defconfigs those are all non-MMU systems, so having binfmt_flat enabled makes some sense there. > arch/sh/configs/rsk7201_defconfig | 1 - > arch/sh/configs/rsk7203_defconfig | 1 - > arch/sh/configs/se7206_defconfig | 1 - Those are all SH2 systems if I am reading the defconfigs correctly. SH2 is non-MMU according to the Kconfig setup. So it makes sense that binfmt_flat is enabled on those too. Don't forget that binfmt_flat works on many MMU enabled systems too. I regularly test/check it on MMU enabled m68k systems for example. Regards Greg > fs/Kconfig.binfmt | 6 - > fs/binfmt_flat.c | 190 ++++++----------------------- > 9 files changed, 40 insertions(+), 163 deletions(-) > > diff --git a/arch/arm/configs/lpc18xx_defconfig b/arch/arm/configs/lpc18xx_defconfig > index be882ea0eee4..688c9849eec8 100644 > --- a/arch/arm/configs/lpc18xx_defconfig > +++ b/arch/arm/configs/lpc18xx_defconfig > @@ -30,7 +30,6 @@ CONFIG_ARM_APPENDED_DTB=y > # CONFIG_BLK_DEV_BSG is not set > CONFIG_BINFMT_FLAT=y > CONFIG_BINFMT_ZFLAT=y > -CONFIG_BINFMT_SHARED_FLAT=y > # CONFIG_COREDUMP is not set > CONFIG_NET=y > CONFIG_PACKET=y > diff --git a/arch/arm/configs/mps2_defconfig b/arch/arm/configs/mps2_defconfig > index 89f4a6ff30bd..c1e98e33a348 100644 > --- a/arch/arm/configs/mps2_defconfig > +++ b/arch/arm/configs/mps2_defconfig > @@ -23,7 +23,6 @@ CONFIG_PREEMPT_VOLUNTARY=y > CONFIG_ZBOOT_ROM_TEXT=0x0 > CONFIG_ZBOOT_ROM_BSS=0x0 > CONFIG_BINFMT_FLAT=y > -CONFIG_BINFMT_SHARED_FLAT=y > # CONFIG_COREDUMP is not set > # CONFIG_SUSPEND is not set > CONFIG_NET=y > diff --git a/arch/arm/configs/stm32_defconfig b/arch/arm/configs/stm32_defconfig > index 551db328009d..71d6bfcf4551 100644 > --- a/arch/arm/configs/stm32_defconfig > +++ b/arch/arm/configs/stm32_defconfig > @@ -28,7 +28,6 @@ CONFIG_ZBOOT_ROM_BSS=0x0 > CONFIG_XIP_KERNEL=y > CONFIG_XIP_PHYS_ADDR=0x08008000 > CONFIG_BINFMT_FLAT=y > -CONFIG_BINFMT_SHARED_FLAT=y > # CONFIG_COREDUMP is not set > CONFIG_DEVTMPFS=y > CONFIG_DEVTMPFS_MOUNT=y > diff --git a/arch/arm/configs/vf610m4_defconfig b/arch/arm/configs/vf610m4_defconfig > index a89f035c3b01..70fdbfd83484 100644 > --- a/arch/arm/configs/vf610m4_defconfig > +++ b/arch/arm/configs/vf610m4_defconfig > @@ -18,7 +18,6 @@ CONFIG_XIP_KERNEL=y > CONFIG_XIP_PHYS_ADDR=0x0f000080 > CONFIG_BINFMT_FLAT=y > CONFIG_BINFMT_ZFLAT=y > -CONFIG_BINFMT_SHARED_FLAT=y > # CONFIG_SUSPEND is not set > # CONFIG_UEVENT_HELPER is not set > # CONFIG_STANDALONE is not set > diff --git a/arch/sh/configs/rsk7201_defconfig b/arch/sh/configs/rsk7201_defconfig > index e41526120be1..619c18699459 100644 > --- a/arch/sh/configs/rsk7201_defconfig > +++ b/arch/sh/configs/rsk7201_defconfig > @@ -25,7 +25,6 @@ CONFIG_CMDLINE_OVERWRITE=y > CONFIG_CMDLINE="console=ttySC0,115200 earlyprintk=serial ignore_loglevel" > CONFIG_BINFMT_FLAT=y > CONFIG_BINFMT_ZFLAT=y > -CONFIG_BINFMT_SHARED_FLAT=y > CONFIG_PM=y > CONFIG_CPU_IDLE=y > # CONFIG_STANDALONE is not set > diff --git a/arch/sh/configs/rsk7203_defconfig b/arch/sh/configs/rsk7203_defconfig > index 6af08fa1ddf8..5a54e2b883f0 100644 > --- a/arch/sh/configs/rsk7203_defconfig > +++ b/arch/sh/configs/rsk7203_defconfig > @@ -30,7 +30,6 @@ CONFIG_CMDLINE_OVERWRITE=y > CONFIG_CMDLINE="console=ttySC0,115200 earlyprintk=serial ignore_loglevel" > CONFIG_BINFMT_FLAT=y > CONFIG_BINFMT_ZFLAT=y > -CONFIG_BINFMT_SHARED_FLAT=y > CONFIG_PM=y > CONFIG_CPU_IDLE=y > CONFIG_NET=y > diff --git a/arch/sh/configs/se7206_defconfig b/arch/sh/configs/se7206_defconfig > index 601d062250d1..122216123e63 100644 > --- a/arch/sh/configs/se7206_defconfig > +++ b/arch/sh/configs/se7206_defconfig > @@ -40,7 +40,6 @@ CONFIG_CMDLINE_OVERWRITE=y > CONFIG_CMDLINE="console=ttySC3,115200 ignore_loglevel earlyprintk=serial" > CONFIG_BINFMT_FLAT=y > CONFIG_BINFMT_ZFLAT=y > -CONFIG_BINFMT_SHARED_FLAT=y > CONFIG_BINFMT_MISC=y > CONFIG_NET=y > CONFIG_PACKET=y > diff --git a/fs/Kconfig.binfmt b/fs/Kconfig.binfmt > index 21c6332fa785..32dff7ba3dda 100644 > --- a/fs/Kconfig.binfmt > +++ b/fs/Kconfig.binfmt > @@ -142,12 +142,6 @@ config BINFMT_ZFLAT > help > Support FLAT format compressed binaries > > -config BINFMT_SHARED_FLAT > - bool "Enable shared FLAT support" > - depends on BINFMT_FLAT > - help > - Support FLAT shared libraries > - > config HAVE_AOUT > def_bool n > > diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c > index 0ad2c7bbaddd..82e4412a9665 100644 > --- a/fs/binfmt_flat.c > +++ b/fs/binfmt_flat.c > @@ -68,11 +68,7 @@ > #define RELOC_FAILED 0xff00ff01 /* Relocation incorrect somewhere */ > #define UNLOADED_LIB 0x7ff000ff /* Placeholder for unused library */ > > -#ifdef CONFIG_BINFMT_SHARED_FLAT > -#define MAX_SHARED_LIBS (4) > -#else > -#define MAX_SHARED_LIBS (1) > -#endif > +#define MAX_SHARED_LIBS (1) > > #ifdef CONFIG_BINFMT_FLAT_NO_DATA_START_OFFSET > #define DATA_START_OFFSET_WORDS (0) > @@ -92,10 +88,6 @@ struct lib_info { > } lib_list[MAX_SHARED_LIBS]; > }; > > -#ifdef CONFIG_BINFMT_SHARED_FLAT > -static int load_flat_shared_library(int id, struct lib_info *p); > -#endif > - > static int load_flat_binary(struct linux_binprm *); > > static struct linux_binfmt flat_format = { > @@ -307,51 +299,18 @@ static int decompress_exec(struct linux_binprm *bprm, loff_t fpos, char *dst, > /****************************************************************************/ > > static unsigned long > -calc_reloc(unsigned long r, struct lib_info *p, int curid, int internalp) > +calc_reloc(unsigned long r, struct lib_info *p) > { > unsigned long addr; > - int id; > unsigned long start_brk; > unsigned long start_data; > unsigned long text_len; > unsigned long start_code; > > -#ifdef CONFIG_BINFMT_SHARED_FLAT > - if (r == 0) > - id = curid; /* Relocs of 0 are always self referring */ > - else { > - id = (r >> 24) & 0xff; /* Find ID for this reloc */ > - r &= 0x00ffffff; /* Trim ID off here */ > - } > - if (id >= MAX_SHARED_LIBS) { > - pr_err("reference 0x%lx to shared library %d", r, id); > - goto failed; > - } > - if (curid != id) { > - if (internalp) { > - pr_err("reloc address 0x%lx not in same module " > - "(%d != %d)", r, curid, id); > - goto failed; > - } else if (!p->lib_list[id].loaded && > - load_flat_shared_library(id, p) < 0) { > - pr_err("failed to load library %d", id); > - goto failed; > - } > - /* Check versioning information (i.e. time stamps) */ > - if (p->lib_list[id].build_date && p->lib_list[curid].build_date && > - p->lib_list[curid].build_date < p->lib_list[id].build_date) { > - pr_err("library %d is younger than %d", id, curid); > - goto failed; > - } > - } > -#else > - id = 0; > -#endif > - > - start_brk = p->lib_list[id].start_brk; > - start_data = p->lib_list[id].start_data; > - start_code = p->lib_list[id].start_code; > - text_len = p->lib_list[id].text_len; > + start_brk = p->lib_list[0].start_brk; > + start_data = p->lib_list[0].start_data; > + start_code = p->lib_list[0].start_code; > + text_len = p->lib_list[0].text_len; > > if (r > start_brk - start_data + text_len) { > pr_err("reloc outside program 0x%lx (0 - 0x%lx/0x%lx)", > @@ -419,7 +378,7 @@ static void old_reloc(unsigned long rl) > /****************************************************************************/ > > static int load_flat_file(struct linux_binprm *bprm, > - struct lib_info *libinfo, int id, unsigned long *extra_stack) > + struct lib_info *libinfo, unsigned long *extra_stack) > { > struct flat_hdr *hdr; > unsigned long textpos, datapos, realdatastart; > @@ -471,14 +430,6 @@ static int load_flat_file(struct linux_binprm *bprm, > goto err; > } > > - /* Don't allow old format executables to use shared libraries */ > - if (rev == OLD_FLAT_VERSION && id != 0) { > - pr_err("shared libraries are not available before rev 0x%lx\n", > - FLAT_VERSION); > - ret = -ENOEXEC; > - goto err; > - } > - > /* > * fix up the flags for the older format, there were all kinds > * of endian hacks, this only works for the simple cases > @@ -529,15 +480,13 @@ static int load_flat_file(struct linux_binprm *bprm, > } > > /* Flush all traces of the currently running executable */ > - if (id == 0) { > - ret = begin_new_exec(bprm); > - if (ret) > - goto err; > + ret = begin_new_exec(bprm); > + if (ret) > + goto err; > > - /* OK, This is the point of no return */ > - set_personality(PER_LINUX_32BIT); > - setup_new_exec(bprm); > - } > + /* OK, This is the point of no return */ > + set_personality(PER_LINUX_32BIT); > + setup_new_exec(bprm); > > /* > * calculate the extra space we need to map in > @@ -717,42 +666,40 @@ static int load_flat_file(struct linux_binprm *bprm, > text_len -= sizeof(struct flat_hdr); /* the real code len */ > > /* The main program needs a little extra setup in the task structure */ > - if (id == 0) { > - current->mm->start_code = start_code; > - current->mm->end_code = end_code; > - current->mm->start_data = datapos; > - current->mm->end_data = datapos + data_len; > - /* > - * set up the brk stuff, uses any slack left in data/bss/stack > - * allocation. We put the brk after the bss (between the bss > - * and stack) like other platforms. > - * Userspace code relies on the stack pointer starting out at > - * an address right at the end of a page. > - */ > - current->mm->start_brk = datapos + data_len + bss_len; > - current->mm->brk = (current->mm->start_brk + 3) & ~3; > + current->mm->start_code = start_code; > + current->mm->end_code = end_code; > + current->mm->start_data = datapos; > + current->mm->end_data = datapos + data_len; > + /* > + * set up the brk stuff, uses any slack left in data/bss/stack > + * allocation. We put the brk after the bss (between the bss > + * and stack) like other platforms. > + * Userspace code relies on the stack pointer starting out at > + * an address right at the end of a page. > + */ > + current->mm->start_brk = datapos + data_len + bss_len; > + current->mm->brk = (current->mm->start_brk + 3) & ~3; > #ifndef CONFIG_MMU > - current->mm->context.end_brk = memp + memp_size - stack_len; > + current->mm->context.end_brk = memp + memp_size - stack_len; > #endif > - } > > if (flags & FLAT_FLAG_KTRACE) { > pr_info("Mapping is %lx, Entry point is %x, data_start is %x\n", > textpos, 0x00ffffff&ntohl(hdr->entry), ntohl(hdr->data_start)); > pr_info("%s %s: TEXT=%lx-%lx DATA=%lx-%lx BSS=%lx-%lx\n", > - id ? "Lib" : "Load", bprm->filename, > + "Load", bprm->filename, > start_code, end_code, datapos, datapos + data_len, > datapos + data_len, (datapos + data_len + bss_len + 3) & ~3); > } > > /* Store the current module values into the global library structure */ > - libinfo->lib_list[id].start_code = start_code; > - libinfo->lib_list[id].start_data = datapos; > - libinfo->lib_list[id].start_brk = datapos + data_len + bss_len; > - libinfo->lib_list[id].text_len = text_len; > - libinfo->lib_list[id].loaded = 1; > - libinfo->lib_list[id].entry = (0x00ffffff & ntohl(hdr->entry)) + textpos; > - libinfo->lib_list[id].build_date = ntohl(hdr->build_date); > + libinfo->lib_list[0].start_code = start_code; > + libinfo->lib_list[0].start_data = datapos; > + libinfo->lib_list[0].start_brk = datapos + data_len + bss_len; > + libinfo->lib_list[0].text_len = text_len; > + libinfo->lib_list[0].loaded = 1; > + libinfo->lib_list[0].entry = (0x00ffffff & ntohl(hdr->entry)) + textpos; > + libinfo->lib_list[0].build_date = ntohl(hdr->build_date); > > /* > * We just load the allocations into some temporary memory to > @@ -774,7 +721,7 @@ static int load_flat_file(struct linux_binprm *bprm, > if (rp_val == 0xffffffff) > break; > if (rp_val) { > - addr = calc_reloc(rp_val, libinfo, id, 0); > + addr = calc_reloc(rp_val, libinfo); > if (addr == RELOC_FAILED) { > ret = -ENOEXEC; > goto err; > @@ -810,7 +757,7 @@ static int load_flat_file(struct linux_binprm *bprm, > return -EFAULT; > relval = ntohl(tmp); > addr = flat_get_relocate_addr(relval); > - rp = (u32 __user *)calc_reloc(addr, libinfo, id, 1); > + rp = (u32 __user *)calc_reloc(addr, libinfo); > if (rp == (u32 __user *)RELOC_FAILED) { > ret = -ENOEXEC; > goto err; > @@ -833,7 +780,7 @@ static int load_flat_file(struct linux_binprm *bprm, > */ > addr = ntohl((__force __be32)addr); > } > - addr = calc_reloc(addr, libinfo, id, 0); > + addr = calc_reloc(addr, libinfo); > if (addr == RELOC_FAILED) { > ret = -ENOEXEC; > goto err; > @@ -861,7 +808,7 @@ static int load_flat_file(struct linux_binprm *bprm, > /* zero the BSS, BRK and stack areas */ > if (clear_user((void __user *)(datapos + data_len), bss_len + > (memp + memp_size - stack_len - /* end brk */ > - libinfo->lib_list[id].start_brk) + /* start brk */ > + libinfo->lib_list[0].start_brk) + /* start brk */ > stack_len)) > return -EFAULT; > > @@ -871,49 +818,6 @@ static int load_flat_file(struct linux_binprm *bprm, > } > > > -/****************************************************************************/ > -#ifdef CONFIG_BINFMT_SHARED_FLAT > - > -/* > - * Load a shared library into memory. The library gets its own data > - * segment (including bss) but not argv/argc/environ. > - */ > - > -static int load_flat_shared_library(int id, struct lib_info *libs) > -{ > - /* > - * This is a fake bprm struct; only the members "buf", "file" and > - * "filename" are actually used. > - */ > - struct linux_binprm bprm; > - int res; > - char buf[16]; > - loff_t pos = 0; > - > - memset(&bprm, 0, sizeof(bprm)); > - > - /* Create the file name */ > - sprintf(buf, "/lib/lib%d.so", id); > - > - /* Open the file up */ > - bprm.filename = buf; > - bprm.file = open_exec(bprm.filename); > - res = PTR_ERR(bprm.file); > - if (IS_ERR(bprm.file)) > - return res; > - > - res = kernel_read(bprm.file, bprm.buf, BINPRM_BUF_SIZE, &pos); > - > - if (res >= 0) > - res = load_flat_file(&bprm, libs, id, NULL); > - > - allow_write_access(bprm.file); > - fput(bprm.file); > - > - return res; > -} > - > -#endif /* CONFIG_BINFMT_SHARED_FLAT */ > /****************************************************************************/ > > /* > @@ -946,7 +850,7 @@ static int load_flat_binary(struct linux_binprm *bprm) > stack_len += (bprm->envc + 1) * sizeof(char *); /* the envp array */ > stack_len = ALIGN(stack_len, FLAT_STACK_ALIGN); > > - res = load_flat_file(bprm, &libinfo, 0, &stack_len); > + res = load_flat_file(bprm, &libinfo, &stack_len); > if (res < 0) > return res; > > @@ -991,20 +895,6 @@ static int load_flat_binary(struct linux_binprm *bprm) > */ > start_addr = libinfo.lib_list[0].entry; > > -#ifdef CONFIG_BINFMT_SHARED_FLAT > - for (i = MAX_SHARED_LIBS-1; i > 0; i--) { > - if (libinfo.lib_list[i].loaded) { > - /* Push previos first to call address */ > - unsigned long __user *sp; > - current->mm->start_stack -= sizeof(unsigned long); > - sp = (unsigned long __user *)current->mm->start_stack; > - if (put_user(start_addr, sp)) > - return -EFAULT; > - start_addr = libinfo.lib_list[i].entry; > - } > - } > -#endif > - > #ifdef FLAT_PLAT_INIT > FLAT_PLAT_INIT(regs); > #endif
On Wed, 20 Apr 2022 09:58:03 -0500, Eric W. Biederman wrote: > In a recent discussion[1] it was reported that the binfmt_flat library > support was only ever used on m68k and even on m68k has not been used > in a very long time. > > The structure of binfmt_flat is different from all of the other binfmt > implementations becasue of this shared library support and it made > life and code review more effort when I refactored the code in fs/exec.c. > > [...] It seems like there is agreement that the shared library support is unused, so let's test that assumption. :) With typos fixed up, applied to for-next/execve, thanks! [1/1] binfmt_flat: Remove shared library support https://git.kernel.org/kees/c/c85b5d88951b
On Thu, Apr 21, 2022 at 1:53 AM Greg Ungerer <gerg@linux-m68k.org> wrote: > On 21/4/22 00:58, Eric W. Biederman wrote: > > In a recent discussion[1] it was reported that the binfmt_flat library > > support was only ever used on m68k and even on m68k has not been used > > in a very long time. > > > > The structure of binfmt_flat is different from all of the other binfmt > > implementations becasue of this shared library support and it made > > life and code review more effort when I refactored the code in fs/exec.c. > > > > Since in practice the code is dead remove the binfmt_flat shared libarary > > support and make maintenance of the code easier. > > > > [1] https://lkml.kernel.org/r/81788b56-5b15-7308-38c7-c7f2502c4e15@linux-m68k.org > > Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com> > > --- > > > > Can the binfmt_flat folks please verify that the shared library support > > really isn't used? > > I can definitely confirm I don't use it on m68k. And I don't know of > anyone that has used it in many years. > > > > Was binfmt_flat being enabled on arm and sh the mistake it looks like? I think the question was intended to be Was *binfmt_flat_shared_flat* being enabled on arm and sh the mistake it looks like? > > > > arch/arm/configs/lpc18xx_defconfig | 1 - > > arch/arm/configs/mps2_defconfig | 1 - > > arch/arm/configs/stm32_defconfig | 1 - > > arch/arm/configs/vf610m4_defconfig | 1 - > > binfmt_flat works on ARM. I use it all the time. > According to those defconfigs those are all non-MMU systems, so > having binfmt_flat enabled makes some sense there. > > > > arch/sh/configs/rsk7201_defconfig | 1 - > > arch/sh/configs/rsk7203_defconfig | 1 - > > arch/sh/configs/se7206_defconfig | 1 - > > Those are all SH2 systems if I am reading the defconfigs correctly. > SH2 is non-MMU according to the Kconfig setup. So it makes sense that > binfmt_flat is enabled on those too. I've checked git history, and CONFIG_BINFMT_SHARED_FLAT was enabled in se7206_defconfig in a non-specific defconfig update, so no further info. The other two had it enabled since their introduction, so I guess they were just based on the former. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On Thu, Apr 21, 2022 at 8:52 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote: > On Thu, Apr 21, 2022 at 1:53 AM Greg Ungerer <gerg@linux-m68k.org> wrote: > > On 21/4/22 00:58, Eric W. Biederman wrote: > > > In a recent discussion[1] it was reported that the binfmt_flat library > > > support was only ever used on m68k and even on m68k has not been used > > > in a very long time. > > > > > > The structure of binfmt_flat is different from all of the other binfmt > > > implementations becasue of this shared library support and it made > > > life and code review more effort when I refactored the code in fs/exec.c. > > > > > > Since in practice the code is dead remove the binfmt_flat shared libarary > > > support and make maintenance of the code easier. > > > > > > [1] https://lkml.kernel.org/r/81788b56-5b15-7308-38c7-c7f2502c4e15@linux-m68k.org > > > Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com> > > > --- > > > > > > Can the binfmt_flat folks please verify that the shared library support > > > really isn't used? > > > > I can definitely confirm I don't use it on m68k. And I don't know of > > anyone that has used it in many years. > > > > > > > Was binfmt_flat being enabled on arm and sh the mistake it looks like? > > I think the question was intended to be > > Was *binfmt_flat_shared_flat* being enabled on arm and sh the > mistake it looks like? > > > > > > > arch/arm/configs/lpc18xx_defconfig | 1 - > > > arch/arm/configs/mps2_defconfig | 1 - > > > arch/arm/configs/stm32_defconfig | 1 - > > > arch/arm/configs/vf610m4_defconfig | 1 - Adding stm32, mps2 and imxrt maintainers to Cc, they are the most active armv7-m users and should know if the shared library support is used anywhere. Arnd
On Thu, Apr 21, 2022 at 08:52:59AM +0200, Geert Uytterhoeven wrote: > On Thu, Apr 21, 2022 at 1:53 AM Greg Ungerer <gerg@linux-m68k.org> wrote: > > On 21/4/22 00:58, Eric W. Biederman wrote: > > > In a recent discussion[1] it was reported that the binfmt_flat library > > > support was only ever used on m68k and even on m68k has not been used > > > in a very long time. > > > > > > The structure of binfmt_flat is different from all of the other binfmt > > > implementations becasue of this shared library support and it made > > > life and code review more effort when I refactored the code in fs/exec.c. > > > > > > Since in practice the code is dead remove the binfmt_flat shared libarary > > > support and make maintenance of the code easier. > > > > > > [1] https://lkml.kernel.org/r/81788b56-5b15-7308-38c7-c7f2502c4e15@linux-m68k.org > > > Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com> > > > --- > > > > > > Can the binfmt_flat folks please verify that the shared library support > > > really isn't used? > > > > I can definitely confirm I don't use it on m68k. And I don't know of > > anyone that has used it in many years. > > > > > > > Was binfmt_flat being enabled on arm and sh the mistake it looks like? > > I think the question was intended to be > > Was *binfmt_flat_shared_flat* being enabled on arm and sh the > mistake it looks like? Early in my work on j2, I tried to research the history of shared flat support on sh, and it turned out the mainline tooling never even supported it, and the out-of-line tooling I eventually found was using all sorts of wrong conditionals for how it did the linking and elf2flt conversion, e.g. mere presence of any PIC-like relocation in any file made it assume the whole program was PIC-compatible. There's no way that stuf was ever used in any meaningful way. It just didn't work. Quickly dropped that and got plain ELF (no shared text/xip, but no worse than the existing flat support) working, and soon after, FDPIC. The whole binfmt_flat ecosystem is a mess with no good reason to exist. Rich
On 4/21/22 8:12 AM, Arnd Bergmann wrote: > On Thu, Apr 21, 2022 at 8:52 AM Geert Uytterhoeven <geert@linux-m68k.org> wrote: >> On Thu, Apr 21, 2022 at 1:53 AM Greg Ungerer <gerg@linux-m68k.org> wrote: >>> On 21/4/22 00:58, Eric W. Biederman wrote: >>>> In a recent discussion[1] it was reported that the binfmt_flat library >>>> support was only ever used on m68k and even on m68k has not been used >>>> in a very long time. >>>> >>>> The structure of binfmt_flat is different from all of the other binfmt >>>> implementations becasue of this shared library support and it made >>>> life and code review more effort when I refactored the code in fs/exec.c. >>>> >>>> Since in practice the code is dead remove the binfmt_flat shared libarary >>>> support and make maintenance of the code easier. >>>> >>>> [1] https://lkml.kernel.org/r/81788b56-5b15-7308-38c7-c7f2502c4e15@linux-m68k.org >>>> Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com> >>>> --- >>>> >>>> Can the binfmt_flat folks please verify that the shared library support >>>> really isn't used? >>> >>> I can definitely confirm I don't use it on m68k. And I don't know of >>> anyone that has used it in many years. >>> >>> >>>> Was binfmt_flat being enabled on arm and sh the mistake it looks like? >> >> I think the question was intended to be >> >> Was *binfmt_flat_shared_flat* being enabled on arm and sh the >> mistake it looks like? >> >>>> >>>> arch/arm/configs/lpc18xx_defconfig | 1 - >>>> arch/arm/configs/mps2_defconfig | 1 - >>>> arch/arm/configs/stm32_defconfig | 1 - >>>> arch/arm/configs/vf610m4_defconfig | 1 - > > Adding stm32, mps2 and imxrt maintainers to Cc, they are the most active > armv7-m users and should know if the shared library support is used anywhere. Never seen shared library in use for flat format, so FWIW Acked-by: Vladimir Murzin <vladimir.murzin@arm.com> # ARM > > Arnd
Hi Arnd
Thanks for forwarding this patch.
As far as i know, we don't need CONFIG_BINFMT_SHARED_FLAT flag.
Tested on stm32f769-disco board
You can add my :
Tested-by: Patrice Chotard <patrice.chotard@foss.st.com>
Thanks
Patrice
On 4/20/22 12:47, Kees Cook wrote: >> For what it's worth, bimfmt_flat (with or without shared library >> support) should be simple to implement as a binfmt_misc handler if >> anyone needs the old shared library support (or if kernel wanted to >> drop it entirely, which I would be in favor of). That's how I handled >> old aout binaries I wanted to run after aout was removed: trivial >> binfmt_misc loader. > > Yeah, I was trying to understand why systems were using binfmt_flat and > not binfmt_elf, given the mention of elf2flat -- is there really such a > large kernel memory footprint savings to be had from removing > binfmt_elf? elf2flat is a terrible name: it doesn't take an executable as input, it takes a .o file as input. (I mean it's an elf format .o file, but... misleading.) > But regardless, yes, it seems like if you're doing anything remotely > needing shared libraries with binfmt_flat, such a system could just use > ELF instead. A) The binfmt_elf.c loader won't run on nommu systems. The fdpic loader will, and in theory can handle normal ELF binaries (it's ELF with _more_ capabilities), but sadly it's not supported on most architectures for reasons that are unclear to me. B) You can't run conventional ELF on nommu, because everything is offset from 0 so PID 1 eats that address range and you can't run exec program. You can run PIE binaries on nommu (the symbols offset from a base pointer which can point anywhere), but they're inefficient (can't share text or rodata sections between instances because every symbol is offset from a single shared base pointer), and highly vulnerable to fragmentation (because it needs a contiguous blob of memory for text, rodata, bss, and data: see single base pointer everything has an integer offset from). All fdpic really does is give you 4 base pointers, one for each section. That way you can share text and rodata, and put bss and data into smaller independent fragments of memory. Various security guys use this as super-aslr even on mmu systems, but tend not to advertise that they're doing so. :) Rob
On 4/21/22 07:43, Rich Felker wrote: > On Thu, Apr 21, 2022 at 08:52:59AM +0200, Geert Uytterhoeven wrote: >> On Thu, Apr 21, 2022 at 1:53 AM Greg Ungerer <gerg@linux-m68k.org> wrote: >> > On 21/4/22 00:58, Eric W. Biederman wrote: >> > > In a recent discussion[1] it was reported that the binfmt_flat library >> > > support was only ever used on m68k and even on m68k has not been used >> > > in a very long time. >> > > >> > > The structure of binfmt_flat is different from all of the other binfmt >> > > implementations becasue of this shared library support and it made >> > > life and code review more effort when I refactored the code in fs/exec.c. >> > > >> > > Since in practice the code is dead remove the binfmt_flat shared libarary >> > > support and make maintenance of the code easier. >> > > >> > > [1] https://lkml.kernel.org/r/81788b56-5b15-7308-38c7-c7f2502c4e15@linux-m68k.org >> > > Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com> >> > > --- >> > > >> > > Can the binfmt_flat folks please verify that the shared library support >> > > really isn't used? >> > >> > I can definitely confirm I don't use it on m68k. And I don't know of >> > anyone that has used it in many years. >> > >> > >> > > Was binfmt_flat being enabled on arm and sh the mistake it looks like? >> >> I think the question was intended to be >> >> Was *binfmt_flat_shared_flat* being enabled on arm and sh the >> mistake it looks like? > > Early in my work on j2, I tried to research the history of shared flat > support on sh, and it turned out the mainline tooling never even > supported it, and the out-of-line tooling I eventually found was using > all sorts of wrong conditionals for how it did the linking and elf2flt > conversion, e.g. mere presence of any PIC-like relocation in any file > made it assume the whole program was PIC-compatible. There's no way > that stuf was ever used in any meaningful way. It just didn't work. > > Quickly dropped that and got plain ELF (no shared text/xip, but no > worse than the existing flat support) working, and soon after, FDPIC. > > The whole binfmt_flat ecosystem is a mess with no good reason to > exist. FYI when I had to come up to speed on this in 2014 I did a writeup on my own research: https://landley.net/notes-2014.html#07-12-2014 The lack of a canonical "upstream" elf2flt repository was probably the biggest problem at the time. (There's a reason I grabbed fdpic hard and tried to make that work everywhere.) > Rich Rob
On 25/4/22 13:38, Rob Landley wrote: > On 4/20/22 12:47, Kees Cook wrote: >>> For what it's worth, bimfmt_flat (with or without shared library >>> support) should be simple to implement as a binfmt_misc handler if >>> anyone needs the old shared library support (or if kernel wanted to >>> drop it entirely, which I would be in favor of). That's how I handled >>> old aout binaries I wanted to run after aout was removed: trivial >>> binfmt_misc loader. >> >> Yeah, I was trying to understand why systems were using binfmt_flat and >> not binfmt_elf, given the mention of elf2flat -- is there really such a >> large kernel memory footprint savings to be had from removing >> binfmt_elf? > > elf2flat is a terrible name: it doesn't take an executable as input, it takes a > .o file as input. (I mean it's an elf format .o file, but... misleading.) No, not at all. "elf2flt" is exactly what it does. Couldn't get a more accurate name. >> But regardless, yes, it seems like if you're doing anything remotely >> needing shared libraries with binfmt_flat, such a system could just use >> ELF instead. > > A) The binfmt_elf.c loader won't run on nommu systems. The fdpic loader will, > and in theory can handle normal ELF binaries (it's ELF with _more_ > capabilities), but sadly it's not supported on most architectures for reasons > that are unclear to me. Inertia. Flat format has been around a very long time. And for most people it just works. Flat format works on MMU systems as well, though you would have to be crazy to choose to do that. > B) You can't run conventional ELF on nommu, because everything is offset from 0 > so PID 1 eats that address range and you can't run exec program. > > You can run PIE binaries on nommu (the symbols offset from a base pointer which > can point anywhere), but they're inefficient (can't share text or rodata > sections between instances because every symbol is offset from a single shared > base pointer), and highly vulnerable to fragmentation (because it needs a > contiguous blob of memory for text, rodata, bss, and data: see single base > pointer everything has an integer offset from). > > All fdpic really does is give you 4 base pointers, one for each section. That > way you can share text and rodata, and put bss and data into smaller independent > fragments of memory. Various security guys use this as super-aslr even on mmu > systems, but tend not to advertise that they're doing so. :) Well flat got half way there. You can have separate text/rodata and data/bss, used a lot back in the day for execute-in-place of the code. Regards Greg
diff --git a/arch/arm/configs/lpc18xx_defconfig b/arch/arm/configs/lpc18xx_defconfig index be882ea0eee4..688c9849eec8 100644 --- a/arch/arm/configs/lpc18xx_defconfig +++ b/arch/arm/configs/lpc18xx_defconfig @@ -30,7 +30,6 @@ CONFIG_ARM_APPENDED_DTB=y # CONFIG_BLK_DEV_BSG is not set CONFIG_BINFMT_FLAT=y CONFIG_BINFMT_ZFLAT=y -CONFIG_BINFMT_SHARED_FLAT=y # CONFIG_COREDUMP is not set CONFIG_NET=y CONFIG_PACKET=y diff --git a/arch/arm/configs/mps2_defconfig b/arch/arm/configs/mps2_defconfig index 89f4a6ff30bd..c1e98e33a348 100644 --- a/arch/arm/configs/mps2_defconfig +++ b/arch/arm/configs/mps2_defconfig @@ -23,7 +23,6 @@ CONFIG_PREEMPT_VOLUNTARY=y CONFIG_ZBOOT_ROM_TEXT=0x0 CONFIG_ZBOOT_ROM_BSS=0x0 CONFIG_BINFMT_FLAT=y -CONFIG_BINFMT_SHARED_FLAT=y # CONFIG_COREDUMP is not set # CONFIG_SUSPEND is not set CONFIG_NET=y diff --git a/arch/arm/configs/stm32_defconfig b/arch/arm/configs/stm32_defconfig index 551db328009d..71d6bfcf4551 100644 --- a/arch/arm/configs/stm32_defconfig +++ b/arch/arm/configs/stm32_defconfig @@ -28,7 +28,6 @@ CONFIG_ZBOOT_ROM_BSS=0x0 CONFIG_XIP_KERNEL=y CONFIG_XIP_PHYS_ADDR=0x08008000 CONFIG_BINFMT_FLAT=y -CONFIG_BINFMT_SHARED_FLAT=y # CONFIG_COREDUMP is not set CONFIG_DEVTMPFS=y CONFIG_DEVTMPFS_MOUNT=y diff --git a/arch/arm/configs/vf610m4_defconfig b/arch/arm/configs/vf610m4_defconfig index a89f035c3b01..70fdbfd83484 100644 --- a/arch/arm/configs/vf610m4_defconfig +++ b/arch/arm/configs/vf610m4_defconfig @@ -18,7 +18,6 @@ CONFIG_XIP_KERNEL=y CONFIG_XIP_PHYS_ADDR=0x0f000080 CONFIG_BINFMT_FLAT=y CONFIG_BINFMT_ZFLAT=y -CONFIG_BINFMT_SHARED_FLAT=y # CONFIG_SUSPEND is not set # CONFIG_UEVENT_HELPER is not set # CONFIG_STANDALONE is not set diff --git a/arch/sh/configs/rsk7201_defconfig b/arch/sh/configs/rsk7201_defconfig index e41526120be1..619c18699459 100644 --- a/arch/sh/configs/rsk7201_defconfig +++ b/arch/sh/configs/rsk7201_defconfig @@ -25,7 +25,6 @@ CONFIG_CMDLINE_OVERWRITE=y CONFIG_CMDLINE="console=ttySC0,115200 earlyprintk=serial ignore_loglevel" CONFIG_BINFMT_FLAT=y CONFIG_BINFMT_ZFLAT=y -CONFIG_BINFMT_SHARED_FLAT=y CONFIG_PM=y CONFIG_CPU_IDLE=y # CONFIG_STANDALONE is not set diff --git a/arch/sh/configs/rsk7203_defconfig b/arch/sh/configs/rsk7203_defconfig index 6af08fa1ddf8..5a54e2b883f0 100644 --- a/arch/sh/configs/rsk7203_defconfig +++ b/arch/sh/configs/rsk7203_defconfig @@ -30,7 +30,6 @@ CONFIG_CMDLINE_OVERWRITE=y CONFIG_CMDLINE="console=ttySC0,115200 earlyprintk=serial ignore_loglevel" CONFIG_BINFMT_FLAT=y CONFIG_BINFMT_ZFLAT=y -CONFIG_BINFMT_SHARED_FLAT=y CONFIG_PM=y CONFIG_CPU_IDLE=y CONFIG_NET=y diff --git a/arch/sh/configs/se7206_defconfig b/arch/sh/configs/se7206_defconfig index 601d062250d1..122216123e63 100644 --- a/arch/sh/configs/se7206_defconfig +++ b/arch/sh/configs/se7206_defconfig @@ -40,7 +40,6 @@ CONFIG_CMDLINE_OVERWRITE=y CONFIG_CMDLINE="console=ttySC3,115200 ignore_loglevel earlyprintk=serial" CONFIG_BINFMT_FLAT=y CONFIG_BINFMT_ZFLAT=y -CONFIG_BINFMT_SHARED_FLAT=y CONFIG_BINFMT_MISC=y CONFIG_NET=y CONFIG_PACKET=y diff --git a/fs/Kconfig.binfmt b/fs/Kconfig.binfmt index 21c6332fa785..32dff7ba3dda 100644 --- a/fs/Kconfig.binfmt +++ b/fs/Kconfig.binfmt @@ -142,12 +142,6 @@ config BINFMT_ZFLAT help Support FLAT format compressed binaries -config BINFMT_SHARED_FLAT - bool "Enable shared FLAT support" - depends on BINFMT_FLAT - help - Support FLAT shared libraries - config HAVE_AOUT def_bool n diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c index 0ad2c7bbaddd..82e4412a9665 100644 --- a/fs/binfmt_flat.c +++ b/fs/binfmt_flat.c @@ -68,11 +68,7 @@ #define RELOC_FAILED 0xff00ff01 /* Relocation incorrect somewhere */ #define UNLOADED_LIB 0x7ff000ff /* Placeholder for unused library */ -#ifdef CONFIG_BINFMT_SHARED_FLAT -#define MAX_SHARED_LIBS (4) -#else -#define MAX_SHARED_LIBS (1) -#endif +#define MAX_SHARED_LIBS (1) #ifdef CONFIG_BINFMT_FLAT_NO_DATA_START_OFFSET #define DATA_START_OFFSET_WORDS (0) @@ -92,10 +88,6 @@ struct lib_info { } lib_list[MAX_SHARED_LIBS]; }; -#ifdef CONFIG_BINFMT_SHARED_FLAT -static int load_flat_shared_library(int id, struct lib_info *p); -#endif - static int load_flat_binary(struct linux_binprm *); static struct linux_binfmt flat_format = { @@ -307,51 +299,18 @@ static int decompress_exec(struct linux_binprm *bprm, loff_t fpos, char *dst, /****************************************************************************/ static unsigned long -calc_reloc(unsigned long r, struct lib_info *p, int curid, int internalp) +calc_reloc(unsigned long r, struct lib_info *p) { unsigned long addr; - int id; unsigned long start_brk; unsigned long start_data; unsigned long text_len; unsigned long start_code; -#ifdef CONFIG_BINFMT_SHARED_FLAT - if (r == 0) - id = curid; /* Relocs of 0 are always self referring */ - else { - id = (r >> 24) & 0xff; /* Find ID for this reloc */ - r &= 0x00ffffff; /* Trim ID off here */ - } - if (id >= MAX_SHARED_LIBS) { - pr_err("reference 0x%lx to shared library %d", r, id); - goto failed; - } - if (curid != id) { - if (internalp) { - pr_err("reloc address 0x%lx not in same module " - "(%d != %d)", r, curid, id); - goto failed; - } else if (!p->lib_list[id].loaded && - load_flat_shared_library(id, p) < 0) { - pr_err("failed to load library %d", id); - goto failed; - } - /* Check versioning information (i.e. time stamps) */ - if (p->lib_list[id].build_date && p->lib_list[curid].build_date && - p->lib_list[curid].build_date < p->lib_list[id].build_date) { - pr_err("library %d is younger than %d", id, curid); - goto failed; - } - } -#else - id = 0; -#endif - - start_brk = p->lib_list[id].start_brk; - start_data = p->lib_list[id].start_data; - start_code = p->lib_list[id].start_code; - text_len = p->lib_list[id].text_len; + start_brk = p->lib_list[0].start_brk; + start_data = p->lib_list[0].start_data; + start_code = p->lib_list[0].start_code; + text_len = p->lib_list[0].text_len; if (r > start_brk - start_data + text_len) { pr_err("reloc outside program 0x%lx (0 - 0x%lx/0x%lx)", @@ -419,7 +378,7 @@ static void old_reloc(unsigned long rl) /****************************************************************************/ static int load_flat_file(struct linux_binprm *bprm, - struct lib_info *libinfo, int id, unsigned long *extra_stack) + struct lib_info *libinfo, unsigned long *extra_stack) { struct flat_hdr *hdr; unsigned long textpos, datapos, realdatastart; @@ -471,14 +430,6 @@ static int load_flat_file(struct linux_binprm *bprm, goto err; } - /* Don't allow old format executables to use shared libraries */ - if (rev == OLD_FLAT_VERSION && id != 0) { - pr_err("shared libraries are not available before rev 0x%lx\n", - FLAT_VERSION); - ret = -ENOEXEC; - goto err; - } - /* * fix up the flags for the older format, there were all kinds * of endian hacks, this only works for the simple cases @@ -529,15 +480,13 @@ static int load_flat_file(struct linux_binprm *bprm, } /* Flush all traces of the currently running executable */ - if (id == 0) { - ret = begin_new_exec(bprm); - if (ret) - goto err; + ret = begin_new_exec(bprm); + if (ret) + goto err; - /* OK, This is the point of no return */ - set_personality(PER_LINUX_32BIT); - setup_new_exec(bprm); - } + /* OK, This is the point of no return */ + set_personality(PER_LINUX_32BIT); + setup_new_exec(bprm); /* * calculate the extra space we need to map in @@ -717,42 +666,40 @@ static int load_flat_file(struct linux_binprm *bprm, text_len -= sizeof(struct flat_hdr); /* the real code len */ /* The main program needs a little extra setup in the task structure */ - if (id == 0) { - current->mm->start_code = start_code; - current->mm->end_code = end_code; - current->mm->start_data = datapos; - current->mm->end_data = datapos + data_len; - /* - * set up the brk stuff, uses any slack left in data/bss/stack - * allocation. We put the brk after the bss (between the bss - * and stack) like other platforms. - * Userspace code relies on the stack pointer starting out at - * an address right at the end of a page. - */ - current->mm->start_brk = datapos + data_len + bss_len; - current->mm->brk = (current->mm->start_brk + 3) & ~3; + current->mm->start_code = start_code; + current->mm->end_code = end_code; + current->mm->start_data = datapos; + current->mm->end_data = datapos + data_len; + /* + * set up the brk stuff, uses any slack left in data/bss/stack + * allocation. We put the brk after the bss (between the bss + * and stack) like other platforms. + * Userspace code relies on the stack pointer starting out at + * an address right at the end of a page. + */ + current->mm->start_brk = datapos + data_len + bss_len; + current->mm->brk = (current->mm->start_brk + 3) & ~3; #ifndef CONFIG_MMU - current->mm->context.end_brk = memp + memp_size - stack_len; + current->mm->context.end_brk = memp + memp_size - stack_len; #endif - } if (flags & FLAT_FLAG_KTRACE) { pr_info("Mapping is %lx, Entry point is %x, data_start is %x\n", textpos, 0x00ffffff&ntohl(hdr->entry), ntohl(hdr->data_start)); pr_info("%s %s: TEXT=%lx-%lx DATA=%lx-%lx BSS=%lx-%lx\n", - id ? "Lib" : "Load", bprm->filename, + "Load", bprm->filename, start_code, end_code, datapos, datapos + data_len, datapos + data_len, (datapos + data_len + bss_len + 3) & ~3); } /* Store the current module values into the global library structure */ - libinfo->lib_list[id].start_code = start_code; - libinfo->lib_list[id].start_data = datapos; - libinfo->lib_list[id].start_brk = datapos + data_len + bss_len; - libinfo->lib_list[id].text_len = text_len; - libinfo->lib_list[id].loaded = 1; - libinfo->lib_list[id].entry = (0x00ffffff & ntohl(hdr->entry)) + textpos; - libinfo->lib_list[id].build_date = ntohl(hdr->build_date); + libinfo->lib_list[0].start_code = start_code; + libinfo->lib_list[0].start_data = datapos; + libinfo->lib_list[0].start_brk = datapos + data_len + bss_len; + libinfo->lib_list[0].text_len = text_len; + libinfo->lib_list[0].loaded = 1; + libinfo->lib_list[0].entry = (0x00ffffff & ntohl(hdr->entry)) + textpos; + libinfo->lib_list[0].build_date = ntohl(hdr->build_date); /* * We just load the allocations into some temporary memory to @@ -774,7 +721,7 @@ static int load_flat_file(struct linux_binprm *bprm, if (rp_val == 0xffffffff) break; if (rp_val) { - addr = calc_reloc(rp_val, libinfo, id, 0); + addr = calc_reloc(rp_val, libinfo); if (addr == RELOC_FAILED) { ret = -ENOEXEC; goto err; @@ -810,7 +757,7 @@ static int load_flat_file(struct linux_binprm *bprm, return -EFAULT; relval = ntohl(tmp); addr = flat_get_relocate_addr(relval); - rp = (u32 __user *)calc_reloc(addr, libinfo, id, 1); + rp = (u32 __user *)calc_reloc(addr, libinfo); if (rp == (u32 __user *)RELOC_FAILED) { ret = -ENOEXEC; goto err; @@ -833,7 +780,7 @@ static int load_flat_file(struct linux_binprm *bprm, */ addr = ntohl((__force __be32)addr); } - addr = calc_reloc(addr, libinfo, id, 0); + addr = calc_reloc(addr, libinfo); if (addr == RELOC_FAILED) { ret = -ENOEXEC; goto err; @@ -861,7 +808,7 @@ static int load_flat_file(struct linux_binprm *bprm, /* zero the BSS, BRK and stack areas */ if (clear_user((void __user *)(datapos + data_len), bss_len + (memp + memp_size - stack_len - /* end brk */ - libinfo->lib_list[id].start_brk) + /* start brk */ + libinfo->lib_list[0].start_brk) + /* start brk */ stack_len)) return -EFAULT; @@ -871,49 +818,6 @@ static int load_flat_file(struct linux_binprm *bprm, } -/****************************************************************************/ -#ifdef CONFIG_BINFMT_SHARED_FLAT - -/* - * Load a shared library into memory. The library gets its own data - * segment (including bss) but not argv/argc/environ. - */ - -static int load_flat_shared_library(int id, struct lib_info *libs) -{ - /* - * This is a fake bprm struct; only the members "buf", "file" and - * "filename" are actually used. - */ - struct linux_binprm bprm; - int res; - char buf[16]; - loff_t pos = 0; - - memset(&bprm, 0, sizeof(bprm)); - - /* Create the file name */ - sprintf(buf, "/lib/lib%d.so", id); - - /* Open the file up */ - bprm.filename = buf; - bprm.file = open_exec(bprm.filename); - res = PTR_ERR(bprm.file); - if (IS_ERR(bprm.file)) - return res; - - res = kernel_read(bprm.file, bprm.buf, BINPRM_BUF_SIZE, &pos); - - if (res >= 0) - res = load_flat_file(&bprm, libs, id, NULL); - - allow_write_access(bprm.file); - fput(bprm.file); - - return res; -} - -#endif /* CONFIG_BINFMT_SHARED_FLAT */ /****************************************************************************/ /* @@ -946,7 +850,7 @@ static int load_flat_binary(struct linux_binprm *bprm) stack_len += (bprm->envc + 1) * sizeof(char *); /* the envp array */ stack_len = ALIGN(stack_len, FLAT_STACK_ALIGN); - res = load_flat_file(bprm, &libinfo, 0, &stack_len); + res = load_flat_file(bprm, &libinfo, &stack_len); if (res < 0) return res; @@ -991,20 +895,6 @@ static int load_flat_binary(struct linux_binprm *bprm) */ start_addr = libinfo.lib_list[0].entry; -#ifdef CONFIG_BINFMT_SHARED_FLAT - for (i = MAX_SHARED_LIBS-1; i > 0; i--) { - if (libinfo.lib_list[i].loaded) { - /* Push previos first to call address */ - unsigned long __user *sp; - current->mm->start_stack -= sizeof(unsigned long); - sp = (unsigned long __user *)current->mm->start_stack; - if (put_user(start_addr, sp)) - return -EFAULT; - start_addr = libinfo.lib_list[i].entry; - } - } -#endif - #ifdef FLAT_PLAT_INIT FLAT_PLAT_INIT(regs); #endif
In a recent discussion[1] it was reported that the binfmt_flat library support was only ever used on m68k and even on m68k has not been used in a very long time. The structure of binfmt_flat is different from all of the other binfmt implementations becasue of this shared library support and it made life and code review more effort when I refactored the code in fs/exec.c. Since in practice the code is dead remove the binfmt_flat shared libarary support and make maintenance of the code easier. [1] https://lkml.kernel.org/r/81788b56-5b15-7308-38c7-c7f2502c4e15@linux-m68k.org Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com> --- Can the binfmt_flat folks please verify that the shared library support really isn't used? Was binfmt_flat being enabled on arm and sh the mistake it looks like? arch/arm/configs/lpc18xx_defconfig | 1 - arch/arm/configs/mps2_defconfig | 1 - arch/arm/configs/stm32_defconfig | 1 - arch/arm/configs/vf610m4_defconfig | 1 - arch/sh/configs/rsk7201_defconfig | 1 - arch/sh/configs/rsk7203_defconfig | 1 - arch/sh/configs/se7206_defconfig | 1 - fs/Kconfig.binfmt | 6 - fs/binfmt_flat.c | 190 ++++++----------------------- 9 files changed, 40 insertions(+), 163 deletions(-)