diff mbox series

binfmt_flat: Remove shared library support

Message ID 87levzzts4.fsf_-_@email.froward.int.ebiederm.org (mailing list archive)
State New, archived
Headers show
Series binfmt_flat: Remove shared library support | expand

Commit Message

Eric W. Biederman April 20, 2022, 2:58 p.m. UTC
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(-)

Comments

Palmer Dabbelt April 20, 2022, 4:17 p.m. UTC | #1
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
Rich Felker April 20, 2022, 4:59 p.m. UTC | #2
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
Kees Cook April 20, 2022, 5:47 p.m. UTC | #3
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.
Arnd Bergmann April 20, 2022, 8:04 p.m. UTC | #4
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
Rich Felker April 20, 2022, 8:23 p.m. UTC | #5
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
Damien Le Moal April 20, 2022, 11 p.m. UTC | #6
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 April 20, 2022, 11:36 p.m. UTC | #7
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>
Greg Ungerer April 20, 2022, 11:53 p.m. UTC | #8
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
Kees Cook April 21, 2022, 12:05 a.m. UTC | #9
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
Geert Uytterhoeven April 21, 2022, 6:52 a.m. UTC | #10
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
Arnd Bergmann April 21, 2022, 7:12 a.m. UTC | #11
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
Rich Felker April 21, 2022, 12:43 p.m. UTC | #12
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
Vladimir Murzin April 22, 2022, 10:26 a.m. UTC | #13
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
Rob Landley April 25, 2022, 3:38 a.m. UTC | #14
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
Rob Landley April 25, 2022, 3:50 a.m. UTC | #15
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
Greg Ungerer April 25, 2022, 7:40 a.m. UTC | #16
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 mbox series

Patch

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