[PATCH] libelf: Only set shdr state when there is at least one shdr
The elf shdr state only needs to be set when scncnt is at least
one. Otherwise e_shoff can be bogus. Also use unsigned arithmetic for
checking e_shoff alignment.
Signed-off-by: Mark Wielaard
---
libelf/ChangeLog | 5 +
libelf/elf_begin.c | 16 ++--
2 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/libelf/ChangeLog b/libelf/ChangeLog
index 617d97a5..29a8aae1 100644
--- a/libelf/ChangeLog
+++ b/libelf/ChangeLog
@@ -1,3 +1,8 @@
+2021-12-19 Mark Wielaard
+
+ * elf_begin.c (file_read_elf): Cast ehdr to uintptr_t before e_shoff
+ alignment check. Only set shdr state when scncnt is larger than zero.
+
2021-12-16 Mark Wielaard
* libelfP.h (NOTE_ALIGN4): And with negative unsigned long.
diff --git a/libelf/elf_begin.c b/libelf/elf_begin.c
index bd3399de..0c9a988d 100644
--- a/libelf/elf_begin.c
+++ b/libelf/elf_begin.c
@@ -383,7 +383,7 @@ file_read_elf (int fildes, void *map_address, unsigned char
*e_ident,
if (map_address != NULL && e_ident[EI_DATA] == MY_ELFDATA
&& cmd != ELF_C_READ_MMAP /* We need a copy to be able to write. */
&& (ALLOW_UNALIGNED
- || (((uintptr_t) ((char *) ehdr + e_shoff)
+ || uintptr_t) ehdr + e_shoff)
& (__alignof__ (Elf32_Shdr) - 1)) == 0)))
{
if (unlikely (scncnt > 0 && e_shoff >= maxsize)
@@ -395,8 +395,10 @@ file_read_elf (int fildes, void *map_address, unsigned
char *e_ident,
__libelf_seterrno (ELF_E_INVALID_ELF);
return NULL;
}
- elf->state.elf32.shdr
- = (Elf32_Shdr *) ((char *) ehdr + e_shoff);
+
+ if (scncnt > 0)
+ elf->state.elf32.shdr
+ = (Elf32_Shdr *) ((char *) ehdr + e_shoff);
for (size_t cnt = 0; cnt < scncnt; ++cnt)
{
@@ -485,15 +487,17 @@ file_read_elf (int fildes, void *map_address, unsigned
char *e_ident,
if (map_address != NULL && e_ident[EI_DATA] == MY_ELFDATA
&& cmd != ELF_C_READ_MMAP /* We need a copy to be able to write. */
&& (ALLOW_UNALIGNED
- || (((uintptr_t) ((char *) ehdr + e_shoff)
+ || uintptr_t) ehdr + e_shoff)
& (__alignof__ (Elf64_Shdr) - 1)) == 0)))
{
if (unlikely (scncnt > 0 && e_shoff >= maxsize)
|| unlikely (maxsize - e_shoff
< scncnt * sizeof (Elf64_Shdr)))
goto free_and_out;
- elf->state.elf64.shdr
- = (Elf64_Shdr *) ((char *) ehdr + e_shoff);
+
+ if (scncnt > 0)
+ elf->state.elf64.shdr
+ = (Elf64_Shdr *) ((char *) ehdr + e_shoff);
for (size_t cnt = 0; cnt < scncnt; ++cnt)
{
--
2.30.2
[PATCH] tests: integrate fuzz-dwfl-core into the test suite
[v2]
1) At https://sourceware.org/pipermail/elfutils-devel/2021q4/004541.html
it was pointed out that build-fuzzers.sh is too tied to OSS-Fuzz
and while it was kind of decoupled from it as much as possible
in the sense that it was enough to install clang and run the script to build
the fuzz target with libFuzzer it's true that it can't be integrated smoothly
into buildbot for example where gcc is used and various configure options
control what exactly is tested. To address that, `--enable-honggfuzz` is
introduced. It looks for hfuzz-gcc, hfuzz-g++ and honggfuzz and if
they exist elfutils is built with those wrappers and the fuzz target
is additionally run for a few minutes under honggfuzz to make
regression testing more effective. It was tested on Fedora 35 and
in https://github.com/evverx/elfutils/pull/49 on Ubuntu Focal with both gcc and
clang with and without sanitizers/Valgrind
and with two versions of honggfuzz (including the latest stable version).
To make it work on Ubuntu the following commands should be run
```
apt-get install libbfd-dev libunwind8-dev
git clone https://github.com/google/honggfuzz
cd honggfuzz
git checkout 2.4
make
make PREFIX=/usr install
cd PATH/TO/ELFUTILS
autoreconf -i -f
./configure --enable-maintainer-mode --enable-honggfuzz
make check V=1 VERBOSE=1 # FUZZ_TIME can be optionally passed
```
If hongfuzz is installed elsewhere it's possible to point
configure to it with CC, CXX and HONGGFUZZ
```
./configure CC=path-to-hfuzz-gcc CXX=path-to-hfuzz-g++
HONGGFUZZ=path-to-honggfuzz
```
I decided to use honggfuzz instead of AFL because AFL doesn't seem
to be maintained actively anymore. Other than that I can't seem to
make it work with various combinations of compilers, sanitizers and
so on. But thanks to the way the fuzz target is written it should
be possible to add it eventually by analogy with honggfuzz.
2) fuzz-dwfl-core-corpus was renamed to fuzz-dwfl-core-crashes to
make it more clear that it isn't exaclty a seed corpus.
3) run-strip-g.sh and run-strip-nothing.sh started to compile test programs
using temporary files instead of gcc -xc -. It should address
https://github.com/google/honggfuzz/issues/431 but more generally
autoconf uses temporary files to make sure compiler works so
it seems in general it's safer to rely on compiler features that
are known to work.
4) A comment was added where I tried to expand on why the fuzz target
is written that way.
[v1]
The fuzz target was integrated into OSS-Fuzz in
https://github.com/google/oss-fuzz/pull/6944 and since then it
has been running there continously (uncovering various issues
along the way). It's all well and good but since OSS-Fuzz
is far from the elfutils repository it's unnecessarily hard
to build the fuzz target locally, verify patches and more generally
test new code to make sure that it doesn't introduce new issues (
or reintroduce regressions). This patch aims to address all those
issues by moving the fuzz target into the elfutils repository,
integrating it into the testsuite and also providing a script
that can be used to build full-fledged fuzzers utilizing libFuzzer.
With this patch applied `make check` can be used to make sure
that files kept in tests/fuzz-dwfl-core-corpus don't crash the
code on various architecures.
`--enable-sanitize-{address,undefined}` and/or `--enable-valgrind`
can additionally be used to uncover issues like
https://sourceware.org/bugzilla/show_bug.cgi?id=28685
that don't always manifest themselves in simple segfaults. On top
of all that now the fuzz target can be built and linked against
libFuzzer locally by just running `./tests/build-fuzzers.sh`.
The patch was tested in https://github.com/evverx/elfutils/pull/49 :
* the testsuite was run on aarch64, armhfp, i386, ppc64le, s390x
and x86_64
* Fedora packages were built on those architectures;
* elfutils was built with both clang and gcc with and without sanitizers
to make sure the tests pass there;
* `make distcheck` passed;
* coverage reports were built to make sure "static" builds are intact;
* the fuzz target was built and run with ClusterFuzzLite to make sure
it's still compatible with OSS-Fuzz;
* the code was analyzed by various static analyzers to make sure new alerts
aren't introduced.
Signed-off-by: Evgeny Vereshchagin
---
ChangeLog | 4 +
configure.ac| 17
tests/.gitignore| 1 +
tests/ChangeLog | 8 ++
tests/Makefile.am | 30 ++-
tests/build-fuzzers.sh | 95
tests/fuzz-dwfl-core-crashes/bugzilla-28660 | Bin 0 -> 20890 bytes
tests/fuzz-dwfl-core-crashes/empty | 0
tests/fuzz-dwfl-core-crashes/oss-fuzz-41566 | Bin 0 -> 1553 bytes
tests/fuzz-dwfl-core-crashes/oss-fuzz-41570 | Bin 0 -> 1233 bytes
tests/fuzz-dwfl-core-crashes/oss-fuzz-41572 | Bin 0 -> 4872 bytes
tests/fuzz-dwfl-core.c
[PATCH] libdwfl: Make sure that ph_buffer_size has room for at least one phdr
dwfl_segment_report_module might otherwise try to handle half a phdr
taking the other half from after the buffer.
Signed-off-by: Mark Wielaard
---
libdwfl/ChangeLog| 5 +
libdwfl/dwfl_segment_report_module.c | 7 ++-
2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/libdwfl/ChangeLog b/libdwfl/ChangeLog
index d00ce702..38e2bdaa 100644
--- a/libdwfl/ChangeLog
+++ b/libdwfl/ChangeLog
@@ -1,3 +1,8 @@
+2021-12-08 Mark Wielaard
+
+ * dwfl_segment_report_module.c (dwfl_segment_report_module): Make sure
+ that ph_buffer_size has room for at least one phdr.
+
2021-12-08 Mark Wielaard
* dwfl_segment_report_module.c (dwfl_segment_report_module): Make
diff --git a/libdwfl/dwfl_segment_report_module.c
b/libdwfl/dwfl_segment_report_module.c
index 89e05103..840d6f44 100644
--- a/libdwfl/dwfl_segment_report_module.c
+++ b/libdwfl/dwfl_segment_report_module.c
@@ -426,7 +426,12 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const
char *name,
buffer, otherwise it will be the size of the new buffer that
could be read. */
if (ph_buffer_size != 0)
-xlatefrom.d_size = ph_buffer_size;
+{
+ phnum = ph_buffer_size / phentsize;
+ if (phnum == 0)
+ goto out;
+ xlatefrom.d_size = ph_buffer_size;
+}
xlatefrom.d_buf = ph_buffer;
--
2.30.2
[PATCH] libdwfl: Make sure dyn_filesz has a sane size
In dwfl_segment_report_module dyn_filesz should be able to hold at least one Elf_Dyn element, and not be larger than possible. Signed-off-by: Mark Wielaard --- libdwfl/ChangeLog| 6 ++ libdwfl/dwfl_segment_report_module.c | 3 +++ 2 files changed, 9 insertions(+) diff --git a/libdwfl/ChangeLog b/libdwfl/ChangeLog index 38e2bdaa..1f83576d 100644 --- a/libdwfl/ChangeLog +++ b/libdwfl/ChangeLog @@ -1,3 +1,9 @@ +2021-12-08 Mark Wielaard + + * dwfl_segment_report_module.c (dwfl_segment_report_module): Make sure + that dyn_filesz can contain at least one Elf_Dyn and isn't larger than + possible. + 2021-12-08 Mark Wielaard * dwfl_segment_report_module.c (dwfl_segment_report_module): Make sure diff --git a/libdwfl/dwfl_segment_report_module.c b/libdwfl/dwfl_segment_report_module.c index 840d6f44..78c70795 100644 --- a/libdwfl/dwfl_segment_report_module.c +++ b/libdwfl/dwfl_segment_report_module.c @@ -787,6 +787,9 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char *name, if (dyn_data_size != 0) dyn_filesz = dyn_data_size; + if ((dyn_filesz / dyn_entsize) == 0 + || dyn_filesz > (SIZE_MAX / dyn_entsize)) + goto out; void *dyns = malloc (dyn_filesz); Elf32_Dyn *d32 = dyns; Elf64_Dyn *d64 = dyns; -- 2.30.2
[PATCH] libdwfl: Rewrite GElf_Nhdr reading in dwfl_segment_report_module
Make sure that the notes filesz is not too big. Rewrite reading of the
notes to check for overflow at every step. Also limit the size of the
buildid bytes.
Signed-off-by: Mark Wielaard
---
libdwfl/ChangeLog| 5 ++
libdwfl/dwfl_segment_report_module.c | 79
2 files changed, 49 insertions(+), 35 deletions(-)
diff --git a/libdwfl/ChangeLog b/libdwfl/ChangeLog
index 1f83576d..18ffc347 100644
--- a/libdwfl/ChangeLog
+++ b/libdwfl/ChangeLog
@@ -1,3 +1,8 @@
+2021-12-19 Mark Wielaard
+
+ * dwfl_segment_report_module.c (dwfl_segment_report_module): Check
+ notes filesz. Rewrite reading of GElf_Nhdr.
+
2021-12-08 Mark Wielaard
* dwfl_segment_report_module.c (dwfl_segment_report_module): Make sure
diff --git a/libdwfl/dwfl_segment_report_module.c
b/libdwfl/dwfl_segment_report_module.c
index 78c70795..a6d6be85 100644
--- a/libdwfl/dwfl_segment_report_module.c
+++ b/libdwfl/dwfl_segment_report_module.c
@@ -520,6 +520,9 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char
*name,
if (data_size != 0)
filesz = data_size;
+ if (filesz > SIZE_MAX / sizeof (Elf32_Nhdr))
+ continue;
+
assert (sizeof (Elf32_Nhdr) == sizeof (Elf64_Nhdr));
void *notes;
@@ -532,6 +535,8 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char
*name,
{
const unsigned int xencoding = ehdr.e32.e_ident[EI_DATA];
+ if (filesz > SIZE_MAX / sizeof (Elf32_Nhdr))
+ continue;
notes = malloc (filesz);
if (unlikely (notes == NULL))
continue; /* Next header */
@@ -552,44 +557,48 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const
char *name,
const GElf_Nhdr *nh = notes;
size_t len = 0;
- size_t last_len;
- while (filesz > len + sizeof (*nh))
+ while (filesz - len > sizeof (*nh))
{
- const void *note_name;
- const void *note_desc;
- last_len = len;
-
- len += sizeof (*nh);
- note_name = notes + len;
-
- len += nh->n_namesz;
- len = align == 8 ? NOTE_ALIGN8 (len) : NOTE_ALIGN4 (len);
- note_desc = notes + len;
-
- if (unlikely (filesz < len + nh->n_descsz
-|| len <= last_len
-|| len + nh->n_descsz < last_len))
-break;
-
- if (nh->n_type == NT_GNU_BUILD_ID
- && nh->n_descsz > 0
- && nh->n_namesz == sizeof "GNU"
- && !memcmp (note_name, "GNU", sizeof "GNU"))
-{
- build_id.vaddr = (note_desc
+ len += sizeof (*nh);
+
+ size_t namesz = nh->n_namesz;
+ namesz = align == 8 ? NOTE_ALIGN8 (namesz) : NOTE_ALIGN4
(namesz);
+ if (namesz > filesz - len || len + namesz < namesz)
+ break;
+
+ void *note_name = notes + len;
+ len += namesz;
+
+ size_t descsz = nh->n_descsz;
+ descsz = align == 8 ? NOTE_ALIGN8 (descsz) : NOTE_ALIGN4
(descsz);
+ if (descsz > filesz - len || len + descsz < descsz)
+ break;
+
+ void *note_desc = notes + len;
+ len += descsz;
+
+ /* We don't handle very short or really large build-ids. We
need at
+at least 3 and allow for up to 64 (normally ids are 20
long). */
+#define MIN_BUILD_ID_BYTES 3
+#define MAX_BUILD_ID_BYTES 64
+ if (nh->n_type == NT_GNU_BUILD_ID
+ && nh->n_descsz >= MIN_BUILD_ID_BYTES
+ && nh->n_descsz <= MAX_BUILD_ID_BYTES
+ && nh->n_namesz == sizeof "GNU"
+ && !memcmp (note_name, "GNU", sizeof "GNU"))
+ {
+ build_id.vaddr = (note_desc
- (const void *) notes
+ note_vaddr);
- build_id.len = nh->n_descsz;
- build_id.memory = malloc (build_id.len);
- if (likely (build_id.memory != NULL))
-memcpy (build_id.memory, note_desc, build_id.len);
- break;
-}
-
- len += nh->n_descsz;
- len = align == 8 ? NOTE_ALIGN8 (len) : NOTE_ALIGN4 (len);
- nh = (void *) notes + len;
-}
+ build_id.len = nh->n_descsz;
+ build_id.memory = malloc (build_id.len);
+ if (likely (build_id.memory !
[Bug libdw/28715] New: There seems to be an infinite loop in dwfl_segment_report_module
https://sourceware.org/bugzilla/show_bug.cgi?id=28715 Bug ID: 28715 Summary: There seems to be an infinite loop in dwfl_segment_report_module Product: elfutils Version: unspecified Status: UNCONFIRMED Severity: normal Priority: P2 Component: libdw Assignee: unassigned at sourceware dot org Reporter: evvers at ya dot ru CC: elfutils-devel at sourceware dot org Target Milestone: --- Created attachment 13863 --> https://sourceware.org/bugzilla/attachment.cgi?id=13863&action=edit File causing an infinite loop It was reported today in https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=42645 . It can be reproduced with `./src/stack`: ``` autoreconf -i -f ./configure --enable-maintainer-mode make -j$(nproc) V=1 LD_LIBRARY_PATH="./libdw;./libelf" ./src/stack --core ../oss-fuzz-42645 ``` According to eu-stack it's in dwfl_segment_report_module ``` PID 212089 - process TID 212089: #0 0x7f6af3447cd5 dwfl_segment_report_module #1 0x7f6af344bf88 dwfl_core_file_report@@ELFUTILS_0.158 #2 0x00402ec7 parse_opt #3 0x7f6af30da472 argp_parse #4 0x004024eb main #5 0x7f6af2fe9560 __libc_start_call_main #6 0x7f6af2fe960c __libc_start_main@@GLIBC_2.34 #7 0x00402725 _start ``` Below is the backtrace OSS-Fuzz attached to the issue (with line numbers): ``` ALARM: working on the last Unit for 61 seconds and the timeout value is 60 (use -timeout=N to change) ==822== ERROR: libFuzzer: timeout after 61 seconds #0 0x4b22d4 in __sanitizer_print_stack_trace /src/llvm-project/compiler-rt/lib/ubsan/ubsan_diag_standalone.cpp:31:3 #1 0x457438 in fuzzer::PrintStackTrace() cxa_noexception.cpp:0 #2 0x43c329 in fuzzer::Fuzzer::AlarmCallback() cxa_noexception.cpp:0 #3 0x7f1be648c3bf in libpthread.so.0 #4 0x4aea5b in atomic_exchange<__sanitizer::atomic_uint32_t> /src/llvm-project/compiler-rt/lib/sanitizer_common/sanitizer_atomic_clang.h:67:7 #5 0x4aea5b in acquire /src/llvm-project/compiler-rt/lib/ubsan/ubsan_value.h:60:21 #6 0x4aea5b in handleNegateOverflowImpl(__ubsan::OverflowData*, unsigned long, __ubsan::ReportOptions) /src/llvm-project/compiler-rt/lib/ubsan/ubsan_handlers.cpp:251:34 #7 0x4aea2d in __ubsan_handle_negate_overflow /src/llvm-project/compiler-rt/lib/ubsan/ubsan_handlers.cpp:277:3 #8 0x517854 in dwfl_segment_report_module /src/elfutils/libdwfl/dwfl_segment_report_module.c:581:58 #9 0x4b8937 in dwfl_core_file_report /src/elfutils/libdwfl/core-file.c:559:17 #10 0x4b388e in LLVMFuzzerTestOneInput /src/fuzz-dwfl-core.c:52:6 #11 0x43d953 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) cxa_noexception.cpp:0 #12 0x429592 in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:324:6 #13 0x42edec in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) cxa_noexception.cpp:0 #14 0x457bf2 in main /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerMain.cpp:20:10 #15 0x7f1be62800b2 in __libc_start_main /build/glibc-eX1tMB/glibc-2.31/csu/libc-start.c:308:16 #16 0x407d4d in _start ``` -- You are receiving this mail because: You are on the CC list for the bug.
[Bug debuginfod/28708] run-debuginfod-webapi-concurrency.sh seems to be flaky
https://sourceware.org/bugzilla/show_bug.cgi?id=28708 --- Comment #7 from Evgeny Vereshchagin --- > Note that packit doesn't use real hardware for various architectures but > "container emulation" which causes various testcases to fail. > I think I ran into issues like that in https://github.com/evverx/elfutils/issues/32 and https://github.com/evverx/elfutils/issues/31. I ignore them for the most part. Though it would be great if they could be skipped there. Some of them seem to be easy to skip because they seem to trigger seccomp filters of some kind but I'm not sure about the rest. > Although in this case it seems it is overloading the host. Maybe we can tune > down the number of concurrent request tested, see also: > https://sourceware.org/pipermail/elfutils-devel/2021q4/thread.html#4488 > If you have a better lower/upper bound or a way to test the limits of the > machine. > Thanks for the link. I'll take a look. > We do have somewhat better buildbot workers for various architectures here: > https://builder.wildebeest.org/buildbot/#/builders?tags=elfutils As far as I understand the tests are run there on commits to the elfutils repository but I'm not sure how to test "PRs" there. If it was possible to use it before commits are merged into the master branch I wouldn't have started using Packit on GitHub probably. > Is there some way of finding out the host's actual limits? Can we detect that > we're running in an unusually constricted environment and skip this test > ulimit -u? I think I can run almost anything there but since I'm not familiar with the test I'm not sure what I should look for. -- You are receiving this mail because: You are on the CC list for the bug.
[Bug debuginfod/28708] run-debuginfod-webapi-concurrency.sh seems to be flaky
https://sourceware.org/bugzilla/show_bug.cgi?id=28708 --- Comment #8 from Frank Ch. Eigler --- This test creates up to 100+few threads in debuginfod, and also 100 concurrent curl processes to talk to debuginfod. -- You are receiving this mail because: You are on the CC list for the bug.
[PATCH] libdwfl: Handle unaligned Ehdr in dwfl_segment_report_module
The xlate functions only handle correctly aligned buffers. But they do
handle src == dest. So if the source buffer isn't aligned correctly
just copy it first into the destination (which is already correctly
aligned).
Signed-off-by: Mark Wielaard
---
libdwfl/ChangeLog| 5 +
libdwfl/dwfl_segment_report_module.c | 14 ++
2 files changed, 19 insertions(+)
diff --git a/libdwfl/ChangeLog b/libdwfl/ChangeLog
index 18ffc347..6c7e0c4a 100644
--- a/libdwfl/ChangeLog
+++ b/libdwfl/ChangeLog
@@ -1,3 +1,8 @@
+2021-12-19 Mark Wielaard
+
+ * dwfl_segment_report_module.c (dwfl_segment_report_module): Copy
+ buffer and set xlatefrom.d_buf to ehdr when buffer is not aligned.
+
2021-12-19 Mark Wielaard
* dwfl_segment_report_module.c (dwfl_segment_report_module): Check
diff --git a/libdwfl/dwfl_segment_report_module.c
b/libdwfl/dwfl_segment_report_module.c
index a6d6be85..7f756392 100644
--- a/libdwfl/dwfl_segment_report_module.c
+++ b/libdwfl/dwfl_segment_report_module.c
@@ -367,6 +367,20 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const
char *name,
e_ident = ((const unsigned char *) buffer);
ei_class = e_ident[EI_CLASS];
ei_data = e_ident[EI_DATA];
+ /* buffer may be unaligned, in which case xlatetom would not work.
+ xlatetom does work when the in and out d_buf are equal (but not
+ for any other overlap). */
+ size_t ehdr_align = (ei_class == ELFCLASS32
+ ? __alignof__ (Elf32_Ehdr)
+ : __alignof__ (Elf64_Ehdr));
+ if (((uintptr_t) buffer & (ehdr_align - 1)) != 0)
+{
+ memcpy (&ehdr, buffer,
+ (ei_class == ELFCLASS32
+ ? sizeof (Elf32_Ehdr)
+ : sizeof (Elf64_Ehdr)));
+ xlatefrom.d_buf = &ehdr;
+}
switch (ei_class)
{
case ELFCLASS32:
--
2.30.2
[PATCH] libdwfl: Handle unaligned Phdr in dwfl_segment_report_module
The xlate functions only handle correctly aligned buffers. But they do
handle src == dest. So if the source buffer isn't aligned correctly
just copy it first into the destination (which is already correctly
aligned).
Signed-off-by: Mark Wielaard
---
libdwfl/ChangeLog| 6 ++
libdwfl/dwfl_segment_report_module.c | 12
2 files changed, 18 insertions(+)
diff --git a/libdwfl/ChangeLog b/libdwfl/ChangeLog
index 6c7e0c4a..ac0fbe0f 100644
--- a/libdwfl/ChangeLog
+++ b/libdwfl/ChangeLog
@@ -1,3 +1,9 @@
+2021-12-19 Mark Wielaard
+
+ * dwfl_segment_report_module.c (dwfl_segment_report_module): Copy
+ ph_buffer and set xlatefrom.d_buf to phdrsp when ph_buffer is not
+ aligned.
+
2021-12-19 Mark Wielaard
* dwfl_segment_report_module.c (dwfl_segment_report_module): Copy
diff --git a/libdwfl/dwfl_segment_report_module.c
b/libdwfl/dwfl_segment_report_module.c
index 7f756392..de190e90 100644
--- a/libdwfl/dwfl_segment_report_module.c
+++ b/libdwfl/dwfl_segment_report_module.c
@@ -461,6 +461,18 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const
char *name,
xlateto.d_buf = phdrsp;
xlateto.d_size = phdrsp_bytes;
+ /* ph_ buffer may be unaligned, in which case xlatetom would not work.
+ xlatetom does work when the in and out d_buf are equal (but not
+ for any other overlap). */
+ size_t phdr_align = (class32
+ ? __alignof__ (Elf32_Phdr)
+ : __alignof__ (Elf64_Phdr));
+ if (((uintptr_t) ph_buffer & (phdr_align - 1)) != 0)
+{
+ memcpy (phdrsp, ph_buffer, phdrsp_bytes);
+ xlatefrom.d_buf = phdrsp;
+}
+
/* Track the bounds of the file visible in memory. */
GElf_Off file_trimmed_end = 0; /* Proper p_vaddr + p_filesz end. */
GElf_Off file_end = 0;/* Rounded up to effective page size. */
--
2.30.2
[Bug libelf/28685] UBSan: member access within misaligned address 0x7ff316818032 for type 'struct Elf32_Phdr'
https://sourceware.org/bugzilla/show_bug.cgi?id=28685 --- Comment #3 from Mark Wielaard --- (In reply to Evgeny Vereshchagin from comment #2) > If callers are > expected to pass correctly aligned buffers it seems > dwfl_segment_report_module should be fixed. But it seems that callers can > sometimes assume that it should be fine to pass unaligned data. For example, > (even though it has nothing to do with the xlateto functions) in one of > libbpf issues it was pointed out that "I don't see anywhere the requirement > that bytes passed to the elf_memory() should be aligned, so this does seem > like libelf bug." I am not sure I like people explicitly passing in unaligned buffers to elf_memory (). We'll need to carefully audit that works. It also means lots of copying data structures around to get a correctly aligned version. Also the xlate functions work on Elf_Data, I think it is reasonable to assume those normally come from other libelf functions and that the d_buf pointers are correctly aligned for the d_type. For now I just fixed up the code in dwfl_segment_report_module to make sure the buffers passed to the xlate functions are properly aligned. See the following proposed patches: https://sourceware.org/pipermail/elfutils-devel/2021q4/004552.html https://sourceware.org/pipermail/elfutils-devel/2021q4/004561.html https://sourceware.org/pipermail/elfutils-devel/2021q4/004562.html -- You are receiving this mail because: You are on the CC list for the bug.
[Bug libdw/28710] ERROR: AddressSanitizer: SEGV on unknown address (on i386)
https://sourceware.org/bugzilla/show_bug.cgi?id=28710 Mark Wielaard changed: What|Removed |Added CC||mark at klomp dot org --- Comment #1 from Mark Wielaard --- I cannot replicate this with my latest proposed patches. I suspect this proposed patch fixed it: https://sourceware.org/pipermail/elfutils-devel/2021q4/004557.html -- You are receiving this mail because: You are on the CC list for the bug.
[PATCH] libdwfl: Handle unaligned Phdr in dwfl_segment_report_module
The xlate functions only handle correctly aligned buffers. But they do
handle src == dest. So if the source buffer isn't aligned correctly
just copy it first into the destination (which is already correctly
aligned).
Signed-off-by: Mark Wielaard
---
libdwfl/ChangeLog| 5 +
libdwfl/dwfl_segment_report_module.c | 12
2 files changed, 17 insertions(+)
diff --git a/libdwfl/ChangeLog b/libdwfl/ChangeLog
index ac0fbe0f..6015f6b7 100644
--- a/libdwfl/ChangeLog
+++ b/libdwfl/ChangeLog
@@ -1,3 +1,8 @@
+2021-12-19 Mark Wielaard
+
+ * dwfl_segment_report_module.c (dwfl_segment_report_module): Copy
+ data and set xlatefrom.d_buf to notes when data is not aligned.
+
2021-12-19 Mark Wielaard
* dwfl_segment_report_module.c (dwfl_segment_report_module): Copy
diff --git a/libdwfl/dwfl_segment_report_module.c
b/libdwfl/dwfl_segment_report_module.c
index de190e90..72c85070 100644
--- a/libdwfl/dwfl_segment_report_module.c
+++ b/libdwfl/dwfl_segment_report_module.c
@@ -573,6 +573,18 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const
char *name,
xlatefrom.d_size = filesz;
xlateto.d_buf = notes;
xlateto.d_size = filesz;
+
+ /* data may be unaligned, in which case xlatetom would not
work.
+xlatetom does work when the in and out d_buf are equal
(but not
+for any other overlap). */
+ if ((uintptr_t) data != (align == 8
+ ? NOTE_ALIGN8 ((uintptr_t) data)
+ : NOTE_ALIGN4 ((uintptr_t) data)))
+ {
+ memcpy (notes, data, filesz);
+ xlatefrom.d_buf = notes;
+ }
+
if (elf32_xlatetom (&xlateto, &xlatefrom, xencoding) == NULL)
{
free (notes);
--
2.30.2
[Bug libdw/28715] There seems to be an infinite loop in dwfl_segment_report_module
https://sourceware.org/bugzilla/show_bug.cgi?id=28715 Mark Wielaard changed: What|Removed |Added Assignee|unassigned at sourceware dot org |mark at klomp dot org Status|UNCONFIRMED |ASSIGNED Last reconfirmed||2021-12-20 Ever confirmed|0 |1 CC||mark at klomp dot org --- Comment #1 from Mark Wielaard --- I couldn't replicate the infinite loop, which I assume has been fixed by: https://sourceware.org/pipermail/elfutils-devel/2021q4/004557.html But I saw it did trigger yet another xlate alignment issue for which I did submit: https://sourceware.org/pipermail/elfutils-devel/2021q4/004565.html P.S. Note that the bugs.chromium.org link doesn't work for me, it apparently requires a google account with permissions to even just look at that bug. -- You are receiving this mail because: You are on the CC list for the bug.
[Bug libdw/28715] There seems to be an infinite loop in dwfl_segment_report_module
https://sourceware.org/bugzilla/show_bug.cgi?id=28715 --- Comment #2 from Evgeny Vereshchagin --- (In reply to Mark Wielaard from comment #1) > I couldn't replicate the infinite loop, which I assume has been fixed by: > https://sourceware.org/pipermail/elfutils-devel/2021q4/004557.html > > But I saw it did trigger yet another xlate alignment issue for which I did > submit: > https://sourceware.org/pipermail/elfutils-devel/2021q4/004565.html Thanks! I'll try to take a look tomorrow. There seem to be quite a few new patches on the mailing list. I wonder if it's possible to somehow fetch a branch with all of them so that I could just rebase the fuzzing infrastructure on top of it? > P.S. Note that the bugs.chromium.org link doesn't work for me, it apparently > requires a google account with permissions to even just look at that bug. It's possible to make all the bug reports public by default. For example, in https://github.com/google/oss-fuzz/pull/6741 the PHP project decided that bug reports should be no longer restricted and flipped the flag. I can do the same but I don't think I can do it without your approval. -- You are receiving this mail because: You are on the CC list for the bug.
