lkml.org 
[lkml]   [2004]   [Jan]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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/

\
 
 \ /
  Last update: 2005-03-22 13:59    [W:0.291 / U:0.184 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site