2009-10-19 14:39:52

by Ryan C. Gordon

[permalink] [raw]
Subject: [RFC][PATCH 1/2] binfmt_elf: FatELF support in the binary loader.

This allows the kernel to load a single file that contains multiple ELF
binaries (a "FatELF" file), selecting the correct one for the system.

Details, rationale, tools, and patches for handling FatELF binaries can be
found at http://icculus.org/fatelf/

Please note that this requires userspace changes to be truly effective,
but a simple "hello world" FatELF binary can work with just the kernel
loader.

Signed-off-by: Ryan C. Gordon <[email protected]>
---
arch/ia64/ia32/binfmt_elf32.c | 4 +-
fs/binfmt_elf.c | 159 +++++++++++++++++++++++++++++++++++------
include/linux/elf.h | 24 ++++++
3 files changed, 162 insertions(+), 25 deletions(-)

diff --git a/arch/ia64/ia32/binfmt_elf32.c b/arch/ia64/ia32/binfmt_elf32.c
index c69552b..02d5dd1 100644
--- a/arch/ia64/ia32/binfmt_elf32.c
+++ b/arch/ia64/ia32/binfmt_elf32.c
@@ -223,12 +223,12 @@ elf32_set_personality (void)

static unsigned long
elf32_map(struct file *filep, unsigned long addr, struct elf_phdr *eppnt,
- int prot, int type, unsigned long unused)
+ int prot, int type, unsigned long unused, unsigned long base)
{
unsigned long pgoff = (eppnt->p_vaddr) & ~IA32_PAGE_MASK;

return ia32_do_mmap(filep, (addr & IA32_PAGE_MASK), eppnt->p_filesz + pgoff, prot, type,
- eppnt->p_offset - pgoff);
+ (eppnt->p_offset + base) - pgoff);
}

#define cpu_uses_ia32el() (local_cpu_data->family > 0x1f)
diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index b9b3bb5..9ac80f7 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -37,8 +37,9 @@

static int load_elf_binary(struct linux_binprm *bprm, struct pt_regs *regs);
static int load_elf_library(struct file *);
-static unsigned long elf_map(struct file *, unsigned long, struct elf_phdr *,
- int, int, unsigned long);
+static unsigned long
+elf_map(struct file *, unsigned long, struct elf_phdr *,
+ int, int, unsigned long, unsigned long);

/*
* If we don't support core dumping, then supply a NULL so we
@@ -319,7 +320,7 @@ create_elf_tables(struct linux_binprm *bprm, struct elfhdr *exec,

static unsigned long elf_map(struct file *filep, unsigned long addr,
struct elf_phdr *eppnt, int prot, int type,
- unsigned long total_size)
+ unsigned long total_size, unsigned long base_offset)
{
unsigned long map_addr;
unsigned long size = eppnt->p_filesz + ELF_PAGEOFFSET(eppnt->p_vaddr);
@@ -343,11 +344,14 @@ static unsigned long elf_map(struct file *filep, unsigned long addr,
*/
if (total_size) {
total_size = ELF_PAGEALIGN(total_size);
- map_addr = do_mmap(filep, addr, total_size, prot, type, off);
+ map_addr = do_mmap(filep, addr, total_size, prot, type,
+ off + base_offset);
if (!BAD_ADDR(map_addr))
do_munmap(current->mm, map_addr+size, total_size-size);
- } else
- map_addr = do_mmap(filep, addr, size, prot, type, off);
+ } else {
+ map_addr = do_mmap(filep, addr, size, prot, type,
+ off + base_offset);
+ }

up_write(&current->mm->mmap_sem);
return(map_addr);
@@ -381,7 +385,7 @@ static unsigned long total_mapping_size(struct elf_phdr *cmds, int nr)

static unsigned long load_elf_interp(struct elfhdr *interp_elf_ex,
struct file *interpreter, unsigned long *interp_map_addr,
- unsigned long no_base)
+ unsigned long no_base, unsigned long base_offset)
{
struct elf_phdr *elf_phdata;
struct elf_phdr *eppnt;
@@ -419,7 +423,7 @@ static unsigned long load_elf_interp(struct elfhdr *interp_elf_ex,
if (!elf_phdata)
goto out;

- retval = kernel_read(interpreter, interp_elf_ex->e_phoff,
+ retval = kernel_read(interpreter, interp_elf_ex->e_phoff + base_offset,
(char *)elf_phdata,size);
error = -EIO;
if (retval != size) {
@@ -455,7 +459,8 @@ static unsigned long load_elf_interp(struct elfhdr *interp_elf_ex,
load_addr = -vaddr;

map_addr = elf_map(interpreter, load_addr + vaddr,
- eppnt, elf_prot, elf_type, total_size);
+ eppnt, elf_prot, elf_type, total_size,
+ base_offset);
total_size = 0;
if (!*interp_map_addr)
*interp_map_addr = map_addr;
@@ -560,6 +565,94 @@ static unsigned long randomize_stack_top(unsigned long stack_top)
#endif
}

+/*
+ * See if we're a valid FatELF binary, find the right record, and
+ * load (*elf) with the actual ELF header. Sets (*offset) to the
+ * base offset of the chosen ELF binary. Returns 0 on success or a negative
+ * error code.
+ * If we're not a FatELF binary, (*elf) is loaded with the existing contents
+ * of (buf) and 0 is returned.
+ */
+static int examine_fatelf(struct file *file, const char *filename, char *buf,
+ int buflen, unsigned long *offset, struct elfhdr *elf)
+{
+ int records, i, rc;
+ const fatelf_hdr *fatelf = (fatelf_hdr *) buf;
+
+ if (likely(le32_to_cpu(fatelf->magic) != FATELF_MAGIC)) {
+ *elf = *((struct elfhdr *)buf); /* treat like normal ELF. */
+ return 0; /* not a FatELF binary; not an error. */
+ } else if (unlikely(le16_to_cpu(fatelf->version) != 1)) {
+ return -ENOEXEC; /* Unrecognized format version. */
+ }
+
+ /*
+ * In theory, there could be 255 separate records packed into this
+ * binary, but for now, bprm->buf (128 bytes) holds exactly 5
+ * records with the fatelf header, and that seems reasonable for
+ * most uses. We could add the complexity to read more records later
+ * if there's a serious need.
+ */
+ records = (int) fatelf->num_records; /* uint8, no byteswap needed */
+
+ if (unlikely(records > 5)) {
+ records = 5; /* clamp, in case we find one we can use. */
+ }
+
+ for (i = 0; i < records; i++) {
+ const fatelf_record *record = &fatelf->records[i];
+ const __u8 osabi = record->osabi;
+ const int abiok = likely( likely(osabi == ELFOSABI_NONE) ||
+ unlikely(osabi == ELFOSABI_LINUX) );
+
+ /* Fill in the data elf_check_arch() might care about. */
+ elf->e_ident[EI_OSABI] = record->osabi;
+ elf->e_ident[EI_CLASS] = record->word_size;
+ elf->e_ident[EI_DATA] = record->byte_order;
+ elf->e_machine = le16_to_cpu(record->machine);
+
+ if (likely(!elf_check_arch(elf))) {
+ continue; /* Unsupported CPU architecture. */
+ } else if (unlikely(!abiok)) {
+ continue; /* Unsupported OS ABI. */
+ } else if (unlikely(record->osabi_version != 0)) {
+ continue; /* Unsupported OS ABI version. */
+ } else {
+ /* We can support this ELF arch/abi. */
+ const __u64 rec_offset = le64_to_cpu(record->offset);
+ const __u64 rec_size = le64_to_cpu(record->size);
+ const __u64 end_offset = rec_offset + rec_size;
+ const unsigned long uloff = (unsigned long) rec_offset;
+
+ if (unlikely(end_offset < rec_offset)) {
+ continue; /* overflow (corrupt file?) */
+ } else if (unlikely(ELF_PAGEOFFSET(uloff) != 0)) {
+ continue; /* bad alignment. */
+ }
+
+#if BITS_PER_LONG == 32
+ if (unlikely(end_offset > 0xFFFFFFFF)) {
+ continue;
+ }
+#endif
+
+ /* replace the FatELF data with the real ELF header. */
+ rc = kernel_read(file, uloff, (char*) elf, sizeof(*elf));
+ if (unlikely((rc != sizeof(*elf)) && (rc >= 0))) {
+ rc = -EIO;
+ } else if (likely(rc == sizeof(*elf))) {
+ *offset = uloff;
+ rc = 0;
+ }
+
+ return rc;
+ }
+ }
+
+ return -ENOEXEC; /* no binaries we could use. */
+}
+
+
static int load_elf_binary(struct linux_binprm *bprm, struct pt_regs *regs)
{
struct file *interpreter = NULL; /* to shut gcc up */
@@ -571,6 +664,8 @@ static int load_elf_binary(struct linux_binprm *bprm, struct pt_regs *regs)
unsigned long elf_bss, elf_brk;
int retval, i;
unsigned int size;
+ unsigned long base_offset = 0;
+ unsigned long interp_base_offset = 0;
unsigned long elf_entry;
unsigned long interp_load_addr = 0;
unsigned long start_code, end_code, start_data, end_data;
@@ -587,9 +682,12 @@ static int load_elf_binary(struct linux_binprm *bprm, struct pt_regs *regs)
retval = -ENOMEM;
goto out_ret;
}
-
- /* Get the exec-header */
- loc->elf_ex = *((struct elfhdr *)bprm->buf);
+
+ retval = examine_fatelf(bprm->file, bprm->filename, bprm->buf,
+ BINPRM_BUF_SIZE, &base_offset, &loc->elf_ex);
+ if (unlikely(retval < 0)) {
+ goto out_ret;
+ }

retval = -ENOEXEC;
/* First of all, some simple consistency checks */
@@ -615,7 +713,7 @@ static int load_elf_binary(struct linux_binprm *bprm, struct pt_regs *regs)
if (!elf_phdata)
goto out;

- retval = kernel_read(bprm->file, loc->elf_ex.e_phoff,
+ retval = kernel_read(bprm->file, loc->elf_ex.e_phoff + base_offset,
(char *)elf_phdata, size);
if (retval != size) {
if (retval >= 0)
@@ -649,7 +747,8 @@ static int load_elf_binary(struct linux_binprm *bprm, struct pt_regs *regs)
if (!elf_interpreter)
goto out_free_ph;

- retval = kernel_read(bprm->file, elf_ppnt->p_offset,
+ retval = kernel_read(bprm->file,
+ elf_ppnt->p_offset + base_offset,
elf_interpreter,
elf_ppnt->p_filesz);
if (retval != elf_ppnt->p_filesz) {
@@ -704,8 +803,13 @@ static int load_elf_binary(struct linux_binprm *bprm, struct pt_regs *regs)
goto out_free_dentry;
}

- /* Get the exec headers */
- loc->interp_elf_ex = *((struct elfhdr *)bprm->buf);
+ retval = examine_fatelf(interpreter, elf_interpreter,
+ bprm->buf, BINPRM_BUF_SIZE,
+ &interp_base_offset,
+ &loc->interp_elf_ex);
+ if (unlikely(retval < 0)) {
+ goto out_free_dentry;
+ }
break;
}
elf_ppnt++;
@@ -830,7 +934,7 @@ static int load_elf_binary(struct linux_binprm *bprm, struct pt_regs *regs)
}

error = elf_map(bprm->file, load_bias + vaddr, elf_ppnt,
- elf_prot, elf_flags, 0);
+ elf_prot, elf_flags, 0, base_offset);
if (BAD_ADDR(error)) {
send_sig(SIGKILL, current, 0);
retval = IS_ERR((void *)error) ?
@@ -911,7 +1015,7 @@ static int load_elf_binary(struct linux_binprm *bprm, struct pt_regs *regs)
elf_entry = load_elf_interp(&loc->interp_elf_ex,
interpreter,
&interp_map_addr,
- load_bias);
+ load_bias, interp_base_offset);
if (!IS_ERR((void *)elf_entry)) {
/*
* load_elf_interp() returns relocation
@@ -1026,11 +1130,19 @@ static int load_elf_library(struct file *file)
unsigned long elf_bss, bss, len;
int retval, error, i, j;
struct elfhdr elf_ex;
+ unsigned long base_offset = 0;
+ char buf[BINPRM_BUF_SIZE];

- error = -ENOEXEC;
- retval = kernel_read(file, 0, (char *)&elf_ex, sizeof(elf_ex));
- if (retval != sizeof(elf_ex))
+ retval = kernel_read(file, 0, buf, sizeof(buf));
+ if (unlikely(retval != sizeof(buf))) {
+ error = (retval >= 0) ? -EIO : retval;
goto out;
+ }
+ error = examine_fatelf(file, 0, buf, sizeof(buf), &base_offset, &elf_ex);
+ if (unlikely(retval < 0)) {
+ goto out;
+ }
+ error = -ENOEXEC;

if (memcmp(elf_ex.e_ident, ELFMAG, SELFMAG) != 0)
goto out;
@@ -1052,7 +1164,8 @@ static int load_elf_library(struct file *file)

eppnt = elf_phdata;
error = -ENOEXEC;
- retval = kernel_read(file, elf_ex.e_phoff, (char *)eppnt, j);
+ retval = kernel_read(file, elf_ex.e_phoff + base_offset,
+ (char *)eppnt, j);
if (retval != j)
goto out_free_ph;

@@ -1074,7 +1187,7 @@ static int load_elf_library(struct file *file)
PROT_READ | PROT_WRITE | PROT_EXEC,
MAP_FIXED | MAP_PRIVATE | MAP_DENYWRITE,
(eppnt->p_offset -
- ELF_PAGEOFFSET(eppnt->p_vaddr)));
+ ELF_PAGEOFFSET(eppnt->p_vaddr)) + base_offset);
up_write(&current->mm->mmap_sem);
if (error != ELF_PAGESTART(eppnt->p_vaddr))
goto out_free_ph;
diff --git a/include/linux/elf.h b/include/linux/elf.h
index 90a4ed0..7c3a0b7 100644
--- a/include/linux/elf.h
+++ b/include/linux/elf.h
@@ -188,6 +188,30 @@ typedef struct elf64_sym {
} Elf64_Sym;


+/* FatELF (multiple ELF binaries in one file) support */
+#define FATELF_MAGIC (0x1F0E70FA)
+
+typedef struct fatelf_record {
+ __le16 machine; /* maps to e_machine */
+ __u8 osabi; /* maps to e_ident[EI_OSABI] */
+ __u8 osabi_version; /* maps to e_ident[EI_ABIVERSION] */
+ __u8 word_size; /* maps to e_ident[EI_CLASS] */
+ __u8 byte_order; /* maps to e_ident[EI_DATA] */
+ __u8 reserved0;
+ __u8 reserved1;
+ __le64 offset;
+ __le64 size;
+} fatelf_record;
+
+typedef struct fatelf_hdr {
+ __le32 magic;
+ __le16 version;
+ __u8 num_records;
+ __u8 reserved0;
+ fatelf_record records[];
+} fatelf_hdr;
+
+
#define EI_NIDENT 16

typedef struct elf32_hdr{
--
1.6.0.4


2009-10-20 00:12:12

by Jeremy Fitzhardinge

[permalink] [raw]
Subject: Re: [RFC][PATCH 1/2] binfmt_elf: FatELF support in the binary loader.

On 10/19/09 23:39, Ryan C. Gordon wrote:
> This allows the kernel to load a single file that contains multiple ELF
> binaries (a "FatELF" file), selecting the correct one for the system.
>
> Details, rationale, tools, and patches for handling FatELF binaries can be
> found at http://icculus.org/fatelf/
>
> Please note that this requires userspace changes to be truly effective,
> but a simple "hello world" FatELF binary can work with just the kernel
> loader.
>

The idea seem interesting, but does it need to be ELF-specific? What
about making the executable a simple archive file format (possibly just
an "ar" archive?) which contains other executables. The archive file
format would be implemented as its own binfmt, and the internal
executables could be arbitrary other executables. The outer loader
would just try execing each executable until one works (or it runs out).

That is, what you have here, but without hacking up binfmt_elf more.

J

> Signed-off-by: Ryan C. Gordon <[email protected]>
> ---
> arch/ia64/ia32/binfmt_elf32.c | 4 +-
> fs/binfmt_elf.c | 159 +++++++++++++++++++++++++++++++++++------
> include/linux/elf.h | 24 ++++++
> 3 files changed, 162 insertions(+), 25 deletions(-)
>
> diff --git a/arch/ia64/ia32/binfmt_elf32.c b/arch/ia64/ia32/binfmt_elf32.c
> index c69552b..02d5dd1 100644
> --- a/arch/ia64/ia32/binfmt_elf32.c
> +++ b/arch/ia64/ia32/binfmt_elf32.c
> @@ -223,12 +223,12 @@ elf32_set_personality (void)
>
> static unsigned long
> elf32_map(struct file *filep, unsigned long addr, struct elf_phdr *eppnt,
> - int prot, int type, unsigned long unused)
> + int prot, int type, unsigned long unused, unsigned long base)
> {
> unsigned long pgoff = (eppnt->p_vaddr) & ~IA32_PAGE_MASK;
>
> return ia32_do_mmap(filep, (addr & IA32_PAGE_MASK), eppnt->p_filesz + pgoff, prot, type,
> - eppnt->p_offset - pgoff);
> + (eppnt->p_offset + base) - pgoff);
> }
>
> #define cpu_uses_ia32el() (local_cpu_data->family > 0x1f)
> diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
> index b9b3bb5..9ac80f7 100644
> --- a/fs/binfmt_elf.c
> +++ b/fs/binfmt_elf.c
> @@ -37,8 +37,9 @@
>
> static int load_elf_binary(struct linux_binprm *bprm, struct pt_regs *regs);
> static int load_elf_library(struct file *);
> -static unsigned long elf_map(struct file *, unsigned long, struct elf_phdr *,
> - int, int, unsigned long);
> +static unsigned long
> +elf_map(struct file *, unsigned long, struct elf_phdr *,
> + int, int, unsigned long, unsigned long);
>
> /*
> * If we don't support core dumping, then supply a NULL so we
> @@ -319,7 +320,7 @@ create_elf_tables(struct linux_binprm *bprm, struct elfhdr *exec,
>
> static unsigned long elf_map(struct file *filep, unsigned long addr,
> struct elf_phdr *eppnt, int prot, int type,
> - unsigned long total_size)
> + unsigned long total_size, unsigned long base_offset)
> {
> unsigned long map_addr;
> unsigned long size = eppnt->p_filesz + ELF_PAGEOFFSET(eppnt->p_vaddr);
> @@ -343,11 +344,14 @@ static unsigned long elf_map(struct file *filep, unsigned long addr,
> */
> if (total_size) {
> total_size = ELF_PAGEALIGN(total_size);
> - map_addr = do_mmap(filep, addr, total_size, prot, type, off);
> + map_addr = do_mmap(filep, addr, total_size, prot, type,
> + off + base_offset);
> if (!BAD_ADDR(map_addr))
> do_munmap(current->mm, map_addr+size, total_size-size);
> - } else
> - map_addr = do_mmap(filep, addr, size, prot, type, off);
> + } else {
> + map_addr = do_mmap(filep, addr, size, prot, type,
> + off + base_offset);
> + }
>
> up_write(&current->mm->mmap_sem);
> return(map_addr);
> @@ -381,7 +385,7 @@ static unsigned long total_mapping_size(struct elf_phdr *cmds, int nr)
>
> static unsigned long load_elf_interp(struct elfhdr *interp_elf_ex,
> struct file *interpreter, unsigned long *interp_map_addr,
> - unsigned long no_base)
> + unsigned long no_base, unsigned long base_offset)
> {
> struct elf_phdr *elf_phdata;
> struct elf_phdr *eppnt;
> @@ -419,7 +423,7 @@ static unsigned long load_elf_interp(struct elfhdr *interp_elf_ex,
> if (!elf_phdata)
> goto out;
>
> - retval = kernel_read(interpreter, interp_elf_ex->e_phoff,
> + retval = kernel_read(interpreter, interp_elf_ex->e_phoff + base_offset,
> (char *)elf_phdata,size);
> error = -EIO;
> if (retval != size) {
> @@ -455,7 +459,8 @@ static unsigned long load_elf_interp(struct elfhdr *interp_elf_ex,
> load_addr = -vaddr;
>
> map_addr = elf_map(interpreter, load_addr + vaddr,
> - eppnt, elf_prot, elf_type, total_size);
> + eppnt, elf_prot, elf_type, total_size,
> + base_offset);
> total_size = 0;
> if (!*interp_map_addr)
> *interp_map_addr = map_addr;
> @@ -560,6 +565,94 @@ static unsigned long randomize_stack_top(unsigned long stack_top)
> #endif
> }
>
> +/*
> + * See if we're a valid FatELF binary, find the right record, and
> + * load (*elf) with the actual ELF header. Sets (*offset) to the
> + * base offset of the chosen ELF binary. Returns 0 on success or a negative
> + * error code.
> + * If we're not a FatELF binary, (*elf) is loaded with the existing contents
> + * of (buf) and 0 is returned.
> + */
> +static int examine_fatelf(struct file *file, const char *filename, char *buf,
> + int buflen, unsigned long *offset, struct elfhdr *elf)
> +{
> + int records, i, rc;
> + const fatelf_hdr *fatelf = (fatelf_hdr *) buf;
> +
> + if (likely(le32_to_cpu(fatelf->magic) != FATELF_MAGIC)) {
> + *elf = *((struct elfhdr *)buf); /* treat like normal ELF. */
> + return 0; /* not a FatELF binary; not an error. */
> + } else if (unlikely(le16_to_cpu(fatelf->version) != 1)) {
> + return -ENOEXEC; /* Unrecognized format version. */
> + }
> +
> + /*
> + * In theory, there could be 255 separate records packed into this
> + * binary, but for now, bprm->buf (128 bytes) holds exactly 5
> + * records with the fatelf header, and that seems reasonable for
> + * most uses. We could add the complexity to read more records later
> + * if there's a serious need.
> + */
> + records = (int) fatelf->num_records; /* uint8, no byteswap needed */
> +
> + if (unlikely(records > 5)) {
> + records = 5; /* clamp, in case we find one we can use. */
> + }
> +
> + for (i = 0; i < records; i++) {
> + const fatelf_record *record = &fatelf->records[i];
> + const __u8 osabi = record->osabi;
> + const int abiok = likely( likely(osabi == ELFOSABI_NONE) ||
> + unlikely(osabi == ELFOSABI_LINUX) );
> +
> + /* Fill in the data elf_check_arch() might care about. */
> + elf->e_ident[EI_OSABI] = record->osabi;
> + elf->e_ident[EI_CLASS] = record->word_size;
> + elf->e_ident[EI_DATA] = record->byte_order;
> + elf->e_machine = le16_to_cpu(record->machine);
> +
> + if (likely(!elf_check_arch(elf))) {
> + continue; /* Unsupported CPU architecture. */
> + } else if (unlikely(!abiok)) {
> + continue; /* Unsupported OS ABI. */
> + } else if (unlikely(record->osabi_version != 0)) {
> + continue; /* Unsupported OS ABI version. */
> + } else {
> + /* We can support this ELF arch/abi. */
> + const __u64 rec_offset = le64_to_cpu(record->offset);
> + const __u64 rec_size = le64_to_cpu(record->size);
> + const __u64 end_offset = rec_offset + rec_size;
> + const unsigned long uloff = (unsigned long) rec_offset;
> +
> + if (unlikely(end_offset < rec_offset)) {
> + continue; /* overflow (corrupt file?) */
> + } else if (unlikely(ELF_PAGEOFFSET(uloff) != 0)) {
> + continue; /* bad alignment. */
> + }
> +
> +#if BITS_PER_LONG == 32
> + if (unlikely(end_offset > 0xFFFFFFFF)) {
> + continue;
> + }
> +#endif
> +
> + /* replace the FatELF data with the real ELF header. */
> + rc = kernel_read(file, uloff, (char*) elf, sizeof(*elf));
> + if (unlikely((rc != sizeof(*elf)) && (rc >= 0))) {
> + rc = -EIO;
> + } else if (likely(rc == sizeof(*elf))) {
> + *offset = uloff;
> + rc = 0;
> + }
> +
> + return rc;
> + }
> + }
> +
> + return -ENOEXEC; /* no binaries we could use. */
> +}
> +
> +
> static int load_elf_binary(struct linux_binprm *bprm, struct pt_regs *regs)
> {
> struct file *interpreter = NULL; /* to shut gcc up */
> @@ -571,6 +664,8 @@ static int load_elf_binary(struct linux_binprm *bprm, struct pt_regs *regs)
> unsigned long elf_bss, elf_brk;
> int retval, i;
> unsigned int size;
> + unsigned long base_offset = 0;
> + unsigned long interp_base_offset = 0;
> unsigned long elf_entry;
> unsigned long interp_load_addr = 0;
> unsigned long start_code, end_code, start_data, end_data;
> @@ -587,9 +682,12 @@ static int load_elf_binary(struct linux_binprm *bprm, struct pt_regs *regs)
> retval = -ENOMEM;
> goto out_ret;
> }
> -
> - /* Get the exec-header */
> - loc->elf_ex = *((struct elfhdr *)bprm->buf);
> +
> + retval = examine_fatelf(bprm->file, bprm->filename, bprm->buf,
> + BINPRM_BUF_SIZE, &base_offset, &loc->elf_ex);
> + if (unlikely(retval < 0)) {
> + goto out_ret;
> + }
>
> retval = -ENOEXEC;
> /* First of all, some simple consistency checks */
> @@ -615,7 +713,7 @@ static int load_elf_binary(struct linux_binprm *bprm, struct pt_regs *regs)
> if (!elf_phdata)
> goto out;
>
> - retval = kernel_read(bprm->file, loc->elf_ex.e_phoff,
> + retval = kernel_read(bprm->file, loc->elf_ex.e_phoff + base_offset,
> (char *)elf_phdata, size);
> if (retval != size) {
> if (retval >= 0)
> @@ -649,7 +747,8 @@ static int load_elf_binary(struct linux_binprm *bprm, struct pt_regs *regs)
> if (!elf_interpreter)
> goto out_free_ph;
>
> - retval = kernel_read(bprm->file, elf_ppnt->p_offset,
> + retval = kernel_read(bprm->file,
> + elf_ppnt->p_offset + base_offset,
> elf_interpreter,
> elf_ppnt->p_filesz);
> if (retval != elf_ppnt->p_filesz) {
> @@ -704,8 +803,13 @@ static int load_elf_binary(struct linux_binprm *bprm, struct pt_regs *regs)
> goto out_free_dentry;
> }
>
> - /* Get the exec headers */
> - loc->interp_elf_ex = *((struct elfhdr *)bprm->buf);
> + retval = examine_fatelf(interpreter, elf_interpreter,
> + bprm->buf, BINPRM_BUF_SIZE,
> + &interp_base_offset,
> + &loc->interp_elf_ex);
> + if (unlikely(retval < 0)) {
> + goto out_free_dentry;
> + }
> break;
> }
> elf_ppnt++;
> @@ -830,7 +934,7 @@ static int load_elf_binary(struct linux_binprm *bprm, struct pt_regs *regs)
> }
>
> error = elf_map(bprm->file, load_bias + vaddr, elf_ppnt,
> - elf_prot, elf_flags, 0);
> + elf_prot, elf_flags, 0, base_offset);
> if (BAD_ADDR(error)) {
> send_sig(SIGKILL, current, 0);
> retval = IS_ERR((void *)error) ?
> @@ -911,7 +1015,7 @@ static int load_elf_binary(struct linux_binprm *bprm, struct pt_regs *regs)
> elf_entry = load_elf_interp(&loc->interp_elf_ex,
> interpreter,
> &interp_map_addr,
> - load_bias);
> + load_bias, interp_base_offset);
> if (!IS_ERR((void *)elf_entry)) {
> /*
> * load_elf_interp() returns relocation
> @@ -1026,11 +1130,19 @@ static int load_elf_library(struct file *file)
> unsigned long elf_bss, bss, len;
> int retval, error, i, j;
> struct elfhdr elf_ex;
> + unsigned long base_offset = 0;
> + char buf[BINPRM_BUF_SIZE];
>
> - error = -ENOEXEC;
> - retval = kernel_read(file, 0, (char *)&elf_ex, sizeof(elf_ex));
> - if (retval != sizeof(elf_ex))
> + retval = kernel_read(file, 0, buf, sizeof(buf));
> + if (unlikely(retval != sizeof(buf))) {
> + error = (retval >= 0) ? -EIO : retval;
> goto out;
> + }
> + error = examine_fatelf(file, 0, buf, sizeof(buf), &base_offset, &elf_ex);
> + if (unlikely(retval < 0)) {
> + goto out;
> + }
> + error = -ENOEXEC;
>
> if (memcmp(elf_ex.e_ident, ELFMAG, SELFMAG) != 0)
> goto out;
> @@ -1052,7 +1164,8 @@ static int load_elf_library(struct file *file)
>
> eppnt = elf_phdata;
> error = -ENOEXEC;
> - retval = kernel_read(file, elf_ex.e_phoff, (char *)eppnt, j);
> + retval = kernel_read(file, elf_ex.e_phoff + base_offset,
> + (char *)eppnt, j);
> if (retval != j)
> goto out_free_ph;
>
> @@ -1074,7 +1187,7 @@ static int load_elf_library(struct file *file)
> PROT_READ | PROT_WRITE | PROT_EXEC,
> MAP_FIXED | MAP_PRIVATE | MAP_DENYWRITE,
> (eppnt->p_offset -
> - ELF_PAGEOFFSET(eppnt->p_vaddr)));
> + ELF_PAGEOFFSET(eppnt->p_vaddr)) + base_offset);
> up_write(&current->mm->mmap_sem);
> if (error != ELF_PAGESTART(eppnt->p_vaddr))
> goto out_free_ph;
> diff --git a/include/linux/elf.h b/include/linux/elf.h
> index 90a4ed0..7c3a0b7 100644
> --- a/include/linux/elf.h
> +++ b/include/linux/elf.h
> @@ -188,6 +188,30 @@ typedef struct elf64_sym {
> } Elf64_Sym;
>
>
> +/* FatELF (multiple ELF binaries in one file) support */
> +#define FATELF_MAGIC (0x1F0E70FA)
> +
> +typedef struct fatelf_record {
> + __le16 machine; /* maps to e_machine */
> + __u8 osabi; /* maps to e_ident[EI_OSABI] */
> + __u8 osabi_version; /* maps to e_ident[EI_ABIVERSION] */
> + __u8 word_size; /* maps to e_ident[EI_CLASS] */
> + __u8 byte_order; /* maps to e_ident[EI_DATA] */
> + __u8 reserved0;
> + __u8 reserved1;
> + __le64 offset;
> + __le64 size;
> +} fatelf_record;
> +
> +typedef struct fatelf_hdr {
> + __le32 magic;
> + __le16 version;
> + __u8 num_records;
> + __u8 reserved0;
> + fatelf_record records[];
> +} fatelf_hdr;
> +
> +
> #define EI_NIDENT 16
>
> typedef struct elf32_hdr{
>

2009-10-20 04:43:15

by Ryan C. Gordon

[permalink] [raw]
Subject: Re: [RFC][PATCH 1/2] binfmt_elf: FatELF support in the binary loader.


> The idea seem interesting, but does it need to be ELF-specific? What
> about making the executable a simple archive file format (possibly just
> an "ar" archive?) which contains other executables. The archive file
> format would be implemented as its own binfmt, and the internal
> executables could be arbitrary other executables. The outer loader
> would just try execing each executable until one works (or it runs out).

I'm not sure the added flexibility is worth the extra complications.
FatELF solves a specific problem: merging multiple ELF targets into one
file, the most compelling use-case being to glue x86_64 and i686 binaries
together.

What you're describing would definitely be the route I'd have chosen if,
say, a.out files were still in widespread use and actively competed with
ELF for mindshare.

> That is, what you have here, but without hacking up binfmt_elf more.

I like to think of it as art, like a chef carving a fine piece of meat. :)

--ryan.

2009-10-21 08:09:08

by Ryan C. Gordon

[permalink] [raw]
Subject: Re: [RFC][PATCH 1/2] binfmt_elf: FatELF support in the binary loader.


Updated version of the FatELF binary loader, based on Americo Wang's code
review.

--ryan.


>From bb5ddbc5a0a9b7853664a15403ae780dab2f87c3 Mon Sep 17 00:00:00 2001
From: Ryan C. Gordon <[email protected]>
Date: Tue, 20 Oct 2009 23:21:38 -0400
Subject: [PATCH] binfmt_elf: FatELF support in the binary loader.

This allows the kernel to load a single file that contains multiple ELF
binaries (a "FatELF" file), selecting the correct one for the system.

Details, rationale, tools, and patches for handling FatELF binaries can be
found at http://icculus.org/fatelf/

Signed-off-by: Ryan C. Gordon <[email protected]>
---
arch/ia64/ia32/binfmt_elf32.c | 4 +-
fs/binfmt_elf.c | 153 ++++++++++++++++++++++++++++++++++-------
include/linux/elf.h | 23 ++++++
3 files changed, 154 insertions(+), 26 deletions(-)

diff --git a/arch/ia64/ia32/binfmt_elf32.c b/arch/ia64/ia32/binfmt_elf32.c
index c69552b..02d5dd1 100644
--- a/arch/ia64/ia32/binfmt_elf32.c
+++ b/arch/ia64/ia32/binfmt_elf32.c
@@ -223,12 +223,12 @@ elf32_set_personality (void)

static unsigned long
elf32_map(struct file *filep, unsigned long addr, struct elf_phdr *eppnt,
- int prot, int type, unsigned long unused)
+ int prot, int type, unsigned long unused, unsigned long base)
{
unsigned long pgoff = (eppnt->p_vaddr) & ~IA32_PAGE_MASK;

return ia32_do_mmap(filep, (addr & IA32_PAGE_MASK), eppnt->p_filesz + pgoff, prot, type,
- eppnt->p_offset - pgoff);
+ (eppnt->p_offset + base) - pgoff);
}

#define cpu_uses_ia32el() (local_cpu_data->family > 0x1f)
diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index b9b3bb5..67ac2e1 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -37,8 +37,9 @@

static int load_elf_binary(struct linux_binprm *bprm, struct pt_regs *regs);
static int load_elf_library(struct file *);
-static unsigned long elf_map(struct file *, unsigned long, struct elf_phdr *,
- int, int, unsigned long);
+static unsigned long
+elf_map(struct file *, unsigned long, struct elf_phdr *,
+ int, int, unsigned long, unsigned long);

/*
* If we don't support core dumping, then supply a NULL so we
@@ -319,7 +320,7 @@ create_elf_tables(struct linux_binprm *bprm, struct elfhdr *exec,

static unsigned long elf_map(struct file *filep, unsigned long addr,
struct elf_phdr *eppnt, int prot, int type,
- unsigned long total_size)
+ unsigned long total_size, unsigned long base_offset)
{
unsigned long map_addr;
unsigned long size = eppnt->p_filesz + ELF_PAGEOFFSET(eppnt->p_vaddr);
@@ -343,11 +344,14 @@ static unsigned long elf_map(struct file *filep, unsigned long addr,
*/
if (total_size) {
total_size = ELF_PAGEALIGN(total_size);
- map_addr = do_mmap(filep, addr, total_size, prot, type, off);
+ map_addr = do_mmap(filep, addr, total_size, prot, type,
+ off + base_offset);
if (!BAD_ADDR(map_addr))
do_munmap(current->mm, map_addr+size, total_size-size);
- } else
- map_addr = do_mmap(filep, addr, size, prot, type, off);
+ } else {
+ map_addr = do_mmap(filep, addr, size, prot, type,
+ off + base_offset);
+ }

up_write(&current->mm->mmap_sem);
return(map_addr);
@@ -381,7 +385,7 @@ static unsigned long total_mapping_size(struct elf_phdr *cmds, int nr)

static unsigned long load_elf_interp(struct elfhdr *interp_elf_ex,
struct file *interpreter, unsigned long *interp_map_addr,
- unsigned long no_base)
+ unsigned long no_base, unsigned long base_offset)
{
struct elf_phdr *elf_phdata;
struct elf_phdr *eppnt;
@@ -419,7 +423,7 @@ static unsigned long load_elf_interp(struct elfhdr *interp_elf_ex,
if (!elf_phdata)
goto out;

- retval = kernel_read(interpreter, interp_elf_ex->e_phoff,
+ retval = kernel_read(interpreter, interp_elf_ex->e_phoff + base_offset,
(char *)elf_phdata,size);
error = -EIO;
if (retval != size) {
@@ -455,7 +459,8 @@ static unsigned long load_elf_interp(struct elfhdr *interp_elf_ex,
load_addr = -vaddr;

map_addr = elf_map(interpreter, load_addr + vaddr,
- eppnt, elf_prot, elf_type, total_size);
+ eppnt, elf_prot, elf_type, total_size,
+ base_offset);
total_size = 0;
if (!*interp_map_addr)
*interp_map_addr = map_addr;
@@ -560,10 +565,91 @@ static unsigned long randomize_stack_top(unsigned long stack_top)
#endif
}

+/*
+ * See if we're a valid FatELF binary, find the right record, and
+ * load (*elf) with the actual ELF header. Sets (*offset) to the
+ * base offset of the chosen ELF binary. Returns 0 on success or a negative
+ * error code.
+ * If we're not a FatELF binary, (*elf) is loaded with the existing contents
+ * of (buf) and 0 is returned.
+ */
+static int examine_fatelf(struct file *file, const char *filename, char *buf,
+ int buflen, unsigned long *offset, struct elfhdr *elf)
+{
+ int records, i, rc;
+ const Fatelf_hdr *fatelf = (Fatelf_hdr *) buf;
+
+ if (likely(le32_to_cpu(fatelf->magic) != FATELF_MAGIC)) {
+ *elf = *((struct elfhdr *)buf); /* treat like normal ELF. */
+ return 0; /* not a FatELF binary; not an error. */
+ }
+
+ if (unlikely(le16_to_cpu(fatelf->version) != 1))
+ return -ENOEXEC; /* Unrecognized format version. */
+
+ /*
+ * In theory, there could be 255 separate records packed into this
+ * binary, but for now, bprm->buf (128 bytes) holds exactly 5
+ * records with the fatelf header, and that seems reasonable for
+ * most uses. We could add the complexity to read more records later
+ * if there's a serious need.
+ */
+ records = (int) fatelf->num_records; /* uint8, no byteswap needed */
+ if (unlikely(records > 5))
+ records = 5; /* clamp, in case we find one we can use. */
+
+ for (i = 0; i < records; i++) {
+ const Fatelf_record *record = &fatelf->records[i];
+ const __u8 osabi = record->osabi;
+ const int abiok = likely( likely(osabi == ELFOSABI_NONE) ||
+ unlikely(osabi == ELFOSABI_LINUX) );
+
+ /* Fill in the data elf_check_arch() might care about. */
+ elf->e_ident[EI_OSABI] = record->osabi;
+ elf->e_ident[EI_CLASS] = record->word_size;
+ elf->e_ident[EI_DATA] = record->byte_order;
+ elf->e_machine = le16_to_cpu(record->machine);
+
+ if (unlikely(elf_check_arch(elf))
+ && likely(abiok)
+ && likely(record->osabi_version == 0)) {
+ /* We can support this ELF arch/abi. */
+ const __u64 rec_offset = le64_to_cpu(record->offset);
+ const __u64 rec_size = le64_to_cpu(record->size);
+ const __u64 end_offset = rec_offset + rec_size;
+ const unsigned long uloff = (unsigned long) rec_offset;
+
+#if BITS_PER_LONG == 32
+ if (unlikely(end_offset > 0xFFFFFFFF))
+ continue;
+#endif
+
+ if (unlikely(end_offset < rec_offset))
+ continue; /* overflow (corrupt file?) */
+ if (unlikely(ELF_PAGEOFFSET(uloff) != 0))
+ continue; /* bad alignment. */
+
+ /* replace the FatELF data with the real ELF header. */
+ rc = kernel_read(file, uloff, (char*) elf, sizeof(*elf));
+ if (unlikely((rc != sizeof(*elf)) && (rc >= 0)))
+ rc = -EIO;
+ else if (likely(rc == sizeof(*elf))) {
+ *offset = uloff;
+ rc = 0;
+ }
+
+ return rc;
+ }
+ }
+
+ return -ENOEXEC; /* no binaries we could use. */
+}
+
+
static int load_elf_binary(struct linux_binprm *bprm, struct pt_regs *regs)
{
struct file *interpreter = NULL; /* to shut gcc up */
- unsigned long load_addr = 0, load_bias = 0;
+ unsigned long load_addr = 0, load_bias = 0;
int load_addr_set = 0;
char * elf_interpreter = NULL;
unsigned long error;
@@ -571,6 +657,8 @@ static int load_elf_binary(struct linux_binprm *bprm, struct pt_regs *regs)
unsigned long elf_bss, elf_brk;
int retval, i;
unsigned int size;
+ unsigned long base_offset = 0;
+ unsigned long interp_base_offset = 0;
unsigned long elf_entry;
unsigned long interp_load_addr = 0;
unsigned long start_code, end_code, start_data, end_data;
@@ -587,9 +675,11 @@ static int load_elf_binary(struct linux_binprm *bprm, struct pt_regs *regs)
retval = -ENOMEM;
goto out_ret;
}
-
- /* Get the exec-header */
- loc->elf_ex = *((struct elfhdr *)bprm->buf);
+
+ retval = examine_fatelf(bprm->file, bprm->filename, bprm->buf,
+ BINPRM_BUF_SIZE, &base_offset, &loc->elf_ex);
+ if (unlikely(retval < 0))
+ goto out_ret;

retval = -ENOEXEC;
/* First of all, some simple consistency checks */
@@ -615,7 +705,7 @@ static int load_elf_binary(struct linux_binprm *bprm, struct pt_regs *regs)
if (!elf_phdata)
goto out;

- retval = kernel_read(bprm->file, loc->elf_ex.e_phoff,
+ retval = kernel_read(bprm->file, loc->elf_ex.e_phoff + base_offset,
(char *)elf_phdata, size);
if (retval != size) {
if (retval >= 0)
@@ -649,7 +739,8 @@ static int load_elf_binary(struct linux_binprm *bprm, struct pt_regs *regs)
if (!elf_interpreter)
goto out_free_ph;

- retval = kernel_read(bprm->file, elf_ppnt->p_offset,
+ retval = kernel_read(bprm->file,
+ elf_ppnt->p_offset + base_offset,
elf_interpreter,
elf_ppnt->p_filesz);
if (retval != elf_ppnt->p_filesz) {
@@ -704,8 +795,13 @@ static int load_elf_binary(struct linux_binprm *bprm, struct pt_regs *regs)
goto out_free_dentry;
}

- /* Get the exec headers */
- loc->interp_elf_ex = *((struct elfhdr *)bprm->buf);
+ retval = examine_fatelf(interpreter, elf_interpreter,
+ bprm->buf, BINPRM_BUF_SIZE,
+ &interp_base_offset,
+ &loc->interp_elf_ex);
+ if (unlikely(retval < 0)) {
+ goto out_free_dentry;
+ }
break;
}
elf_ppnt++;
@@ -830,7 +926,7 @@ static int load_elf_binary(struct linux_binprm *bprm, struct pt_regs *regs)
}

error = elf_map(bprm->file, load_bias + vaddr, elf_ppnt,
- elf_prot, elf_flags, 0);
+ elf_prot, elf_flags, 0, base_offset);
if (BAD_ADDR(error)) {
send_sig(SIGKILL, current, 0);
retval = IS_ERR((void *)error) ?
@@ -911,7 +1007,7 @@ static int load_elf_binary(struct linux_binprm *bprm, struct pt_regs *regs)
elf_entry = load_elf_interp(&loc->interp_elf_ex,
interpreter,
&interp_map_addr,
- load_bias);
+ load_bias, interp_base_offset);
if (!IS_ERR((void *)elf_entry)) {
/*
* load_elf_interp() returns relocation
@@ -1026,11 +1122,19 @@ static int load_elf_library(struct file *file)
unsigned long elf_bss, bss, len;
int retval, error, i, j;
struct elfhdr elf_ex;
+ unsigned long base_offset = 0;
+ char buf[BINPRM_BUF_SIZE];

- error = -ENOEXEC;
- retval = kernel_read(file, 0, (char *)&elf_ex, sizeof(elf_ex));
- if (retval != sizeof(elf_ex))
+ retval = kernel_read(file, 0, buf, sizeof(buf));
+ if (unlikely(retval != sizeof(buf))) {
+ error = (retval >= 0) ? -EIO : retval;
goto out;
+ }
+ error = examine_fatelf(file, 0, buf, sizeof(buf), &base_offset, &elf_ex);
+ if (unlikely(retval < 0)) {
+ goto out;
+ }
+ error = -ENOEXEC;

if (memcmp(elf_ex.e_ident, ELFMAG, SELFMAG) != 0)
goto out;
@@ -1052,7 +1156,8 @@ static int load_elf_library(struct file *file)

eppnt = elf_phdata;
error = -ENOEXEC;
- retval = kernel_read(file, elf_ex.e_phoff, (char *)eppnt, j);
+ retval = kernel_read(file, elf_ex.e_phoff + base_offset,
+ (char *)eppnt, j);
if (retval != j)
goto out_free_ph;

@@ -1074,7 +1179,7 @@ static int load_elf_library(struct file *file)
PROT_READ | PROT_WRITE | PROT_EXEC,
MAP_FIXED | MAP_PRIVATE | MAP_DENYWRITE,
(eppnt->p_offset -
- ELF_PAGEOFFSET(eppnt->p_vaddr)));
+ ELF_PAGEOFFSET(eppnt->p_vaddr)) + base_offset);
up_write(&current->mm->mmap_sem);
if (error != ELF_PAGESTART(eppnt->p_vaddr))
goto out_free_ph;
diff --git a/include/linux/elf.h b/include/linux/elf.h
index 90a4ed0..ded1fb6 100644
--- a/include/linux/elf.h
+++ b/include/linux/elf.h
@@ -33,6 +33,29 @@ typedef __u32 Elf64_Word;
typedef __u64 Elf64_Xword;
typedef __s64 Elf64_Sxword;

+/* FatELF (multiple ELF binaries in one file) support */
+#define FATELF_MAGIC (0x1F0E70FA)
+
+typedef struct Fatelf_record {
+ __le16 machine; /* maps to e_machine */
+ __u8 osabi; /* maps to e_ident[EI_OSABI] */
+ __u8 osabi_version; /* maps to e_ident[EI_ABIVERSION] */
+ __u8 word_size; /* maps to e_ident[EI_CLASS] */
+ __u8 byte_order; /* maps to e_ident[EI_DATA] */
+ __u8 reserved0;
+ __u8 reserved1;
+ __le64 offset;
+ __le64 size;
+} Fatelf_record;
+
+typedef struct Fatelf_hdr {
+ __le32 magic;
+ __le16 version;
+ __u8 num_records;
+ __u8 reserved0;
+ Fatelf_record records[];
+} Fatelf_hdr;
+
/* These constants are for the segment types stored in the image headers */
#define PT_NULL 0
#define PT_LOAD 1
--
1.6.0.4

2009-10-22 07:31:11

by Nicholas Miell

[permalink] [raw]
Subject: Re: [RFC][PATCH 1/2] binfmt_elf: FatELF support in the binary loader.

On Mon, 2009-10-19 at 10:39 -0400, Ryan C. Gordon wrote:
> This allows the kernel to load a single file that contains multiple ELF
> binaries (a "FatELF" file), selecting the correct one for the system.
>
> Details, rationale, tools, and patches for handling FatELF binaries can be
> found at http://icculus.org/fatelf/
>
> Please note that this requires userspace changes to be truly effective,
> but a simple "hello world" FatELF binary can work with just the kernel
> loader.

Apple's fat binaries have the virtue of allowing redundant identical
sections to be merged among the different included binaries, but your
format can't do that.

Could you explain how your FatELF format is an improvement over multiple
ELF binaries and a simple shell script that selects between them?

--
Nicholas Miell <[email protected]>

2009-10-22 09:22:55

by Ryan C. Gordon

[permalink] [raw]
Subject: Re: [RFC][PATCH 1/2] binfmt_elf: FatELF support in the binary loader.


> Apple's fat binaries have the virtue of allowing redundant identical
> sections to be merged among the different included binaries, but your
> format can't do that.

Neither can Apple's. They employ a method similar to FatELF: a list of
offsets and sizes of self-contained Mach-O binaries within a container
file. Apple's previous attempt, for 68k+PowerPC binaries on the classic
Mac OS, stored one architecture in the data fork and one in the resource
fork, so they definitely couldn't overlap.

At the high level, ease of use and flexibility are primary values in
keeping the ELF objects separate. You can easily glue arbitrary ELF
binaries together from a simple command line tool without having to
rewrite the actual ELF content in any of them. Also, it makes the changes
needed to support FatELF extremely minimal: parse a simple array of
records, adjust the file offset, and then use the heavily-tested and
well-maintained ELF code that's been in the kernel for over a decade. This
email is probably longer than the FatELF kernel patch. :)

At the more technical level: there probably isn't much you could actually
share between ELF binaries for any non-trivial program, and even if you
could, I'm not sure the sharp increase complexity is worth it for the
small space gains.

> Could you explain how your FatELF format is an improvement over multiple
> ELF binaries and a simple shell script that selects between them?

I'll preface my answer with a note: I am an _idiot_ at shell scripting.
Here're two examples that prove it.

- I once shipped a game that used pushd in its shell script to choose the
correct binary's folder. Anyone that's running Ubuntu can tell you that
this fails when /bin/sh ceases to point to bash. That game now fails to
start up. Human error, but still avoidable had there been a better
solution.

- Many years ago, I shipped a game that ran on i686 and PowerPC Linux. I
could not have predicted that one day people would be running x86_64
systems that would be able to run the i686 version, so doing something
like: exec $(uname -m)/mygame would fail, and there's really no good way
to future-proof that sort of thing. As that game now fails to start on
x86_64 systems, it would have been better to just ship for i686 and not
try to select a CPU arch.

There are places where this sort of mentality is suboptimal anyhow: I
consider the symlinks to /lib64 and /lib32 that you see on various distros
to be a version of the shell script tactic. It works, but it's not
really the cleanest approach.

I can also envision places where shell scripts aren't useful: web browser
plugins, scripting language bindings, and other places where we want to
load a shared library into a process that runs the risk of being 64-bit
when the other thing is 32-bit. Were the shared library a FatELF that had
both architectures, it would work without concern.

All of these things could have a million incompatible, one-off solutions:
shell scripts and symlinks and a million human errors. Doing it simply and
cleanly in one central place makes sense. I prefer that over loading an
entire scripting language interpreter just to load the correct ELF file,
and praying nothing ever changes to jeopardize the delicate balance.

--ryan.

2009-10-23 22:16:19

by Jeremy Fitzhardinge

[permalink] [raw]
Subject: Re: [RFC][PATCH 1/2] binfmt_elf: FatELF support in the binary loader.

On 10/19/09 21:43, Ryan C. Gordon wrote:
> I'm not sure the added flexibility is worth the extra complications.
> FatELF solves a specific problem: merging multiple ELF targets into one
> file, the most compelling use-case being to glue x86_64 and i686 binaries
> together.
>
> What you're describing would definitely be the route I'd have chosen if,
> say, a.out files were still in widespread use and actively competed with
> ELF for mindshare.
>

It would be nice to have something that would conceptually work for
other architectures with other executable file formats. Granted ELF is
most common, but its far from the only one.

A generic approach would allow the last-option fallback executable to be
a shell/python/perl script which could do something useful (like display
a useful message).

>> That is, what you have here, but without hacking up binfmt_elf more.
>>
> I like to think of it as art, like a chef carving a fine piece of meat. :)
>

Well, its really a bit diseased, with lots of gristly bits and a few
unexpected tubes sticking out the side.

J

2009-10-24 00:25:52

by Ryan C. Gordon

[permalink] [raw]
Subject: Re: [RFC][PATCH 1/2] binfmt_elf: FatELF support in the binary loader.


> It would be nice to have something that would conceptually work for
> other architectures with other executable file formats. Granted ELF is
> most common, but its far from the only one.

It would be nice, but I'm not certain it's practical. I mean, if we embed
a Windows .exe in there, Windows is never going to be able to load it, I
presume.

That being said, I suppose there's no reason you _couldn't_ do that with
the existing FatELF format. Just add an OSABI for, say, Windows PE files
and embed one like anything else. Non-Windows platforms will ignore it
because it's the wrong OSABI. If something tried to process it as an ELF
file, it'll correctly believe it to be a corrupt file.

(Replace Windows with your favorite non-ELF platform if you like.)

This isn't really a goal of the project, but since you brought it up, it
IS actually feasible with the existing code and framework.

> Well, its really a bit diseased, with lots of gristly bits and a few
> unexpected tubes sticking out the side.

Eww.

--ryan.