Hi,
I saw some curious crashes and broken ELF files in the Fedora builder
for really big packages (e.g. Qt5). It turned out that when libelf tries
to update or write out a 4GB+ files it used some 32bit offsets/types
where it should have been using 64bit offsets/types.
While searching for the bad coding pattern (using Elf64_Word as if it
was a 64bit type, it isn't!) I found that chromium probably is also
so big to hit this issue. There is even a somewhat similar fix!
https://chromium.googlesource.com/chromium/src/+/refs/heads/master/buildtools/third_party/eu-strip/fix-elf-size.patch
My apologies if you tried to upstream this and I missed it. But I think
the patch below is a more complete fix. If you could test it in your
setup that would be great.
The patch also contains a testcase. But since it is necessary to create
and process a 4GB+ file it is guarded by some checks to make sure the
machine has enough disk and memory. Do these checks look reasonable?
They probably prevent (make the test SKIP) on all buildbot CI workers.
Cheers,
Mark
-
Some years ago, in commit b1d0b0fc "libelf: Use int64_t for offsets in
libelf.h", we changed the public interface to use 64bit offsets/sizes.
This wasn't really a API change, before we relied on loff_t always
being 64bits on all platforms.
We didn't change the implementation to use the int64_t type though.
That was a little confusing, since the function definitions used a
different type, int64_t, from the function implementations, off_t.
Since we always build with _FILE_OFFSET_BITS=64 this should be fine.
But it was a bit sloppy and confusing.
Worse is that we got the translation of offset/sizes wrong in a
couple of places when translating to ELF types. In various places
we would use Elf32_Word or Elf64_Word. But both are 32bit (unsigned)
types! As is GElf_Word. Elf32_Off is 32bits and Elf64_Off is 64bits.
But we were not using those consistently.
This patch introduces comments for the usage of [G]Elf(32|64)Word in
libelf that are correct. And introduces Elf(32|64)_SizeWord in
elf32_updatenull.c where we want to make a difference between sizes
and offsets (the ELF variants are both unsigned, while int64_t/loff_t
is signed).
It also includes a new run-large-elf-file.sh test that creates a
large ELF files (one 64bit, little endian, rel and another big endian,
non-rel) and runs eu-strip, eu-elflint, eu-unstrip and eu-elfcmp.
Before this patch, that test case fails and creates corrupt ELF files.
The test is guarded by some checks that try to make sure there is
enough disk space and memory available on the machine. The test is
skipped otherwise.
Signed-off-by: Mark Wielaard
---
libelf/ChangeLog | 32 ++
libelf/common.h | 2 +-
libelf/elf32_newphdr.c| 3 ++
libelf/elf32_updatefile.c | 8 ++--
libelf/elf32_updatenull.c | 34 ---
libelf/elf_begin.c| 15 ---
libelf/elf_getaroff.c | 2 +-
libelf/elf_getbase.c | 4 +-
libelf/elf_getdata_rawchunk.c | 2 +-
libelf/elf_update.c | 8 ++--
libelf/libelfP.h | 18
tests/ChangeLog | 9
tests/Makefile.am | 2 +
tests/addsections.c | 44 +++
tests/run-large-elf-file.sh | 99 +++
15 files changed, 230 insertions(+), 52 deletions(-)
create mode 100755 tests/run-large-elf-file.sh
diff --git a/libelf/ChangeLog b/libelf/ChangeLog
index 82e18eb..dde6c81 100644
--- a/libelf/ChangeLog
+++ b/libelf/ChangeLog
@@ -1,3 +1,35 @@
+2019-06-18 Mark Wielaard
+
+ * common.h (allocate_elf): Use int64_t instead of off_t for offset.
+ * elf32_newphdr.c (newphdr): Document why Elf32/64_Word is correct.
+ * elf32_updatefile.c (fill): Use int64_t instead of off_t for pos.
+ (updatefile): Define last_offset, shdr_offset and scn_start as
+ int64_t instead of off_t.
+ * elf32_updatenull.c: Define Elf32_SizeWord and Elf64_SizeWord.
+ (updatenull_wrlock): Return int64_t instead of off_t. Define size,
+ sh_entsize, sh_align and sh_size as SizeWords. Define offset as
+ int64_t. Cast data->d_off to SizeWord instead of GElf_Word. Drop
+ size GElf_Word cast. Cast offset to SizeWord instead of GElf_Word
+ when comparing with sh_size.
+ * elf_begin.c (get_shnum): Define offset as int64_t instead of
+ off_t. Document why use GElf_Word is correct.
+ (file_read_elf): Define offset as int64_t instead of off_t.
+ (__libelf_read_mmaped_file): Likewise.
+ (read_unmmaped_file): Likewise.
+ (read_file): Likewise.
+ * elf_getaroff.c (elf_getaroff): Return int64_t.
+ * elf_getbase.c (elf_getbase): Likewise.
+ * elf_getdata_rawchunk.c (elf_getdata_rawchunk): Define offset as
+ int64_t instead of off_t.
+ * elf_update.c (write_file): Return int64_t instead of off_t,
+ define size as int64_t instead
