This is the mail archive of the
elfutils-devel@sourceware.org
mailing list for the elfutils project.
[PATCH] libelf: Split checks for ehdr and shdr, drop phdr check in file_read_elf.
- From: Mark Wielaard <mjw at redhat dot com>
- To: elfutils-devel at lists dot fedorahosted dot org
- Date: Tue, 02 Jun 2015 14:01:53 +0200
- Subject: [PATCH] libelf: Split checks for ehdr and shdr, drop phdr check in file_read_elf.
There are various places in the code that check whether mmapped structures
are correctly aligned (or ALLOW_UNALIGNED is set). Some of these checks
are asserts. Like the one in elf(32|64)_getshdr. We should not get into
that part of the code if the shdr scn structure was cached in elf_begin
because it was mmapped in and properly aligned.
These asserts could trigger because in elf_begin.c file_read_elf ()
all alignment checks were combined. So even though only one of the ehdr,
shdr or phdr structures were not properly aligned all structures would be
copied. Also the phdr structure was not even read in elf_begin, so the
alignment check was unnecessary.
This patch splits the alignment checks and reading of ehdr and shdr
structures into separate code paths. It also drops the phdr alignment
checks in elf_begin. Those phdr checks are done in elf(32|64)_getphdr
already.
Signed-off-by: Mark Wielaard <mjw@redhat.com>
---
libelf/ChangeLog | 5 ++
libelf/elf_begin.c | 140 ++++++++++++++++++++++++++++-------------------------
2 files changed, 80 insertions(+), 65 deletions(-)
diff --git a/libelf/ChangeLog b/libelf/ChangeLog
index 79308fe..fd2fc53 100644
--- a/libelf/ChangeLog
+++ b/libelf/ChangeLog
@@ -1,3 +1,8 @@
+2015-06-02 Mark Wielaard <mjw@redhat.com>
+
+ * elf_begin.c (file_read_elf): Split checks for ehdr and shdr
+ alignment, drop phdr alignment check.
+
2015-05-31 Mark Wielaard <mjw@redhat.com>
* elf32_getshdr.c (load_shdr_wrlock): Allocate shdrs with malloc,
diff --git a/libelf/elf_begin.c b/libelf/elf_begin.c
index ae1e712..e2e3b6b 100644
--- a/libelf/elf_begin.c
+++ b/libelf/elf_begin.c
@@ -1,5 +1,5 @@
/* Create descriptor for processing file.
- Copyright (C) 1998-2010, 2012, 2014 Red Hat, Inc.
+ Copyright (C) 1998-2010, 2012, 2014, 2015 Red Hat, Inc.
This file is part of elfutils.
Written by Ulrich Drepper <drepper@redhat.com>, 1998.
@@ -306,17 +306,46 @@ file_read_elf (int fildes, void *map_address, unsigned char *e_ident,
/* This is a 32-bit binary. */
if (map_address != NULL && e_ident[EI_DATA] == MY_ELFDATA
&& (ALLOW_UNALIGNED
- || ((((uintptr_t) ehdr) & (__alignof__ (Elf32_Ehdr) - 1)) == 0
- && ((uintptr_t) ((char *) ehdr + ehdr->e_shoff)
- & (__alignof__ (Elf32_Shdr) - 1)) == 0
- && ((uintptr_t) ((char *) ehdr + ehdr->e_phoff)
- & (__alignof__ (Elf32_Phdr) - 1)) == 0)))
+ || (((uintptr_t) ehdr) & (__alignof__ (Elf32_Ehdr) - 1)) == 0))
{
/* We can use the mmapped memory. */
elf->state.elf32.ehdr = ehdr;
+ }
+ else
+ {
+ /* Copy the ELF header. */
+ elf->state.elf32.ehdr = memcpy (&elf->state.elf32.ehdr_mem, e_ident,
+ sizeof (Elf32_Ehdr));
+
+ if (e_ident[EI_DATA] != MY_ELFDATA)
+ {
+ CONVERT (elf->state.elf32.ehdr_mem.e_type);
+ CONVERT (elf->state.elf32.ehdr_mem.e_machine);
+ CONVERT (elf->state.elf32.ehdr_mem.e_version);
+ CONVERT (elf->state.elf32.ehdr_mem.e_entry);
+ CONVERT (elf->state.elf32.ehdr_mem.e_phoff);
+ CONVERT (elf->state.elf32.ehdr_mem.e_shoff);
+ CONVERT (elf->state.elf32.ehdr_mem.e_flags);
+ CONVERT (elf->state.elf32.ehdr_mem.e_ehsize);
+ CONVERT (elf->state.elf32.ehdr_mem.e_phentsize);
+ CONVERT (elf->state.elf32.ehdr_mem.e_phnum);
+ CONVERT (elf->state.elf32.ehdr_mem.e_shentsize);
+ CONVERT (elf->state.elf32.ehdr_mem.e_shnum);
+ CONVERT (elf->state.elf32.ehdr_mem.e_shstrndx);
+ }
+ }
+
+ /* Don't precache the phdr pointer here.
+ elf32_getphdr will validate it against the size when asked. */
- if (unlikely (ehdr->e_shoff >= maxsize)
- || unlikely (maxsize - ehdr->e_shoff
+ Elf32_Off e_shoff = elf->state.elf32.ehdr->e_shoff;
+ if (map_address != NULL && e_ident[EI_DATA] == MY_ELFDATA
+ && (ALLOW_UNALIGNED
+ || (((uintptr_t) ((char *) ehdr + e_shoff)
+ & (__alignof__ (Elf32_Shdr) - 1)) == 0)))
+ {
+ if (unlikely (e_shoff >= maxsize)
+ || unlikely (maxsize - e_shoff
< scncnt * sizeof (Elf32_Shdr)))
{
free_and_out:
@@ -325,10 +354,7 @@ file_read_elf (int fildes, void *map_address, unsigned char *e_ident,
return NULL;
}
elf->state.elf32.shdr
- = (Elf32_Shdr *) ((char *) ehdr + ehdr->e_shoff);
-
- /* Don't precache the phdr pointer here.
- elf32_getphdr will validate it against the size when asked. */
+ = (Elf32_Shdr *) ((char *) ehdr + e_shoff);
for (size_t cnt = 0; cnt < scncnt; ++cnt)
{
@@ -361,27 +387,6 @@ file_read_elf (int fildes, void *map_address, unsigned char *e_ident,
}
else
{
- /* Copy the ELF header. */
- elf->state.elf32.ehdr = memcpy (&elf->state.elf32.ehdr_mem, e_ident,
- sizeof (Elf32_Ehdr));
-
- if (e_ident[EI_DATA] != MY_ELFDATA)
- {
- CONVERT (elf->state.elf32.ehdr_mem.e_type);
- CONVERT (elf->state.elf32.ehdr_mem.e_machine);
- CONVERT (elf->state.elf32.ehdr_mem.e_version);
- CONVERT (elf->state.elf32.ehdr_mem.e_entry);
- CONVERT (elf->state.elf32.ehdr_mem.e_phoff);
- CONVERT (elf->state.elf32.ehdr_mem.e_shoff);
- CONVERT (elf->state.elf32.ehdr_mem.e_flags);
- CONVERT (elf->state.elf32.ehdr_mem.e_ehsize);
- CONVERT (elf->state.elf32.ehdr_mem.e_phentsize);
- CONVERT (elf->state.elf32.ehdr_mem.e_phnum);
- CONVERT (elf->state.elf32.ehdr_mem.e_shentsize);
- CONVERT (elf->state.elf32.ehdr_mem.e_shnum);
- CONVERT (elf->state.elf32.ehdr_mem.e_shstrndx);
- }
-
for (size_t cnt = 0; cnt < scncnt; ++cnt)
{
elf->state.elf32.scns.data[cnt].index = cnt;
@@ -402,24 +407,50 @@ file_read_elf (int fildes, void *map_address, unsigned char *e_ident,
/* This is a 64-bit binary. */
if (map_address != NULL && e_ident[EI_DATA] == MY_ELFDATA
&& (ALLOW_UNALIGNED
- || ((((uintptr_t) ehdr) & (__alignof__ (Elf64_Ehdr) - 1)) == 0
- && ((uintptr_t) ((char *) ehdr + ehdr->e_shoff)
- & (__alignof__ (Elf64_Shdr) - 1)) == 0
- && ((uintptr_t) ((char *) ehdr + ehdr->e_phoff)
- & (__alignof__ (Elf64_Phdr) - 1)) == 0)))
+ || (((uintptr_t) ehdr) & (__alignof__ (Elf64_Ehdr) - 1)) == 0))
{
/* We can use the mmapped memory. */
elf->state.elf64.ehdr = ehdr;
+ }
+ else
+ {
+ /* Copy the ELF header. */
+ elf->state.elf64.ehdr = memcpy (&elf->state.elf64.ehdr_mem, e_ident,
+ sizeof (Elf64_Ehdr));
+
+ if (e_ident[EI_DATA] != MY_ELFDATA)
+ {
+ CONVERT (elf->state.elf64.ehdr_mem.e_type);
+ CONVERT (elf->state.elf64.ehdr_mem.e_machine);
+ CONVERT (elf->state.elf64.ehdr_mem.e_version);
+ CONVERT (elf->state.elf64.ehdr_mem.e_entry);
+ CONVERT (elf->state.elf64.ehdr_mem.e_phoff);
+ CONVERT (elf->state.elf64.ehdr_mem.e_shoff);
+ CONVERT (elf->state.elf64.ehdr_mem.e_flags);
+ CONVERT (elf->state.elf64.ehdr_mem.e_ehsize);
+ CONVERT (elf->state.elf64.ehdr_mem.e_phentsize);
+ CONVERT (elf->state.elf64.ehdr_mem.e_phnum);
+ CONVERT (elf->state.elf64.ehdr_mem.e_shentsize);
+ CONVERT (elf->state.elf64.ehdr_mem.e_shnum);
+ CONVERT (elf->state.elf64.ehdr_mem.e_shstrndx);
+ }
+ }
+
+ /* Don't precache the phdr pointer here.
+ elf64_getphdr will validate it against the size when asked. */
- if (unlikely (ehdr->e_shoff >= maxsize)
- || unlikely (maxsize - ehdr->e_shoff
+ Elf64_Off e_shoff = elf->state.elf64.ehdr->e_shoff;
+ if (map_address != NULL && e_ident[EI_DATA] == MY_ELFDATA
+ && (ALLOW_UNALIGNED
+ || (((uintptr_t) ((char *) ehdr + e_shoff)
+ & (__alignof__ (Elf64_Shdr) - 1)) == 0)))
+ {
+ if (unlikely (e_shoff >= maxsize)
+ || unlikely (maxsize - e_shoff
< scncnt * sizeof (Elf64_Shdr)))
goto free_and_out;
elf->state.elf64.shdr
- = (Elf64_Shdr *) ((char *) ehdr + ehdr->e_shoff);
-
- /* Don't precache the phdr pointer here.
- elf64_getphdr will validate it against the size when asked. */
+ = (Elf64_Shdr *) ((char *) ehdr + e_shoff);
for (size_t cnt = 0; cnt < scncnt; ++cnt)
{
@@ -452,27 +483,6 @@ file_read_elf (int fildes, void *map_address, unsigned char *e_ident,
}
else
{
- /* Copy the ELF header. */
- elf->state.elf64.ehdr = memcpy (&elf->state.elf64.ehdr_mem, e_ident,
- sizeof (Elf64_Ehdr));
-
- if (e_ident[EI_DATA] != MY_ELFDATA)
- {
- CONVERT (elf->state.elf64.ehdr_mem.e_type);
- CONVERT (elf->state.elf64.ehdr_mem.e_machine);
- CONVERT (elf->state.elf64.ehdr_mem.e_version);
- CONVERT (elf->state.elf64.ehdr_mem.e_entry);
- CONVERT (elf->state.elf64.ehdr_mem.e_phoff);
- CONVERT (elf->state.elf64.ehdr_mem.e_shoff);
- CONVERT (elf->state.elf64.ehdr_mem.e_flags);
- CONVERT (elf->state.elf64.ehdr_mem.e_ehsize);
- CONVERT (elf->state.elf64.ehdr_mem.e_phentsize);
- CONVERT (elf->state.elf64.ehdr_mem.e_phnum);
- CONVERT (elf->state.elf64.ehdr_mem.e_shentsize);
- CONVERT (elf->state.elf64.ehdr_mem.e_shnum);
- CONVERT (elf->state.elf64.ehdr_mem.e_shstrndx);
- }
-
for (size_t cnt = 0; cnt < scncnt; ++cnt)
{
elf->state.elf64.scns.data[cnt].index = cnt;
--
1.8.3.1