Messages in this thread | | | Date | Fri, 9 Jan 2004 05:28:51 -0500 | From | Jakub Jelinek <> | Subject | Re: [PATCH][RFC] invalid ELF binaries can execute - better sanity checking |
| |
On Fri, Jan 09, 2004 at 03:19:12AM +0100, Jesper Juhl wrote: > --- linux-2.6.1-rc1-mm2-orig/fs/binfmt_elf.c 2003-12-31 05:47:13.000000000 +0100 > +++ linux-2.6.1-rc1-mm2/fs/binfmt_elf.c 2004-01-09 01:41:05.000000000 +0100 > @@ -482,11 +482,14 @@ static int load_elf_binary(struct linux_ > /* First of all, some simple consistency checks */ > if (memcmp(elf_ex.e_ident, ELFMAG, SELFMAG) != 0) > goto out; > - > + if (elf_ex.e_ident[EI_CLASS] == ELFCLASSNONE) > + goto out; > if (elf_ex.e_type != ET_EXEC && elf_ex.e_type != ET_DYN) > goto out; > if (!elf_check_arch(&elf_ex)) > goto out; > + if (elf_ex.e_version == EV_NONE) > + goto out; > if (!bprm->file->f_op||!bprm->file->f_op->mmap) > goto out;
These checks look useless for me. If you want to check EI_CLASS or e_version, why is 0 so special and not say 157? If you want to be sure EI_CLASS and e_version are correct, the test should be: if (elf_ex.e_ident[EI_CLASS] != ELF_CLASS) goto out; resp. if (elf_ex.e_version != EV_CURRENT) goto out; Furthermore, there is load_elf_interp, which needs similar checks, otherwise they are useless, because you can create a proper ELF binary loading incorrect ELF interpreter. Why not to check EI_DATA and EI_VERSION as well though? glibc loader does: #ifndef VALID_ELF_HEADER # define VALID_ELF_HEADER(hdr,exp,size) (memcmp (hdr, exp, size) == 0) # define VALID_ELF_OSABI(osabi) (osabi == ELFOSABI_SYSV) # define VALID_ELF_ABIVERSION(ver) (ver == 0) #endif static const unsigned char expected[EI_PAD] = { [EI_MAG0] = ELFMAG0, [EI_MAG1] = ELFMAG1, [EI_MAG2] = ELFMAG2, [EI_MAG3] = ELFMAG3, [EI_CLASS] = ELFW(CLASS), [EI_DATA] = byteorder, [EI_VERSION] = EV_CURRENT, [EI_OSABI] = ELFOSABI_SYSV, [EI_ABIVERSION] = 0 }; if (__builtin_expect (! VALID_ELF_HEADER (ehdr->e_ident, expected, EI_PAD), 0)) { ... } if (__builtin_expect (ehdr->e_version, EV_CURRENT) != EV_CURRENT) { errstring = N_("ELF file version does not match current one"); goto call_lose; } if (! __builtin_expect (elf_machine_matches_host (ehdr), 1)) goto close_and_out; ...
Perhaps binfmt_elf.c wants to be able to load different OSABI ELF objects, if so, it could just memcmp the first EI_OSABI bytes of e_ident and check e_version and other fields outside of e_ident.
Jakub - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |