From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 37291 invoked by alias); 23 Nov 2018 20:20:10 -0000 Mailing-List: contact elfutils-devel-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Post: List-Help: List-Subscribe: Sender: elfutils-devel-owner@sourceware.org Received: (qmail 37275 invoked by uid 89); 23 Nov 2018 20:20:09 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Checked: by ClamAV 0.100.2 on sourceware.org X-Virus-Found: No X-Spam-SWARE-Status: No, score=-26.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,SPF_PASS autolearn=ham version=3.3.2 spammy= X-Spam-Status: No, score=-26.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,SPF_PASS autolearn=ham version=3.3.2 X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on sourceware.org X-Spam-Level: X-HELO: gnu.wildebeest.org Received: from wildebeest.demon.nl (HELO gnu.wildebeest.org) (212.238.236.112) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 23 Nov 2018 20:20:08 +0000 Received: from tarox.wildebeest.org (tarox.wildebeest.org [172.31.17.39]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by gnu.wildebeest.org (Postfix) with ESMTPSA id CCFF730008B8; Fri, 23 Nov 2018 21:20:05 +0100 (CET) Received: by tarox.wildebeest.org (Postfix, from userid 1000) id B98D8403F6EC; Fri, 23 Nov 2018 21:20:05 +0100 (CET) From: Mark Wielaard To: elfutils-devel@sourceware.org Cc: Mark Wielaard Subject: [PATCH] libdwfl: Fix relocation overlap sanity check. Date: Fri, 23 Nov 2018 20:20:00 -0000 Message-Id: <1543004401-15437-1-git-send-email-mark@klomp.org> X-Mailer: git-send-email 1.8.3.1 X-Spam-Flag: NO X-IsSubscribed: yes X-SW-Source: 2018-q4/txt/msg00175.txt.bz2 We would not relocate when the relocation section data or the target section date would overlap with one of the ELF headers. This is only really necessary if the data comes directly from the mmapped file. Otherwise there is no real overlap and the relocations can be safely applied. One particular thing we got wrong with the original sanity check was when the relocation data or target data section was compressed. In that case it could happen we overestimated the size (because the Shdr would have been updated to show the uncompressed data size). But uncompressed data is always malloced and so cannot overlap with the mmapped Elf header structures. When building with CFLAGS="-g -Og" this showed up as a failure in run-strip-reloc.sh for strip-compressed.o. Where the .debug_loc section decompressed would "overlap" with the shdrs at the end of the file and so wouldn't get relocations applied. Signed-off-by: Mark Wielaard --- The diff might look big, but without the whitespace changes it is just: @@ -561,7 +562,13 @@ relocate_section (Dwfl_Module *mod, Elf *relocated, const GElf_Ehdr *ehdr, shdrs or phdrs data then we refuse to do the relocations. It isn't illegal for ELF section data to overlap the header data, but updating the (relocation) data might corrupt the in-memory - libelf headers causing strange corruptions or errors. */ + libelf headers causing strange corruptions or errors. + + This is only an issue if the ELF is mmapped and the section data + comes from the mmapped region (is not malloced or decompressed). + */ + if (relocated->map_address != NULL) + { size_t ehsize = gelf_fsize (relocated, ELF_T_EHDR, 1, EV_CURRENT); if (unlikely (shdr->sh_offset < ehsize || tshdr->sh_offset < ehsize)) @@ -574,10 +581,14 @@ relocate_section (Dwfl_Module *mod, Elf *relocated, const GElf_Ehdr *ehdr, /* Overflows will have been checked by elf_getshdrnum/get|rawdata. */ size_t shentsize = gelf_fsize (relocated, ELF_T_SHDR, 1, EV_CURRENT); GElf_Off shdrs_end = shdrs_start + shnums * shentsize; - if (unlikely ((shdrs_start < shdr->sh_offset + shdr->sh_size - && shdr->sh_offset < shdrs_end) - || (shdrs_start < tshdr->sh_offset + tshdr->sh_size - && tshdr->sh_offset < shdrs_end))) + if (unlikely (shdrs_start < shdr->sh_offset + shdr->sh_size + && shdr->sh_offset < shdrs_end)) + if ((scn->flags & ELF_F_MALLOCED) == 0) + return DWFL_E_BADELF; + + if (unlikely (shdrs_start < tshdr->sh_offset + tshdr->sh_size + && tshdr->sh_offset < shdrs_end)) + if ((tscn->flags & ELF_F_MALLOCED) == 0) return DWFL_E_BADELF; GElf_Off phdrs_start = ehdr->e_phoff; @@ -589,11 +600,16 @@ relocate_section (Dwfl_Module *mod, Elf *relocated, const GElf_Ehdr *ehdr, /* Overflows will have been checked by elf_getphdrnum/get|rawdata. */ size_t phentsize = gelf_fsize (relocated, ELF_T_PHDR, 1, EV_CURRENT); GElf_Off phdrs_end = phdrs_start + phnums * phentsize; - if (unlikely ((phdrs_start < shdr->sh_offset + shdr->sh_size - && shdr->sh_offset < phdrs_end) - || (phdrs_start < tshdr->sh_offset + tshdr->sh_size - && tshdr->sh_offset < phdrs_end))) + if (unlikely (phdrs_start < shdr->sh_offset + shdr->sh_size + && shdr->sh_offset < phdrs_end)) + if ((scn->flags & ELF_F_MALLOCED) == 0) return DWFL_E_BADELF; + + if (unlikely (phdrs_start < tshdr->sh_offset + tshdr->sh_size + && tshdr->sh_offset < phdrs_end)) + if ((tscn->flags & ELF_F_MALLOCED) == 0) + return DWFL_E_BADELF; + } } /* Fetch the relocation section and apply each reloc in it. */ --- libdwfl/ChangeLog | 5 ++++ libdwfl/relocate.c | 80 ++++++++++++++++++++++++++++++++---------------------- 2 files changed, 53 insertions(+), 32 deletions(-) diff --git a/libdwfl/ChangeLog b/libdwfl/ChangeLog index 45cc1b4..b96cebf 100644 --- a/libdwfl/ChangeLog +++ b/libdwfl/ChangeLog @@ -1,3 +1,8 @@ +2018-10-23 Mark Wielaard + + * relocate.c (relocate_section): Only sanity check mmapped Elf files + for overlap. Don't error when section data was malloced, not mmapped. + 2018-10-20 Mark Wielaard * libdwflP.h (__libdw_open_elf): New internal function declaration. diff --git a/libdwfl/relocate.c b/libdwfl/relocate.c index 58c5678..88b5211 100644 --- a/libdwfl/relocate.c +++ b/libdwfl/relocate.c @@ -1,5 +1,5 @@ /* Relocate debug information. - Copyright (C) 2005-2011, 2014, 2016 Red Hat, Inc. + Copyright (C) 2005-2011, 2014, 2016, 2018 Red Hat, Inc. This file is part of elfutils. This file is free software; you can redistribute it and/or modify @@ -30,6 +30,7 @@ # include #endif +#include "libelfP.h" #include "libdwflP.h" typedef uint8_t GElf_Byte; @@ -561,39 +562,54 @@ relocate_section (Dwfl_Module *mod, Elf *relocated, const GElf_Ehdr *ehdr, shdrs or phdrs data then we refuse to do the relocations. It isn't illegal for ELF section data to overlap the header data, but updating the (relocation) data might corrupt the in-memory - libelf headers causing strange corruptions or errors. */ - size_t ehsize = gelf_fsize (relocated, ELF_T_EHDR, 1, EV_CURRENT); - if (unlikely (shdr->sh_offset < ehsize - || tshdr->sh_offset < ehsize)) - return DWFL_E_BADELF; - - GElf_Off shdrs_start = ehdr->e_shoff; - size_t shnums; - if (elf_getshdrnum (relocated, &shnums) < 0) - return DWFL_E_LIBELF; - /* Overflows will have been checked by elf_getshdrnum/get|rawdata. */ - size_t shentsize = gelf_fsize (relocated, ELF_T_SHDR, 1, EV_CURRENT); - GElf_Off shdrs_end = shdrs_start + shnums * shentsize; - if (unlikely ((shdrs_start < shdr->sh_offset + shdr->sh_size - && shdr->sh_offset < shdrs_end) - || (shdrs_start < tshdr->sh_offset + tshdr->sh_size - && tshdr->sh_offset < shdrs_end))) - return DWFL_E_BADELF; - - GElf_Off phdrs_start = ehdr->e_phoff; - size_t phnums; - if (elf_getphdrnum (relocated, &phnums) < 0) - return DWFL_E_LIBELF; - if (phdrs_start != 0 && phnums != 0) + libelf headers causing strange corruptions or errors. + + This is only an issue if the ELF is mmapped and the section data + comes from the mmapped region (is not malloced or decompressed). + */ + if (relocated->map_address != NULL) { - /* Overflows will have been checked by elf_getphdrnum/get|rawdata. */ - size_t phentsize = gelf_fsize (relocated, ELF_T_PHDR, 1, EV_CURRENT); - GElf_Off phdrs_end = phdrs_start + phnums * phentsize; - if (unlikely ((phdrs_start < shdr->sh_offset + shdr->sh_size - && shdr->sh_offset < phdrs_end) - || (phdrs_start < tshdr->sh_offset + tshdr->sh_size - && tshdr->sh_offset < phdrs_end))) + size_t ehsize = gelf_fsize (relocated, ELF_T_EHDR, 1, EV_CURRENT); + if (unlikely (shdr->sh_offset < ehsize + || tshdr->sh_offset < ehsize)) return DWFL_E_BADELF; + + GElf_Off shdrs_start = ehdr->e_shoff; + size_t shnums; + if (elf_getshdrnum (relocated, &shnums) < 0) + return DWFL_E_LIBELF; + /* Overflows will have been checked by elf_getshdrnum/get|rawdata. */ + size_t shentsize = gelf_fsize (relocated, ELF_T_SHDR, 1, EV_CURRENT); + GElf_Off shdrs_end = shdrs_start + shnums * shentsize; + if (unlikely (shdrs_start < shdr->sh_offset + shdr->sh_size + && shdr->sh_offset < shdrs_end)) + if ((scn->flags & ELF_F_MALLOCED) == 0) + return DWFL_E_BADELF; + + if (unlikely (shdrs_start < tshdr->sh_offset + tshdr->sh_size + && tshdr->sh_offset < shdrs_end)) + if ((tscn->flags & ELF_F_MALLOCED) == 0) + return DWFL_E_BADELF; + + GElf_Off phdrs_start = ehdr->e_phoff; + size_t phnums; + if (elf_getphdrnum (relocated, &phnums) < 0) + return DWFL_E_LIBELF; + if (phdrs_start != 0 && phnums != 0) + { + /* Overflows will have been checked by elf_getphdrnum/get|rawdata. */ + size_t phentsize = gelf_fsize (relocated, ELF_T_PHDR, 1, EV_CURRENT); + GElf_Off phdrs_end = phdrs_start + phnums * phentsize; + if (unlikely (phdrs_start < shdr->sh_offset + shdr->sh_size + && shdr->sh_offset < phdrs_end)) + if ((scn->flags & ELF_F_MALLOCED) == 0) + return DWFL_E_BADELF; + + if (unlikely (phdrs_start < tshdr->sh_offset + tshdr->sh_size + && tshdr->sh_offset < phdrs_end)) + if ((tscn->flags & ELF_F_MALLOCED) == 0) + return DWFL_E_BADELF; + } } /* Fetch the relocation section and apply each reloc in it. */ -- 1.8.3.1