* Re: [PATCH] binfmt_flat: Remove shared library support
2022-04-20 14:58 ` [PATCH] binfmt_flat: Remove shared library support Eric W. Biederman
@ 2022-04-20 16:17 ` Palmer Dabbelt
2022-04-20 16:59 ` Rich Felker
2022-04-20 23:36 ` Damien Le Moal
` (2 subsequent siblings)
3 siblings, 1 reply; 17+ messages in thread
From: Palmer Dabbelt @ 2022-04-20 16:17 UTC (permalink / raw)
To: ebiederm, damien.lemoal
Cc: keescook, Niklas.Cassel, viro, Paul Walmsley, aou, vapier, stable,
linux-fsdevel, linux-mm, linux-riscv, linux-arch, geert,
linux-m68k, gerg, linux-arm-kernel, linux-sh, ysato, dalias
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
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH] binfmt_flat: Remove shared library support
2022-04-20 16:17 ` Palmer Dabbelt
@ 2022-04-20 16:59 ` Rich Felker
2022-04-20 17:47 ` Kees Cook
0 siblings, 1 reply; 17+ messages in thread
From: Rich Felker @ 2022-04-20 16:59 UTC (permalink / raw)
To: Palmer Dabbelt
Cc: ebiederm, damien.lemoal, keescook, Niklas.Cassel, viro,
Paul Walmsley, aou, vapier, stable, linux-fsdevel, linux-mm,
linux-riscv, linux-arch, geert, linux-m68k, gerg,
linux-arm-kernel, linux-sh, ysato
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] binfmt_flat: Remove shared library support
2022-04-20 16:59 ` Rich Felker
@ 2022-04-20 17:47 ` Kees Cook
2022-04-20 20:04 ` Arnd Bergmann
2022-04-25 3:38 ` Rob Landley
0 siblings, 2 replies; 17+ messages in thread
From: Kees Cook @ 2022-04-20 17:47 UTC (permalink / raw)
To: Rich Felker
Cc: Palmer Dabbelt, ebiederm, damien.lemoal, Niklas.Cassel, viro,
Paul Walmsley, aou, vapier, stable, linux-fsdevel, linux-mm,
linux-riscv, linux-arch, geert, linux-m68k, gerg,
linux-arm-kernel, linux-sh, ysato
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.
--
Kees Cook
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] binfmt_flat: Remove shared library support
2022-04-20 17:47 ` Kees Cook
@ 2022-04-20 20:04 ` Arnd Bergmann
2022-04-20 20:23 ` Rich Felker
2022-04-25 3:38 ` Rob Landley
1 sibling, 1 reply; 17+ messages in thread
From: Arnd Bergmann @ 2022-04-20 20:04 UTC (permalink / raw)
To: Kees Cook
Cc: Rich Felker, Palmer Dabbelt, Eric W . Biederman, Damien Le Moal,
Niklas.Cassel, Al Viro, Paul Walmsley, Albert Ou, Mike Frysinger,
# 3.4.x, Linux FS-devel Mailing List, Linux-MM, linux-riscv,
linux-arch, Geert Uytterhoeven, linux-m68k, Greg Ungerer,
Linux ARM, Linux-sh list, Yoshinori Sato
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
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH] binfmt_flat: Remove shared library support
2022-04-20 20:04 ` Arnd Bergmann
@ 2022-04-20 20:23 ` Rich Felker
2022-04-20 23:00 ` Damien Le Moal
0 siblings, 1 reply; 17+ messages in thread
From: Rich Felker @ 2022-04-20 20:23 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Kees Cook, Palmer Dabbelt, Eric W . Biederman, Damien Le Moal,
Niklas.Cassel, Al Viro, Paul Walmsley, Albert Ou, Mike Frysinger,
# 3.4.x, Linux FS-devel Mailing List, Linux-MM, linux-riscv,
linux-arch, Geert Uytterhoeven, linux-m68k, Greg Ungerer,
Linux ARM, Linux-sh list, Yoshinori Sato
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] binfmt_flat: Remove shared library support
2022-04-20 20:23 ` Rich Felker
@ 2022-04-20 23:00 ` Damien Le Moal
0 siblings, 0 replies; 17+ messages in thread
From: Damien Le Moal @ 2022-04-20 23:00 UTC (permalink / raw)
To: Rich Felker, Arnd Bergmann
Cc: Kees Cook, Palmer Dabbelt, Eric W . Biederman, Niklas.Cassel,
Al Viro, Paul Walmsley, Albert Ou, Mike Frysinger, # 3.4.x,
Linux FS-devel Mailing List, Linux-MM, linux-riscv, linux-arch,
Geert Uytterhoeven, linux-m68k, Greg Ungerer, Linux ARM,
Linux-sh list, Yoshinori Sato
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.
--
Damien Le Moal
Western Digital Research
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] binfmt_flat: Remove shared library support
2022-04-20 17:47 ` Kees Cook
2022-04-20 20:04 ` Arnd Bergmann
@ 2022-04-25 3:38 ` Rob Landley
2022-04-25 7:40 ` Greg Ungerer
1 sibling, 1 reply; 17+ messages in thread
From: Rob Landley @ 2022-04-25 3:38 UTC (permalink / raw)
To: Kees Cook, Rich Felker
Cc: Palmer Dabbelt, ebiederm, damien.lemoal, Niklas.Cassel, viro,
Paul Walmsley, aou, vapier, stable, linux-fsdevel, linux-mm,
linux-riscv, linux-arch, geert, linux-m68k, gerg,
linux-arm-kernel, linux-sh, ysato
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] binfmt_flat: Remove shared library support
2022-04-25 3:38 ` Rob Landley
@ 2022-04-25 7:40 ` Greg Ungerer
0 siblings, 0 replies; 17+ messages in thread
From: Greg Ungerer @ 2022-04-25 7:40 UTC (permalink / raw)
To: Rob Landley, Kees Cook, Rich Felker
Cc: Palmer Dabbelt, ebiederm, damien.lemoal, Niklas.Cassel, viro,
Paul Walmsley, aou, vapier, stable, linux-fsdevel, linux-mm,
linux-riscv, linux-arch, geert, linux-m68k, linux-arm-kernel,
linux-sh, ysato
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] binfmt_flat: Remove shared library support
2022-04-20 14:58 ` [PATCH] binfmt_flat: Remove shared library support Eric W. Biederman
2022-04-20 16:17 ` Palmer Dabbelt
@ 2022-04-20 23:36 ` Damien Le Moal
2022-04-20 23:53 ` Greg Ungerer
2022-04-21 0:05 ` (subset) " Kees Cook
3 siblings, 0 replies; 17+ messages in thread
From: Damien Le Moal @ 2022-04-20 23:36 UTC (permalink / raw)
To: Eric W. Biederman, Kees Cook
Cc: Niklas Cassel, Alexander Viro, Paul Walmsley, Palmer Dabbelt,
Albert Ou, Mike Frysinger, stable@vger.kernel.org,
linux-fsdevel@vger.kernel.org, linux-mm@kvack.org,
linux-riscv@lists.infradead.org, linux-arch, Geert Uytterhoeven,
linux-m68k, Greg Ungerer, linux-arm-kernel, linux-sh,
Yoshinori Sato, Rich Felker
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>
--
Damien Le Moal
Western Digital Research
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH] binfmt_flat: Remove shared library support
2022-04-20 14:58 ` [PATCH] binfmt_flat: Remove shared library support Eric W. Biederman
2022-04-20 16:17 ` Palmer Dabbelt
2022-04-20 23:36 ` Damien Le Moal
@ 2022-04-20 23:53 ` Greg Ungerer
2022-04-21 6:52 ` Geert Uytterhoeven
2022-04-21 0:05 ` (subset) " Kees Cook
3 siblings, 1 reply; 17+ messages in thread
From: Greg Ungerer @ 2022-04-20 23:53 UTC (permalink / raw)
To: Eric W. Biederman, Kees Cook
Cc: Niklas Cassel, Damien Le Moal, Alexander Viro, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Mike Frysinger, stable@vger.kernel.org,
linux-fsdevel@vger.kernel.org, linux-mm@kvack.org,
linux-riscv@lists.infradead.org, linux-arch, Geert Uytterhoeven,
linux-m68k, linux-arm-kernel, linux-sh, Yoshinori Sato,
Rich Felker
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
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH] binfmt_flat: Remove shared library support
2022-04-20 23:53 ` Greg Ungerer
@ 2022-04-21 6:52 ` Geert Uytterhoeven
2022-04-21 7:12 ` Arnd Bergmann
2022-04-21 12:43 ` Rich Felker
0 siblings, 2 replies; 17+ messages in thread
From: Geert Uytterhoeven @ 2022-04-21 6:52 UTC (permalink / raw)
To: Greg Ungerer
Cc: Eric W. Biederman, Kees Cook, Niklas Cassel, Damien Le Moal,
Alexander Viro, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Mike Frysinger, stable@vger.kernel.org,
linux-fsdevel@vger.kernel.org, linux-mm@kvack.org,
linux-riscv@lists.infradead.org, Linux-Arch, linux-m68k,
Linux ARM, Linux-sh list, Yoshinori Sato, Rich Felker
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
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH] binfmt_flat: Remove shared library support
2022-04-21 6:52 ` Geert Uytterhoeven
@ 2022-04-21 7:12 ` Arnd Bergmann
2022-04-22 10:26 ` Vladimir Murzin
2022-04-21 12:43 ` Rich Felker
1 sibling, 1 reply; 17+ messages in thread
From: Arnd Bergmann @ 2022-04-21 7:12 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Greg Ungerer, Eric W. Biederman, Kees Cook, Niklas Cassel,
Damien Le Moal, Alexander Viro, Paul Walmsley, Palmer Dabbelt,
Albert Ou, Mike Frysinger, stable@vger.kernel.org,
linux-fsdevel@vger.kernel.org, linux-mm@kvack.org,
linux-riscv@lists.infradead.org, Linux-Arch, linux-m68k,
Linux ARM, Linux-sh list, Yoshinori Sato, Rich Felker,
Sudeep Holla, Vladimir Murzin, Jesse Taube, Giulio Benetti,
Patrice Chotard, Pierre-Yves MORDRET, Alexandre Torgue
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
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH] binfmt_flat: Remove shared library support
2022-04-21 7:12 ` Arnd Bergmann
@ 2022-04-22 10:26 ` Vladimir Murzin
0 siblings, 0 replies; 17+ messages in thread
From: Vladimir Murzin @ 2022-04-22 10:26 UTC (permalink / raw)
To: Arnd Bergmann, Geert Uytterhoeven
Cc: Greg Ungerer, Eric W. Biederman, Kees Cook, Niklas Cassel,
Damien Le Moal, Alexander Viro, Paul Walmsley, Palmer Dabbelt,
Albert Ou, Mike Frysinger, stable@vger.kernel.org,
linux-fsdevel@vger.kernel.org, linux-mm@kvack.org,
linux-riscv@lists.infradead.org, Linux-Arch, linux-m68k,
Linux ARM, Linux-sh list, Yoshinori Sato, Rich Felker,
Sudeep Holla, Jesse Taube, Giulio Benetti, Patrice Chotard,
Pierre-Yves MORDRET, Alexandre Torgue
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] binfmt_flat: Remove shared library support
2022-04-21 6:52 ` Geert Uytterhoeven
2022-04-21 7:12 ` Arnd Bergmann
@ 2022-04-21 12:43 ` Rich Felker
2022-04-25 3:50 ` Rob Landley
1 sibling, 1 reply; 17+ messages in thread
From: Rich Felker @ 2022-04-21 12:43 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Greg Ungerer, Eric W. Biederman, Kees Cook, Niklas Cassel,
Damien Le Moal, Alexander Viro, Paul Walmsley, Palmer Dabbelt,
Albert Ou, Mike Frysinger, stable@vger.kernel.org,
linux-fsdevel@vger.kernel.org, linux-mm@kvack.org,
linux-riscv@lists.infradead.org, Linux-Arch, linux-m68k,
Linux ARM, Linux-sh list, Yoshinori Sato
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH] binfmt_flat: Remove shared library support
2022-04-21 12:43 ` Rich Felker
@ 2022-04-25 3:50 ` Rob Landley
0 siblings, 0 replies; 17+ messages in thread
From: Rob Landley @ 2022-04-25 3:50 UTC (permalink / raw)
To: Rich Felker, Geert Uytterhoeven
Cc: Greg Ungerer, Eric W. Biederman, Kees Cook, Niklas Cassel,
Damien Le Moal, Alexander Viro, Paul Walmsley, Palmer Dabbelt,
Albert Ou, Mike Frysinger, stable@vger.kernel.org,
linux-fsdevel@vger.kernel.org, linux-mm@kvack.org,
linux-riscv@lists.infradead.org, Linux-Arch, linux-m68k,
Linux ARM, Linux-sh list, Yoshinori Sato
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
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: (subset) [PATCH] binfmt_flat: Remove shared library support
2022-04-20 14:58 ` [PATCH] binfmt_flat: Remove shared library support Eric W. Biederman
` (2 preceding siblings ...)
2022-04-20 23:53 ` Greg Ungerer
@ 2022-04-21 0:05 ` Kees Cook
3 siblings, 0 replies; 17+ messages in thread
From: Kees Cook @ 2022-04-21 0:05 UTC (permalink / raw)
To: ebiederm
Cc: Kees Cook, linux-arm-kernel, dalias, aou, gerg, geert, vapier,
palmer, damien.lemoal, linux-m68k, linux-riscv, Al Viro, stable,
linux-fsdevel, paul.walmsley, ysato, Niklas.Cassel, linux-sh,
linux-mm, linux-arch
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
--
Kees Cook
^ permalink raw reply [flat|nested] 17+ messages in thread