[PATCH] libelf: Use offsetof to get field of unaligned
gcc undefined sanitizer flags: elf_begin.c:230:18: runtime error: member access within misaligned address 0xf796400a for type 'struct Elf64_Shdr', which requires 4 byte alignment struct. This seems a wrong warning since we aren't accessing the field member of the struct, but are taking the address of it. But we can do the same by adding the field offsetof to the base address. Which doesn't trigger a runtime error. Signed-off-by: Mark Wielaard --- libelf/ChangeLog | 5 + libelf/elf_begin.c | 15 +-- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/libelf/ChangeLog b/libelf/ChangeLog index 041da9b1..96059eff 100644 --- a/libelf/ChangeLog +++ b/libelf/ChangeLog @@ -1,3 +1,8 @@ +2021-12-15 Mark Wielaard + + * elf_begin.c (get_shnum): Use offsetof to get field of unaligned + struct. + 2021-09-06 Dmitry V. Levin * common.h (allocate_elf): Remove cast of calloc return value. diff --git a/libelf/elf_begin.c b/libelf/elf_begin.c index 93d1e12f..bd3399de 100644 --- a/libelf/elf_begin.c +++ b/libelf/elf_begin.c @@ -1,5 +1,6 @@ /* Create descriptor for processing file. Copyright (C) 1998-2010, 2012, 2014, 2015, 2016 Red Hat, Inc. + Copyright (C) 2021 Mark J. Wielaard This file is part of elfutils. Written by Ulrich Drepper , 1998. @@ -170,9 +171,10 @@ get_shnum (void *map_address, unsigned char *e_ident, int fildes, if (likely (map_address != NULL)) /* gcc will optimize the memcpy to a simple memory access while taking care of alignment issues. */ - memcpy (&size, &((Elf32_Shdr *) ((char *) map_address -+ ehdr.e32->e_shoff -+ offset))->sh_size, + memcpy (&size, ((char *) map_address ++ ehdr.e32->e_shoff ++ offset ++ offsetof (Elf32_Shdr, sh_size)), sizeof (Elf32_Word)); else if (unlikely ((r = pread_retry (fildes, &size, @@ -227,9 +229,10 @@ get_shnum (void *map_address, unsigned char *e_ident, int fildes, if (likely (map_address != NULL)) /* gcc will optimize the memcpy to a simple memory access while taking care of alignment issues. */ - memcpy (&size, &((Elf64_Shdr *) ((char *) map_address -+ ehdr.e64->e_shoff -+ offset))->sh_size, + memcpy (&size, ((char *) map_address ++ ehdr.e64->e_shoff ++ offset ++ offsetof (Elf64_Shdr, sh_size)), sizeof (Elf64_Xword)); else if (unlikely ((r = pread_retry (fildes, &size, -- 2.30.2
Re: [PATCH] libelf: Use offsetof to get field of unaligned
* Mark Wielaard: > This seems a wrong warning since we aren't accessing the field member > of the struct, but are taking the address of it. But we can do the > same by adding the field offsetof to the base address. Which doesn't > trigger a runtime error. I think the warning is correct. I believe it is motivated by the GCC optimizers using this to infer alignment of the original pointer. It won't make a difference for this expression, but it can cause crashes elsewhere with strict-alignment targets. Thanks, Florian
[PATCH] libdwfl: Make sure phent is sane and there is at least one phdr
dwfl_link_map_report can only handle program headers that are the
correct (32 or 64 bit) size. The buffer read in needs to contain room
for at least one Phdr.
https://sourceware.org/bugzilla/show_bug.cgi?id=28660
Signed-off-by: Mark Wielaard
---
libdwfl/ChangeLog | 6 ++
libdwfl/link_map.c | 16 ++--
2 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/libdwfl/ChangeLog b/libdwfl/ChangeLog
index aaea164c..7bf789e0 100644
--- a/libdwfl/ChangeLog
+++ b/libdwfl/ChangeLog
@@ -1,3 +1,9 @@
+2021-12-15 Mark Wielaard
+
+ * link_map.c (dwfl_link_map_report): Make sure phent is either sizeof
+ Elf32_Phdr or sizeof Elf64_Phdr. Check in.d_size can hold at least one
+ Phdr.
+
2021-12-12 Mark Wielaard
* dwfl_segment_report_module.c (dwfl_segment_report_module): Don't
diff --git a/libdwfl/link_map.c b/libdwfl/link_map.c
index ad93501e..82df7b69 100644
--- a/libdwfl/link_map.c
+++ b/libdwfl/link_map.c
@@ -1,5 +1,6 @@
/* Report modules by examining dynamic linker data structures.
Copyright (C) 2008-2016 Red Hat, Inc.
+ Copyright (C) 2021 Mark J. Wielaard
This file is part of elfutils.
This file is free software; you can redistribute it and/or modify
@@ -784,7 +785,9 @@ dwfl_link_map_report (Dwfl *dwfl, const void *auxv, size_t
auxv_size,
GElf_Xword dyn_filesz = 0;
GElf_Addr dyn_bias = (GElf_Addr) -1;
- if (phdr != 0 && phnum != 0 && phent != 0)
+ if (phdr != 0 && phnum != 0
+ && ((elfclass == ELFCLASS32 && phent == sizeof (Elf32_Phdr))
+ || (elfclass == ELFCLASS64 && phent == sizeof (Elf64_Phdr
{
Dwfl_Module *phdr_mod;
int phdr_segndx = INTUSE(dwfl_addrsegment) (dwfl, phdr, &phdr_mod);
@@ -904,7 +907,16 @@ dwfl_link_map_report (Dwfl *dwfl, const void *auxv, size_t
auxv_size,
.d_buf = buf
};
if (in.d_size > out.d_size)
- in.d_size = out.d_size;
+ {
+ in.d_size = out.d_size;
+ phnum = in.d_size / phent;
+ if (phnum == 0)
+ {
+ free (buf);
+ __libdwfl_seterrno (DWFL_E_BADELF);
+ return false;
+ }
+ }
if (likely ((elfclass == ELFCLASS32
? elf32_xlatetom : elf64_xlatetom)
(&out, &in, elfdata) != NULL))
--
2.30.2
[Bug libdw/28660] ASan seems to complain about a "heap-buffer-overflow"
https://sourceware.org/bugzilla/show_bug.cgi?id=28660 Mark Wielaard changed: What|Removed |Added Status|UNCONFIRMED |ASSIGNED Assignee|unassigned at sourceware dot org |mark at klomp dot org Last reconfirmed||2021-12-15 Ever confirmed|0 |1 --- Comment #5 from Mark Wielaard --- Proposed some more sanity checks: https://sourceware.org/pipermail/elfutils-devel/2021q4/004523.html -- You are receiving this mail because: You are on the CC list for the bug.
Re: [PATCH] debuginfod/debuginfod-client.c: use long for cache time configurations
Hi, On Thu, Dec 09, 2021 at 06:35:14AM -0500, Frank Ch. Eigler via Elfutils-devel wrote: > > x32, riscv32, arc use 64bit time_t even while they are 32bit > > architectures, therefore directly using integer printf formats will not > > work portably. > > > Use a plain long everywhere as the intervals are small enough > > that it will not be problematic. > > lgtm! Added ChangeLog entry and pushed. Thanks, Mark
[Bug libdw/28660] ASan seems to complain about a "heap-buffer-overflow"
https://sourceware.org/bugzilla/show_bug.cgi?id=28660 --- Comment #6 from Evgeny Vereshchagin --- Thanks! I can confirm that the issue is gone. I tested it in https://github.com/evverx/elfutils/pull/53 by adding that file to the testsuite in https://github.com/evverx/elfutils/pull/53/commits/38c241bf6269ab5a1dd93b606e11001dc3b6c1f4. I also ran the fuzzer for about 10 minutes. Interestingly, something started to trigger unreproducible MSan crashes but I'm inclined to say it was probably a fluke. -- You are receiving this mail because: You are on the CC list for the bug.
Buildbot failure in Wildebeest Builder on whole buildset
The Buildbot has detected a new failure on builder elfutils-centos-x86_64 while building elfutils. Full details are available at: https://builder.wildebeest.org/buildbot/#builders/1/builds/884 Buildbot URL: https://builder.wildebeest.org/buildbot/ Worker for this Build: centos-x86_64 Build Reason: Blamelist: Alexander Kanavin BUILD FAILED: failed test (failure) Sincerely, -The BuildbotThe Buildbot has detected a new failure on builder elfutils-fedora-x86_64 while building elfutils. Full details are available at: https://builder.wildebeest.org/buildbot/#builders/3/builds/876 Buildbot URL: https://builder.wildebeest.org/buildbot/ Worker for this Build: fedora-x86_64 Build Reason: Blamelist: Alexander Kanavin BUILD FAILED: failed test (failure) Sincerely, -The Buildbot
[Bug libdw/28660] ASan seems to complain about a "heap-buffer-overflow"
https://sourceware.org/bugzilla/show_bug.cgi?id=28660 --- Comment #7 from Evgeny Vereshchagin --- > Interestingly, something started to trigger unreproducible MSan crashes but > I'm inclined to say it was probably a fluke. It wasn't a glitch. The file I added to the test suite was also automatically added to the seed corpus, which in turn triggered new code paths and led to large allocations I reported elsewhere. So it doesn't have anything to do with this issue and the patch. -- You are receiving this mail because: You are on the CC list for the bug.
