[PATCH] binfmt_flat: Remove shared library support

Greg Ungerer gerg at linux-m68k.org
Wed Apr 20 16:53:38 PDT 2022


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 at 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



More information about the linux-riscv mailing list