* Fix some .debug checking and gnu hash xlate logic @ 2023-02-20 15:55 Mark Wielaard 2023-02-20 15:55 ` [PATCH 1/2] libelf: memmove any extra bytes left by elf_cvt_gnuhash conversion Mark Wielaard 2023-02-20 15:55 ` [PATCH 2/2] libdw: Use elf_rawdata when checking .debug section Mark Wielaard 0 siblings, 2 replies; 8+ messages in thread From: Mark Wielaard @ 2023-02-20 15:55 UTC (permalink / raw) To: elfutils-devel; +Cc: Evgeny Vereshchagin, Aleksei Vetrov Hi, The last fuzzer found some use (checking) of undefine/uninitialized data. Either of these two patches will fix that: [PATCH 1/2] libelf: memmove any extra bytes left by elf_cvt_gnuhash [PATCH 2/2] libdw: Use elf_rawdata when checking .debug section Note that the bad data wouldn't actually be used, just checked for validity. But these patches make sure the result is deterministic. Cheers, Mark ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/2] libelf: memmove any extra bytes left by elf_cvt_gnuhash conversion 2023-02-20 15:55 Fix some .debug checking and gnu hash xlate logic Mark Wielaard @ 2023-02-20 15:55 ` Mark Wielaard 2023-02-20 15:55 ` [PATCH 2/2] libdw: Use elf_rawdata when checking .debug section Mark Wielaard 1 sibling, 0 replies; 8+ messages in thread From: Mark Wielaard @ 2023-02-20 15:55 UTC (permalink / raw) To: elfutils-devel; +Cc: Evgeny Vereshchagin, Aleksei Vetrov, Mark Wielaard Otherwise some undefined bytes might be left in the buffer. Now they might still be not useful, but at least they are as defined in the file. Signed-off-by: Mark Wielaard <mark@klomp.org> --- ChangeLog | 4 ++++ libelf/gnuhash_xlate.h | 12 ++++++++++-- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/ChangeLog b/ChangeLog index d99d837d..55787f64 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,7 @@ +2023-02-20 Mark Wielaard <mark@klomp.org> + + * gnuhash_xlate.h (elf_cvt_gnuhash): memmove any left over bytes. + 2023-02-15 Mark Wielaard <mark@klomp.org> * configure.ac: Error out when demangler is enabled, but diff --git a/libelf/gnuhash_xlate.h b/libelf/gnuhash_xlate.h index 6faf1136..3a00ae0a 100644 --- a/libelf/gnuhash_xlate.h +++ b/libelf/gnuhash_xlate.h @@ -1,5 +1,6 @@ /* Conversion functions for versioning information. Copyright (C) 2006, 2007 Red Hat, Inc. + Copyright (C) 2023, Mark J. Wielaard <mark@klomp.org> This file is part of elfutils. Written by Ulrich Drepper <drepper@redhat.com>, 2006. @@ -36,6 +37,7 @@ static void elf_cvt_gnuhash (void *dest, const void *src, size_t len, int encode) { + size_t size = len; /* The GNU hash table format on 64 bit machines mixes 32 bit and 64 bit words. We must detangle them here. */ Elf32_Word *dest32 = dest; @@ -45,7 +47,7 @@ elf_cvt_gnuhash (void *dest, const void *src, size_t len, int encode) for (unsigned int cnt = 0; cnt < 4; ++cnt) { if (len < 4) - return; + goto done; dest32[cnt] = bswap_32 (src32[cnt]); len -= 4; } @@ -58,7 +60,7 @@ elf_cvt_gnuhash (void *dest, const void *src, size_t len, int encode) for (unsigned int cnt = 0; cnt < bitmask_words; ++cnt) { if (len < 8) - return; + goto done; dest64[cnt] = bswap_64 (src64[cnt]); len -= 8; } @@ -71,4 +73,10 @@ elf_cvt_gnuhash (void *dest, const void *src, size_t len, int encode) *dest32++ = bswap_32 (*src32++); len -= 4; } + + done: + /* If there are any bytes left, we weren't able to convert the + partial structures, just copy them over. */ + if (len > 0) + memmove (dest + size - len, src + size - len, len); } -- 2.39.2 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/2] libdw: Use elf_rawdata when checking .debug section 2023-02-20 15:55 Fix some .debug checking and gnu hash xlate logic Mark Wielaard 2023-02-20 15:55 ` [PATCH 1/2] libelf: memmove any extra bytes left by elf_cvt_gnuhash conversion Mark Wielaard @ 2023-02-20 15:55 ` Mark Wielaard 2023-02-20 16:02 ` Aleksei Vetrov 1 sibling, 1 reply; 8+ messages in thread From: Mark Wielaard @ 2023-02-20 15:55 UTC (permalink / raw) To: elfutils-devel; +Cc: Evgeny Vereshchagin, Aleksei Vetrov, Mark Wielaard .debug sections are raw bytes and don't need conversion even when host and file have different endian order. Signed-off-by: Mark Wielaard <mark@klomp.org> --- libdw/ChangeLog | 4 ++++ libdw/dwarf_begin_elf.c | 5 +++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/libdw/ChangeLog b/libdw/ChangeLog index e0cd8f21..5e60f786 100644 --- a/libdw/ChangeLog +++ b/libdw/ChangeLog @@ -1,3 +1,7 @@ +2023-02-20 Mark Wielaard <mark@klomp.org> + + * dwarf_begin_elf.c (check_section): Use elf_rawdata. + 2023-02-14 Mark Wielaard <mark@klomp.org> * dwarf_getlocation.c (__libdw_intern_expression): Correct check diff --git a/libdw/dwarf_begin_elf.c b/libdw/dwarf_begin_elf.c index 1d4bb333..92d76d24 100644 --- a/libdw/dwarf_begin_elf.c +++ b/libdw/dwarf_begin_elf.c @@ -1,5 +1,6 @@ /* Create descriptor from ELF descriptor for processing file. Copyright (C) 2002-2011, 2014, 2015, 2017, 2018 Red Hat, Inc. + Copyright (C) 2023, Mark J. Wielaard <mark@klomp.org> This file is part of elfutils. This file is free software; you can redistribute it and/or modify @@ -242,8 +243,8 @@ check_section (Dwarf *result, size_t shstrndx, Elf_Scn *scn, bool inscngrp) } } - /* Get the section data. */ - Elf_Data *data = elf_getdata (scn, NULL); + /* Get the section data. Should be raw bytes, no conversion needed. */ + Elf_Data *data = elf_rawdata (scn, NULL); if (data == NULL) goto err; -- 2.39.2 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] libdw: Use elf_rawdata when checking .debug section 2023-02-20 15:55 ` [PATCH 2/2] libdw: Use elf_rawdata when checking .debug section Mark Wielaard @ 2023-02-20 16:02 ` Aleksei Vetrov 2023-02-21 2:28 ` Evgeny Vereshchagin 0 siblings, 1 reply; 8+ messages in thread From: Aleksei Vetrov @ 2023-02-20 16:02 UTC (permalink / raw) To: Mark Wielaard; +Cc: elfutils-devel, Evgeny Vereshchagin [-- Attachment #1: Type: text/plain, Size: 281 bytes --] Hello, Mark On Mon, Feb 20, 2023 at 3:55 PM Mark Wielaard <mark@klomp.org> wrote: > > .debug sections are raw bytes and don't need conversion even when host > and file have different endian order. Thank you! I like this patch more for its simplicity, looks good to me. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] libdw: Use elf_rawdata when checking .debug section 2023-02-20 16:02 ` Aleksei Vetrov @ 2023-02-21 2:28 ` Evgeny Vereshchagin 2023-02-21 11:24 ` Mark Wielaard 2023-02-21 16:40 ` Aleksei Vetrov 0 siblings, 2 replies; 8+ messages in thread From: Evgeny Vereshchagin @ 2023-02-21 2:28 UTC (permalink / raw) To: Aleksei Vetrov; +Cc: Mark Wielaard, elfutils-devel Hi, On Mon, 20 Feb 2023 at 19:03, Aleksei Vetrov <vvvvvv@google.com> wrote: > On Mon, Feb 20, 2023 at 3:55 PM Mark Wielaard <mark@klomp.org> wrote: > > > > .debug sections are raw bytes and don't need conversion even when host > > and file have different endian order. > > Thank you! I like this patch more for its simplicity, looks good to me. Agreed. I haven't actually tested the patch though but since it's covered by the fuzz target it should be tested once it's merged anyway. On a somewhat related looking at some recent patches and especially https://sourceware.org/git/?p=elfutils.git;a=commit;h=64ee2cb792e7b6ba6ad2a5759bff7ce8714e4668 it seems apart from OSS-Fuzz elfutils is fuzzed elsewhere. Aleksei I wonder if it would be possible to add those fuzz targets to OSS-Fuzz? There are blind spots there and I think it would be really great to start covering at least some of them. Thanks, Evgeny Vereshchagin ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] libdw: Use elf_rawdata when checking .debug section 2023-02-21 2:28 ` Evgeny Vereshchagin @ 2023-02-21 11:24 ` Mark Wielaard 2023-02-23 10:20 ` Mark Wielaard 2023-02-21 16:40 ` Aleksei Vetrov 1 sibling, 1 reply; 8+ messages in thread From: Mark Wielaard @ 2023-02-21 11:24 UTC (permalink / raw) To: Evgeny Vereshchagin, Aleksei Vetrov; +Cc: elfutils-devel Hi, On Tue, 2023-02-21 at 05:28 +0300, Evgeny Vereshchagin wrote: > On Mon, 20 Feb 2023 at 19:03, Aleksei Vetrov <vvvvvv@google.com> wrote: > > On Mon, Feb 20, 2023 at 3:55 PM Mark Wielaard <mark@klomp.org> wrote: > > > > > > .debug sections are raw bytes and don't need conversion even when host > > > and file have different endian order. > > > > Thank you! I like this patch more for its simplicity, looks good to me. > > Agreed. I haven't actually tested the patch though but since it's > covered by the fuzz > target it should be tested once it's merged anyway. I was actually planning on pushing both patches. This one makes sure the conversion code isn't called, because that is unnecessary in this case. The first patch adjusts the conversion code so it doesn't leave some undefined bytes in the section data. > On a somewhat related looking at some recent patches and especially > https://sourceware.org/git/?p=elfutils.git;a=commit;h=64ee2cb792e7b6ba6ad2a5759bff7ce8714e4668 > it seems apart from OSS-Fuzz elfutils is fuzzed elsewhere. Aleksei I > wonder if it would > be possible to add those fuzz targets to OSS-Fuzz? There are blind > spots there and I think it would be > really great to start covering at least some of them. I do often run a fuzzer (afl with --enable-sanitize-undefined and -- enable-sanitize-address with CC="afl-gcc -m32") when writing a new testcase. Some testcases are nice as fuzz targets because they test just one function, so running the fuzzer for a couple of hours exhausts the different input values. Cheers, Mark ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] libdw: Use elf_rawdata when checking .debug section 2023-02-21 11:24 ` Mark Wielaard @ 2023-02-23 10:20 ` Mark Wielaard 0 siblings, 0 replies; 8+ messages in thread From: Mark Wielaard @ 2023-02-23 10:20 UTC (permalink / raw) To: Evgeny Vereshchagin, Aleksei Vetrov; +Cc: elfutils-devel Hi, On Tue, 2023-02-21 at 12:24 +0100, Mark Wielaard wrote: > On Tue, 2023-02-21 at 05:28 +0300, Evgeny Vereshchagin wrote: > > On Mon, 20 Feb 2023 at 19:03, Aleksei Vetrov <vvvvvv@google.com> wrote: > > > On Mon, Feb 20, 2023 at 3:55 PM Mark Wielaard <mark@klomp.org> wrote: > > > > > > > > .debug sections are raw bytes and don't need conversion even when host > > > > and file have different endian order. > > > > > > Thank you! I like this patch more for its simplicity, looks good to me. > > > > Agreed. I haven't actually tested the patch though but since it's > > covered by the fuzz > > target it should be tested once it's merged anyway. > > I was actually planning on pushing both patches. > This one makes sure the conversion code isn't called, because that is > unnecessary in this case. The first patch adjusts the conversion code > so it doesn't leave some undefined bytes in the section data. I have pushed both now. Cheers, Mark ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] libdw: Use elf_rawdata when checking .debug section 2023-02-21 2:28 ` Evgeny Vereshchagin 2023-02-21 11:24 ` Mark Wielaard @ 2023-02-21 16:40 ` Aleksei Vetrov 1 sibling, 0 replies; 8+ messages in thread From: Aleksei Vetrov @ 2023-02-21 16:40 UTC (permalink / raw) To: Evgeny Vereshchagin; +Cc: Mark Wielaard, elfutils-devel [-- Attachment #1: Type: text/plain, Size: 1001 bytes --] Hi Evgeny, On Tue, Feb 21, 2023 at 2:29 AM Evgeny Vereshchagin <evverx@gmail.com> wrote: > Aleksei I wonder if it would be possible to add those fuzz targets to > OSS-Fuzz? There are blind spots there and I think it would be really great to > start covering at least some of them. We are fuzzing a tool named STG (https://android.googlesource.com/platform/external/stg/+/refs/heads/master ), which is using libdw and libdwfl from elfutils. And it already has support for execution through libFuzzer: https://android.googlesource.com/platform/external/stg/+/refs/heads/master/fuzz/ The problem is in building infrastructure. STG as fuzzing target is built inside Google using internal build and fuzzing infrastructure, but in principle it does the same thing as OSS-Fuzz. An AOSP version of STG is built using Android build system, which doesn't support the same simplicity of building with libFuzzer and sanitizers. So it needs some work to integrate STG into OSS-Fuzz. ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2023-02-23 10:20 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-02-20 15:55 Fix some .debug checking and gnu hash xlate logic Mark Wielaard 2023-02-20 15:55 ` [PATCH 1/2] libelf: memmove any extra bytes left by elf_cvt_gnuhash conversion Mark Wielaard 2023-02-20 15:55 ` [PATCH 2/2] libdw: Use elf_rawdata when checking .debug section Mark Wielaard 2023-02-20 16:02 ` Aleksei Vetrov 2023-02-21 2:28 ` Evgeny Vereshchagin 2023-02-21 11:24 ` Mark Wielaard 2023-02-23 10:20 ` Mark Wielaard 2023-02-21 16:40 ` Aleksei Vetrov
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).