Re: ☠ Buildbot (GNU Toolchain): elfutils - failed test (failure) (master)

[Mark Wielaard <mark@klomp.org>]

[-- Attachment #1: Type: text/plain, Size: 4214 bytes --]

Hi,

On Tue, Apr 19, 2022 at 09:05:22AM +0000, builder--- via Elfutils-devel wrote:
> A new failure has been detected on builder elfutils-debian-armhf while building elfutils.
> 
> Full details are available at:
>     https://builder.sourceware.org/buildbot/#builders/6/builds/3
> 
> Build state: failed test (failure)
> Revision: 5b497d8da4920bf7b63a4aa3752cf580b3ad654c
> Worker: debian-armhf
> Build Reason: (unknown)
> Blamelist: Di Chen <dichen@redhat.com>
> [...]
> - 7: make check ( failure )
>     Logs:
>         - stdio: https://builder.sourceware.org/buildbot/#builders/6/builds/3/steps/7/logs/stdio
>         - test-suite.log: https://builder.sourceware.org/buildbot/#builders/6/builds/3/steps/7/logs/test-suite_log
> [...]
> A new failure has been detected on builder elfutils-debian-i386 while building elfutils.
> 
> Full details are available at:
>     https://builder.sourceware.org/buildbot/#builders/17/builds/2
> 
> Build state: failed test (failure)
> Revision: 5b497d8da4920bf7b63a4aa3752cf580b3ad654c
> Worker: debian-i386
> Build Reason: (unknown)
> Blamelist: Di Chen <dichen@redhat.com>
> [...]
> - 7: make check ( failure )
>     Logs:
>         - stdio: https://builder.sourceware.org/buildbot/#builders/17/builds/2/steps/7/logs/stdio
>         - test-suite.log: https://builder.sourceware.org/buildbot/#builders/17/builds/2/steps/7/logs/test-suite_log

These are interesting failures:

==28027==ERROR: AddressSanitizer: stack-use-after-scope on address 0xbef24d60 at pc 0x004d9a6f bp 0xbef24c38 sp 0xbef24c3c
READ of size 8 at 0xbef24d60 thread T0
    #0 0x4d9a6d in get_dyn_ents /var/lib/buildbot/workers/wildebeest/elfutils-debian-armhf/build/src/readelf.c:1787
    #1 0x4d9a6d in handle_dynamic /var/lib/buildbot/workers/wildebeest/elfutils-debian-armhf/build/src/readelf.c:1810
    #2 0x4fcc75 in print_dynamic /var/lib/buildbot/workers/wildebeest/elfutils-debian-armhf/build/src/readelf.c:1946
    #3 0x4fcc75 in process_elf_file /var/lib/buildbot/workers/wildebeest/elfutils-debian-armhf/build/src/readelf.c:1008
    #4 0x5001c1 in process_dwflmod /var/lib/buildbot/workers/wildebeest/elfutils-debian-armhf/build/src/readelf.c:792
    #5 0xb67fd7ad in dwfl_getmodules /var/lib/buildbot/workers/wildebeest/elfutils-debian-armhf/build/libdwfl/dwfl_getmodules.c:86
    #6 0x4e352f in process_file /var/lib/buildbot/workers/wildebeest/elfutils-debian-armhf/build/src/readelf.c:900
    #7 0x4d3c2b in main /var/lib/buildbot/workers/wildebeest/elfutils-debian-armhf/build/src/readelf.c:372
    #8 0xb5fde523 in __libc_start_main /build/glibc-RK4xiD/glibc-2.28/csu/libc-start.c:308
Address 0xbef24d60 is located in stack of thread T0 at offset 160 in frame
    #0 0x4d8f83 in handle_dynamic /var/lib/buildbot/workers/wildebeest/elfutils-debian-armhf/build/src/readelf.c:1795
  This frame has 5 object(s):
    [32, 36) 'shstrndx'
    [96, 112) 'dynmem'
    [160, 176) 'dyn_mem' <== Memory access at offset 160 is inside this variable
    [224, 288) 'glink_mem'
    [320, 384) 'buf'
HINT: this may be a false positive if your program uses some custom stack unwind mechanism or swapcontext
      (longjmp and C++ exceptions *are* supported)
SUMMARY: AddressSanitizer: stack-use-after-scope /var/lib/buildbot/workers/wildebeest/elfutils-debian-armhf/build/src/readelf.c:1787 in get_dyn_ents

I think they are not real bugs, or the valgrind builders would have
picked them up. But the are theoretically issues since a compiler
could optimize its stack usage to make this an issue.

static size_t 
get_dyn_ents (Elf_Data * dyn_data)
{
  GElf_Dyn *dyn;
  GElf_Dyn dyn_mem;
  size_t dyn_idx = 0;
  do
    {
      GElf_Dyn dyn_mem;
      dyn = gelf_getdyn(dyn_data, dyn_idx, &dyn_mem);
      if (dyn != NULL)
        ++dyn_idx;
    }
  while (dyn != NULL && dyn->d_tag != DT_NULL);

  return dyn_idx;
}

The issue is dyn pointing to dyn_mem, but dyn_mem being defined inside
the do while loop, while dyn is accessed "outside" it (or more
correctly, in the condition of the do while loop). Technically it
means at that point dyn_mem could have been purged from the
stack. Although unlikely moving the dyn_mem definition outside the
loop should fix it.

I'll push the attached fix.

Cheers,

Mark

[-- Attachment #2: 0001-readelf-Define-dyn_mem-outside-the-while-loop.patch --]
[-- Type: text/x-diff, Size: 1201 bytes --]

From 21fa92319657ca479ae108967fd41ac523a2f876 Mon Sep 17 00:00:00 2001
From: Mark Wielaard <mark@klomp.org>
Date: Tue, 19 Apr 2022 11:25:42 +0200
Subject: [PATCH] readelf: Define dyn_mem outside the while loop.

The GCC address sanitizer might complain otherwise:
stack-use-after-scope src/readelf.c:1787 in get_dyn_ents

Signed-off-by: Mark Wielaard <mark@klomp.org>
---
 src/ChangeLog | 4 ++++
 src/readelf.c | 2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/ChangeLog b/src/ChangeLog
index f563e993..6ef81862 100644
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,7 @@
+2022-04-19  Mark Wielaard  <mark@klomp.org>
+
+	* readelf.c (get_dyn_ents): Define dyn_mem outside the while loop.
+
 2022-03-01  Di Chen  <dichen@redhat.com>
 
 	* readelf.c (get_dyn_ents): New function.
diff --git a/src/readelf.c b/src/readelf.c
index 4b275ece..4b6aab2b 100644
--- a/src/readelf.c
+++ b/src/readelf.c
@@ -1776,10 +1776,10 @@ static size_t
 get_dyn_ents (Elf_Data * dyn_data)
 {
   GElf_Dyn *dyn;
+  GElf_Dyn dyn_mem;
   size_t dyn_idx = 0;
   do
     {
-      GElf_Dyn dyn_mem;
       dyn = gelf_getdyn(dyn_data, dyn_idx, &dyn_mem);
       if (dyn != NULL)
 	++dyn_idx;
-- 
2.30.2