From patchwork Mon Jun 8 00:54:42 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 3982 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id A55C56002E for ; Mon, 8 Jun 2020 02:55:06 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="jTvCwPL1"; dkim-atps=neutral Received: from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi [81.175.216.236]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 1EE772C9 for ; Mon, 8 Jun 2020 02:55:06 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1591577706; bh=Hdanr5Uk8mo+xaM/e+YgKFzsLPbznNo94NY1WTD375w=; h=From:To:Subject:Date:From; b=jTvCwPL1phaOjA+y5C10iDDb8vyG8gQyxWBPzQ5Tx8Qn5ZP0na9ZwTvpEgsxD3cu9 KWvEvXroFmH1VAXtBbT8OADXKzbTCpK6e9QaLmd6w0NFw/7gCJ5ZWtaPBidk4hPT0n ilpSpAdsMSfQQ78u9QnjVSV1zgv63DdUqayofY1g= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Mon, 8 Jun 2020 03:54:42 +0300 Message-Id: <20200608005442.1771-1-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.27.0 MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH] libcamera: ipa_module: Fix valgrind assertion failure X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 08 Jun 2020 00:55:07 -0000 As the ELF parsing code uses non-const pointers to the ELF mapping, we have to map the module in private read-write mode. This causes issues with valgrind, due to the IPA manager mapping the module in shared read-only mode and valgrind having trouble loading debugging symbols later at dlopen time due to conflicting mappings. This is likely a bug in valgrind (reported as [1]), but we can easily work around it by using shared read-only mappings only. As such a mapping shouldn't be less efficient than private read-write mappings, switch the mapping type. This requires modifying the ELF parsing functions to operate on const memory, which is a good idea anyway as they're not supposed to modify the ELF file. [1] https://bugs.kde.org/show_bug.cgi?id=422601 Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund Reviewed-by: Paul Elder --- src/libcamera/ipa_module.cpp | 42 ++++++++++++++++++++---------------- 1 file changed, 23 insertions(+), 19 deletions(-) diff --git a/src/libcamera/ipa_module.cpp b/src/libcamera/ipa_module.cpp index 72e357ec97ca..de512a7f3a0d 100644 --- a/src/libcamera/ipa_module.cpp +++ b/src/libcamera/ipa_module.cpp @@ -45,26 +45,27 @@ LOG_DEFINE_CATEGORY(IPAModule) namespace { template -typename std::remove_extent_t *elfPointer(Span elf, off_t offset, - size_t objSize) +typename std::remove_extent_t *elfPointer(Span elf, + off_t offset, size_t objSize) { size_t size = offset + objSize; if (size > elf.size() || size < objSize) return nullptr; return reinterpret_cast *> - (reinterpret_cast(elf.data()) + offset); + (reinterpret_cast(elf.data()) + offset); } template -typename std::remove_extent_t *elfPointer(Span elf, off_t offset) +typename std::remove_extent_t *elfPointer(Span elf, + off_t offset) { return elfPointer(elf, offset, sizeof(T)); } -int elfVerifyIdent(Span elf) +int elfVerifyIdent(Span elf) { - char *e_ident = elfPointer(elf, 0); + const char *e_ident = elfPointer(elf, 0); if (!e_ident) return -ENOEXEC; @@ -88,14 +89,15 @@ int elfVerifyIdent(Span elf) return 0; } -ElfW(Shdr) *elfSection(Span elf, ElfW(Ehdr) *eHdr, ElfW(Half) idx) +const ElfW(Shdr) *elfSection(Span elf, const ElfW(Ehdr) *eHdr, + ElfW(Half) idx) { if (idx >= eHdr->e_shnum) return nullptr; off_t offset = eHdr->e_shoff + idx * static_cast(eHdr->e_shentsize); - return elfPointer(elf, offset); + return elfPointer(elf, offset); } /** @@ -106,26 +108,26 @@ ElfW(Shdr) *elfSection(Span elf, ElfW(Ehdr) *eHdr, ElfW(Half) idx) * \return The memory region storing the symbol on success, or an empty span * otherwise */ -Span elfLoadSymbol(Span elf, const char *symbol) +Span elfLoadSymbol(Span elf, const char *symbol) { - ElfW(Ehdr) *eHdr = elfPointer(elf, 0); + const ElfW(Ehdr) *eHdr = elfPointer(elf, 0); if (!eHdr) return {}; - ElfW(Shdr) *sHdr = elfSection(elf, eHdr, eHdr->e_shstrndx); + const ElfW(Shdr) *sHdr = elfSection(elf, eHdr, eHdr->e_shstrndx); if (!sHdr) return {}; off_t shnameoff = sHdr->sh_offset; /* Locate .dynsym section header. */ - ElfW(Shdr) *dynsym = nullptr; + const ElfW(Shdr) *dynsym = nullptr; for (unsigned int i = 0; i < eHdr->e_shnum; i++) { sHdr = elfSection(elf, eHdr, i); if (!sHdr) return {}; off_t offset = shnameoff + sHdr->sh_name; - char *name = elfPointer(elf, offset); + const char *name = elfPointer(elf, offset); if (!name) return {}; @@ -146,16 +148,17 @@ Span elfLoadSymbol(Span elf, const char *symbol) off_t dynsym_nameoff = sHdr->sh_offset; /* Locate symbol in the .dynsym section. */ - ElfW(Sym) *targetSymbol = nullptr; + const ElfW(Sym) *targetSymbol = nullptr; unsigned int dynsym_num = dynsym->sh_size / dynsym->sh_entsize; for (unsigned int i = 0; i < dynsym_num; i++) { off_t offset = dynsym->sh_offset + dynsym->sh_entsize * i; - ElfW(Sym) *sym = elfPointer(elf, offset); + const ElfW(Sym) *sym = elfPointer(elf, offset); if (!sym) return {}; offset = dynsym_nameoff + sym->st_name; - char *name = elfPointer(elf, offset, strlen(symbol) + 1); + const char *name = elfPointer(elf, offset, + strlen(symbol) + 1); if (!name) return {}; @@ -176,7 +179,8 @@ Span elfLoadSymbol(Span elf, const char *symbol) if (!sHdr) return {}; off_t offset = sHdr->sh_offset + (targetSymbol->st_value - sHdr->sh_addr); - uint8_t *data = elfPointer(elf, offset, targetSymbol->st_size); + const uint8_t *data = elfPointer(elf, offset, + targetSymbol->st_size); if (!data) return {}; @@ -277,14 +281,14 @@ int IPAModule::loadIPAModuleInfo() return file.error(); } - Span data = file.map(0, -1, File::MapPrivate); + Span data = file.map(); int ret = elfVerifyIdent(data); if (ret) { LOG(IPAModule, Error) << "IPA module is not an ELF file"; return ret; } - Span info = elfLoadSymbol(data, "ipaModuleInfo"); + Span info = elfLoadSymbol(data, "ipaModuleInfo"); if (info.size() != sizeof(info_)) { LOG(IPAModule, Error) << "IPA module has no valid info"; return -EINVAL;