Bug#820334: Segfaults caused by new DT_MIPS_RLD_MAP_REL tag and RPATH removers
Package: src:cmake Version: 3.0.2-1 Hi, I've managed to find the cause of the openmpi segfault (#818909). It might affect a number of different packages. The segfault is caused by the interaction of the new DT_MIPS_RLD_MAP_REL dynamic tag (from binutils 2.26) and chrpath. Unlike all other tags, this tag is relative to the offset of the tag within the executable. chrpath is used to remove rpaths from ELF files. It does this by moving all of the other dynamic tags up one entry, but since the DT_MIPS_RLD_MAP_REL is not updated, it now points to an incorrect offset. The dynamic linker will then overwrite some other memory when processing the DT_MIPS_RLD_MAP_REL tag. The openmpi segfault was caused by a global variable being initialized incorrectly (overwritten by the dynamic linker). I expect other executables using chrpath will also be affected - possibly in strange ways (not nessesarily a segfault). It also seems that at least cmake uses the same technique for removing the RPATH so any cmake reverse dependencies could be affected. The DT_MIPS_RLD_MAP_REL is only created for executables which limits the effect of this slightly. Only packages built using binutils >= 2.25.51.20151014-1 will be affected. There is a convinient way to test if a package is broken using the presence of the old DT_MIPS_RLD_MAP tag. When correct (DT_MIPS_RLD_MAP_REL + tag offset + executable base address) equals DT_MIPS_RLD_MAP, so someone could analyze the archive to find which packages are affected (any if any tools other than chrpath and cmake are broken). Based only on chrpath and cmake reverse dependencies, there is an upper bound of about 1500 binNMUs (after the tools after fixed). Hopefully that can be reduced! I really don't have any time to fix all this. Please can someone else have a look! OpenMPI maintainers (and anyone else affected): One possible workaround is to use chrpath -r "" on mips* architectures until this is fixed since that command does not cause any tags to be moved. It has a tiny performance penalty but should otherwise work properly. James signature.asc Description: PGP signature
Bug#820334: Segfaults caused by new DT_MIPS_RLD_MAP_REL tag and RPATH removers
Thanks to aurel32@d.o chrpath has been fixed. Could an equivalent
patch be included in cmake ?
On 2016-04-07 15:46, Mathieu Malaterre wrote:
> Package: src:chrpath
> Version: 0.16-1
>
> Hi,
>
> I've managed to find the cause of the openmpi segfault (#818909). It
> might affect a number of different packages.
>
> The segfault is caused by the interaction of the
> new DT_MIPS_RLD_MAP_REL dynamic tag (from binutils 2.26) and chrpath.
> Unlike all other tags, this tag is relative to the offset of the tag
> within the executable. chrpath is used to remove rpaths from ELF files.
> It does this by moving all of the other dynamic tags up one entry, but
> since the DT_MIPS_RLD_MAP_REL is not updated, it now points to an
> incorrect offset. The dynamic linker will then overwrite some other
> memory when processing the DT_MIPS_RLD_MAP_REL tag.
Please find below a patch to correctly handle this tag in chrpath. If
you are fine with the patch, a quick upload would be appreciated as
chrpath currently generates broken binaries. Thanks!
Aurelien
--- chrpath-0.16.orig/killrpath.c
+++ chrpath-0.16/killrpath.c
@@ -73,10 +73,26 @@
dynpos = 0;
for (i = 0; DYNSS(i, d_tag) != DT_NULL; i++)
{
- if (is_e32())
+ if (is_e32()) {
((Elf32_Dyn *)dyns)[dynpos] = ((Elf32_Dyn *)dyns)[i];
- else
+#ifdef DT_MIPS_RLD_MAP_REL
+/* DT_MIPS_RLD_MAP_REL is relative to the offset of the tag.
+ Adjust it consequently. */
+if (DYNSS(i, d_tag) == DT_MIPS_RLD_MAP_REL)
+ ((Elf32_Dyn *)dyns)[dynpos].d_un.d_val =
+ DO_SWAPU32(DYNSU(i, d_un.d_val) +
+ (i - dynpos) * sizeof(Elf32_Dyn));
+#endif
+ } else {
((Elf64_Dyn *)dyns)[dynpos] = ((Elf64_Dyn *)dyns)[i];
+#ifdef DT_MIPS_RLD_MAP_REL
+/* Ditto */
+if (DYNSS(i, d_tag) == DT_MIPS_RLD_MAP_REL)
+ ((Elf64_Dyn *)dyns)[dynpos].d_un.d_val =
+ DO_SWAPU64(DYNSU(i, d_un.d_val) +
+ (i - dynpos) * sizeof(Elf64_Dyn));
+#endif
+ }
if ( ! elf_dynpath_tag(DYNSS(i, d_tag)) )
dynpos++;
}
--
Aurelien Jarno GPG: 4096R/1DDD8C9B
aurel...@aurel32.net http://www.aurel32.net
Bug#820334: Segfaults caused by new DT_MIPS_RLD_MAP_REL tag and RPATH removers
On 2016-04-09 13:22, Mathieu Malaterre wrote: > Thanks to aurel32@d.o chrpath has been fixed. Could an equivalent > patch be included in cmake ? You mean that cmake is not calling chrpath, but instead has an embedded copy? In that case I'll look at that later today. -- Aurelien Jarno GPG: 4096R/1DDD8C9B aurel...@aurel32.net http://www.aurel32.net
Bug#820334: Segfaults caused by new DT_MIPS_RLD_MAP_REL tag and RPATH removers
Salut Aurélien, On Sat, Apr 9, 2016 at 2:14 PM, Aurelien Jarno wrote: > On 2016-04-09 13:22, Mathieu Malaterre wrote: >> Thanks to aurel32@d.o chrpath has been fixed. Could an equivalent >> patch be included in cmake ? > > You mean that cmake is not calling chrpath, but instead has an embedded > copy? In that case I'll look at that later today. That is how I understand the code in cmSystemTools::RemoveRPath (it should hopefully be very close to chrpath implementation). Brad, is this the only location that needs to be fixed ? Thanks
Bug#820334: Segfaults caused by new DT_MIPS_RLD_MAP_REL tag and RPATH removers
On 04/09/2016 11:01 AM, Mathieu Malaterre wrote: > On Sat, Apr 9, 2016 at 2:14 PM, Aurelien Jarno wrote: >> You mean that cmake is not calling chrpath, but instead has an embedded >> copy? In that case I'll look at that later today. Yes, CMake has a separate implementation. It is not a copy of chrpath but its own implementation, and was created because (at least at the time) chrpath did not support changing cross-compiled binaries. > That is how I understand the code in cmSystemTools::RemoveRPath (it > should hopefully be very close to chrpath implementation). > > Brad, is this the only location that needs to be fixed ? Yes, IIRC. -Brad
Bug#820334: Segfaults caused by new DT_MIPS_RLD_MAP_REL tag and RPATH removers
Control: forwarded -1 https://gitlab.kitware.com/cmake/cmake/merge_requests/147 Control: tags -1 sid stretch Hi, I've submitted the above PR upstream which should fix this issue. The patch was a bit bigger than what I would have liked due to the cmELF class requiring some changes. Tagging for stretch since jessie contains binutils 2.25 which is unaffected. Thanks, James signature.asc Description: OpenPGP digital signature
Bug#820334: Segfaults caused by new DT_MIPS_RLD_MAP_REL tag and RPATH removers
Control: tags -1 patch fixed-upstream
Hi,
On 06/10/16 09:42, James Cowgill wrote:
> Control: forwarded -1
> https://gitlab.kitware.com/cmake/cmake/merge_requests/147
> Control: tags -1 sid stretch
>
> Hi,
>
> I've submitted the above PR upstream which should fix this issue. The
> patch was a bit bigger than what I would have liked due to the cmELF
> class requiring some changes.
The PR was accepted and I've attached a patch which backports it to 3.6.
Thanks,
James
From c51099bb1018191964e5ee05e17f133233160806 Mon Sep 17 00:00:00 2001
From: James Cowgill
Date: Thu, 13 Oct 2016 11:00:00 +0100
Subject: [PATCH] MIPS: Handle DT_MIPS_RLD_MAP_REL tag when removing RPath from
ELFs
This is a combination of 5 commits.
elf: remove tag switch from ELF_Dyn ByteSwap function
elf: add DynamicEntryList methods and rpath tag constants
cmSystemTools: rewrite RemoveRPath using DyanmicEntryList methods
cmSystemTools, elf: handle DT_MIPS_RLD_REL_MAP in RemoveRPath
elf: Remove GetDynamicEntryCount and ReadBytes methods
Backported from upstream 3.8.0 commit:
ea563a27a2042cfb3be33d0f74efecc7687b86bb
---
Source/cmELF.cxx | 225 ---
Source/cmELF.h | 24 +++--
Source/cmSystemTools.cxx | 72 ---
3 files changed, 135 insertions(+), 186 deletions(-)
diff --git a/Source/cmELF.cxx b/Source/cmELF.cxx
index 26f1a44..9fe8a43 100644
--- a/Source/cmELF.cxx
+++ b/Source/cmELF.cxx
@@ -134,18 +134,13 @@ public:
// Forward to the per-class implementation.
virtual unsigned int GetNumberOfSections() const = 0;
- virtual unsigned int GetDynamicEntryCount() = 0;
virtual unsigned long GetDynamicEntryPosition(int j) = 0;
+ virtual cmELF::DynamicEntryList GetDynamicEntries() = 0;
+ virtual std::vector EncodeDynamicEntries(
+const cmELF::DynamicEntryList&) = 0;
virtual StringEntry const* GetDynamicSectionString(unsigned int tag) = 0;
virtual void PrintInfo(std::ostream& os) const = 0;
- bool ReadBytes(unsigned long pos, unsigned long size, char* buf)
- {
-this->Stream.seekg(pos);
-this->Stream.read(buf, size);
-return this->Stream ? true : false;
- }
-
// Lookup the SONAME in the DYNAMIC section.
StringEntry const* GetSOName()
{
@@ -246,10 +241,12 @@ public:
return static_cast(this->ELFHeader.e_shnum);
}
- // Get the file position and size of a dynamic section entry.
- virtual unsigned int GetDynamicEntryCount();
+ // Get the file position of a dynamic section entry.
virtual unsigned long GetDynamicEntryPosition(int j);
+ virtual cmELF::DynamicEntryList GetDynamicEntries();
+ virtual std::vector EncodeDynamicEntries(const cmELF::DynamicEntryList&);
+
// Lookup a string from the dynamic section with the given tag.
virtual StringEntry const* GetDynamicSectionString(unsigned int tag);
@@ -289,6 +286,10 @@ public:
}
private:
+ // ByteSwap(ELF_Dyn) assumes d_val and d_ptr are the same size
+ typedef char dyn_size_assert
+[sizeof(ELF_Dyn().d_un.d_val) == sizeof(ELF_Dyn().d_un.d_ptr) ? 1 : -1];
+
void ByteSwap(ELF_Ehdr& elf_header)
{
cmELFByteSwap(elf_header.e_type);
@@ -323,121 +324,7 @@ private:
void ByteSwap(ELF_Dyn& dyn)
{
cmELFByteSwap(dyn.d_tag);
-switch (dyn.d_tag) {
- case DT_NULL: /* dyn.d_un ignored */
-break;
- case DT_NEEDED:
-cmELFByteSwap(dyn.d_un.d_val);
-break;
- case DT_PLTRELSZ:
-cmELFByteSwap(dyn.d_un.d_val);
-break;
- case DT_PLTGOT:
-cmELFByteSwap(dyn.d_un.d_ptr);
-break;
- case DT_HASH:
-cmELFByteSwap(dyn.d_un.d_ptr);
-break;
- case DT_STRTAB:
-cmELFByteSwap(dyn.d_un.d_ptr);
-break;
- case DT_SYMTAB:
-cmELFByteSwap(dyn.d_un.d_ptr);
-break;
- case DT_RELA:
-cmELFByteSwap(dyn.d_un.d_ptr);
-break;
- case DT_RELASZ:
-cmELFByteSwap(dyn.d_un.d_val);
-break;
- case DT_RELAENT:
-cmELFByteSwap(dyn.d_un.d_val);
-break;
- case DT_STRSZ:
-cmELFByteSwap(dyn.d_un.d_val);
-break;
- case DT_SYMENT:
-cmELFByteSwap(dyn.d_un.d_val);
-break;
- case DT_INIT:
-cmELFByteSwap(dyn.d_un.d_ptr);
-break;
- case DT_FINI:
-cmELFByteSwap(dyn.d_un.d_ptr);
-break;
- case DT_SONAME:
-cmELFByteSwap(dyn.d_un.d_val);
-break;
- case DT_RPATH:
-cmELFByteSwap(dyn.d_un.d_val);
-break;
- case DT_SYMBOLIC: /* dyn.d_un ignored */
-break;
- case DT_REL:
-cmELFByteSwap(dyn.d_un.d_ptr);
-break;
- case DT_RELSZ:
-cmELFByteSwap(dyn.d_un.d_val);
-break;
- case DT_RELENT:
-cmELFByteSwap(dyn.d_un.d_val);
-break;
- case DT_PLTREL:
-cmELFByteSwap(dyn.d_un.d_val);
-break;
- case DT_DEBUG:
-cmELFByteSwap(dyn.d_un.d_ptr);
-break;
- case DT_TEXTREL: /*
