[2/2] libdw: Use elf_rawdata when checking .debug section

Message ID 20230220155518.86598-3-mark@klomp.org
State Committed
Headers
Series [1/2] libelf: memmove any extra bytes left by elf_cvt_gnuhash conversion |

Commit Message

Mark Wielaard Feb. 20, 2023, 3:55 p.m. UTC
  .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(-)
  

Comments

Aleksei Vetrov Feb. 20, 2023, 4:02 p.m. UTC | #1
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.
  
Evgeny Vereshchagin Feb. 21, 2023, 2:28 a.m. UTC | #2
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
  
Mark Wielaard Feb. 21, 2023, 11:24 a.m. UTC | #3
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
  
Aleksei Vetrov Feb. 21, 2023, 4:40 p.m. UTC | #4
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.
  
Mark Wielaard Feb. 23, 2023, 10:20 a.m. UTC | #5
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
  

Patch

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;