date:20231117

Re: [PATCH] libdw: check offset dwarf_formstring in all cases

2023-11-17 Thread Mark Wielaard

Hi Aleksei,

On Thu, 2023-11-16 at 21:29 +, vvv...@google.com wrote:
> This check was initially added to test if offset overflows the safe
> prefix where any string will be null-terminated. However the check
> was placed in a wrong place and didn't cover all `attrp->form` cases.
> 
> * libdw/dwarf_formstring.c (dwarf_formstring): Move offset check
>   right before returning the result.

Oops. I see how this happened for DW_FORM_strp and DW_FORM_line_strp we
use __libdw_read_offset which already check the section offset. But of
course those use d_size, not the string_section_size that was setup in
elf_begin_elf. So the check is also needed for them.

> Signed-off-by: Aleksei Vetrov 
> ---
>  libdw/dwarf_formstring.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/libdw/dwarf_formstring.c b/libdw/dwarf_formstring.c
> index 0ee42411..65f03a5e 100644
> --- a/libdw/dwarf_formstring.c
> +++ b/libdw/dwarf_formstring.c
> @@ -173,11 +173,11 @@ dwarf_formstring (Dwarf_Attribute *attrp)
>   off = read_4ubyte_unaligned (dbg, datap);
>else
>   off = read_8ubyte_unaligned (dbg, datap);
> -
> -  if (off >= data_size)
> - goto invalid_offset;
>  }
>  
> +  if (off >= data_size)
> +goto invalid_offset;
> +
>return (const char *) data->d_buf + off;
>  }
>  INTDEF(dwarf_formstring)

Applied.

Thanks,

Mark

☺ Buildbot (Sourceware): elfutils - build successful (main)

2023-11-17 Thread builder

A restored build has been detected on builder elfutils-debian-testing-x86_64 
while building elfutils.

Full details are available at:
https://builder.sourceware.org/buildbot/#builders/145/builds/218

Build state: build successful
Revision: 1bd9deb9aa19ac2e2fa9665009e0d5924adcf4d3
Worker: bb2-1
Build Reason: (unknown)
Blamelist: Aleksei Vetrov , Frank Ch. Eigler 


Steps:

- 0: worker_preparation ( success )

- 1: set package name ( success )

- 2: git checkout ( success )
Logs:
- stdio: 
https://builder.sourceware.org/buildbot/#builders/145/builds/218/steps/2/logs/stdio

- 3: autoreconf ( success )
Logs:
- stdio: 
https://builder.sourceware.org/buildbot/#builders/145/builds/218/steps/3/logs/stdio

- 4: configure ( success )
Logs:
- stdio: 
https://builder.sourceware.org/buildbot/#builders/145/builds/218/steps/4/logs/stdio
- config.log: 
https://builder.sourceware.org/buildbot/#builders/145/builds/218/steps/4/logs/config_log

- 5: get version ( success )
Logs:
- stdio: 
https://builder.sourceware.org/buildbot/#builders/145/builds/218/steps/5/logs/stdio
- property changes: 
https://builder.sourceware.org/buildbot/#builders/145/builds/218/steps/5/logs/property_changes

- 6: make ( warnings )
Logs:
- stdio: 
https://builder.sourceware.org/buildbot/#builders/145/builds/218/steps/6/logs/stdio
- warnings (3): 
https://builder.sourceware.org/buildbot/#builders/145/builds/218/steps/6/logs/warnings__3_

- 7: make check ( success )
Logs:
- stdio: 
https://builder.sourceware.org/buildbot/#builders/145/builds/218/steps/7/logs/stdio
- test-suite.log: 
https://builder.sourceware.org/buildbot/#builders/145/builds/218/steps/7/logs/test-suite_log

- 8: make distcheck ( warnings )
Logs:
- stdio: 
https://builder.sourceware.org/buildbot/#builders/145/builds/218/steps/8/logs/stdio
- test-suite.log: 
https://builder.sourceware.org/buildbot/#builders/145/builds/218/steps/8/logs/test-suite_log
- warnings (4): 
https://builder.sourceware.org/buildbot/#builders/145/builds/218/steps/8/logs/warnings__4_

- 9: prep ( success )
Logs:
- stdio: 
https://builder.sourceware.org/buildbot/#builders/145/builds/218/steps/9/logs/stdio

- 10: build bunsen.cpio.gz ( success )
Logs:
- stdio: 
https://builder.sourceware.org/buildbot/#builders/145/builds/218/steps/10/logs/stdio

- 11: fetch bunsen.cpio.gz ( success )
Logs:
- stdio: 
https://builder.sourceware.org/buildbot/#builders/145/builds/218/steps/11/logs/stdio

- 12: unpack bunsen.cpio.gz ( success )
Logs:
- stdio: 
https://builder.sourceware.org/buildbot/#builders/145/builds/218/steps/12/logs/stdio

- 13: pass .bunsen.source.gitname ( success )
Logs:
- stdio: 
https://builder.sourceware.org/buildbot/#builders/145/builds/218/steps/13/logs/stdio

- 14: pass .bunsen.source.gitdescribe ( success )
Logs:
- stdio: 
https://builder.sourceware.org/buildbot/#builders/145/builds/218/steps/14/logs/stdio

- 15: pass .bunsen.source.gitbranch ( success )
Logs:
- stdio: 
https://builder.sourceware.org/buildbot/#builders/145/builds/218/steps/15/logs/stdio

- 16: pass .bunsen.source.gitrepo ( success )
Logs:
- stdio: 
https://builder.sourceware.org/buildbot/#builders/145/builds/218/steps/16/logs/stdio

- 17: upload to bunsen ( success )
Logs:
- stdio: 
https://builder.sourceware.org/buildbot/#builders/145/builds/218/steps/17/logs/stdio

- 18: clean up ( success )
Logs:
- stdio: 
https://builder.sourceware.org/buildbot/#builders/145/builds/218/steps/18/logs/stdio

- 19: make distclean ( success )
Logs:
- stdio: 
https://builder.sourceware.org/buildbot/#builders/145/builds/218/steps/19/logs/stdio

[PATCH 1/2] libdwfl: handle duplicate ELFs when reporting archives

2023-11-17 Thread vvvvvv

From: Aleksei Vetrov 

When archive is processed in process_archive (libdwfl/offline.c), it
creates an Elf object for each archive member. Then in
process_archive_member it calls process_file to create a Dwfl_Module
through __libdwfl_report_elf.

The ownership of the Elf object is expected to be:

* either transfered to the Dwfl_Module, if __libdwfl_report_elf returns
  not NULL;

* or handled at the end of process_archive_member by calling elf_end.

Moreover, Elf object is expected to be alive, if __libdwfl_report_elf
returns not NULL, because at the end of process_archive_member it
advances to the next member through the elf_next call.

The problem happens when __libdwfl_report_elf encounters Elf with the
same name and content as it seen before. In that case dwfl_report_module
will reuse existing Dwfl_Module object. This leads to a codepath that
calls elf_end on the Elf object, while returning not NULL, breaking the
elf_next call to the next member.

The fix is to destroy m->main.elf instead and put the new Elf object in
the already existing Dwfl_Module.

* libdwfl/dwfl_report_elf.c (__libdwfl_report_elf): Replace Elf in
  the Dwfl_Module in case of overlapping or duplicate modules to
  prolong its lifetime for subsequent processing.

Signed-off-by: Aleksei Vetrov 
---
 libdwfl/dwfl_report_elf.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/libdwfl/dwfl_report_elf.c b/libdwfl/dwfl_report_elf.c
index 581f4079..58b06aea 100644
--- a/libdwfl/dwfl_report_elf.c
+++ b/libdwfl/dwfl_report_elf.c
@@ -276,7 +276,8 @@ __libdwfl_report_elf (Dwfl *dwfl, const char *name, const 
char *file_name,
}
   else
{
- elf_end (elf);
+ elf_end (m->main.elf);
+ m->main.elf = elf;
  if (m->main_bias != bias
  || m->main.vaddr != vaddr || m->main.address_sync != address_sync)
goto overlap;
-- 
2.43.0.rc1.413.gea7ed67945-goog

[PATCH 2/2] tests: Add test for duplicate entries in archive

2023-11-17 Thread vvvvvv

From: Aleksei Vetrov 

Test dwfl-report-offline-memory against an archive that contains
non-relocatable ELFs with the same name and contents.

Signed-off-by: Aleksei Vetrov 
---
 tests/run-dwfl-report-offline-memory.sh |   7 +++
 tests/test-ar-duplicates.a.bz2  | Bin 0 -> 783 bytes
 2 files changed, 7 insertions(+)
 create mode 100644 tests/test-ar-duplicates.a.bz2

diff --git a/tests/run-dwfl-report-offline-memory.sh 
b/tests/run-dwfl-report-offline-memory.sh
index 644a45dc..85f43f53 100755
--- a/tests/run-dwfl-report-offline-memory.sh
+++ b/tests/run-dwfl-report-offline-memory.sh
@@ -20,7 +20,14 @@
 testfiles testfile-dwfl-report-elf-align-shlib.so
 testfiles testarchive64.a
 
+# echo "int _start(void) { return 0; }" > test.c
+# gcc test.c -nostdlib -static -o test.o
+# ar -r test-ar-duplicates.a test.o test.o test.o
+# bzip2 -zf test-ar-duplicates.a
+testfiles test-ar-duplicates.a
+
 testrun ${abs_builddir}/dwfl-report-offline-memory 
./testfile-dwfl-report-elf-align-shlib.so 1
 testrun ${abs_builddir}/dwfl-report-offline-memory ./testarchive64.a 3
+testrun ${abs_builddir}/dwfl-report-offline-memory ./test-ar-duplicates.a 1
 
 exit 0
diff --git a/tests/test-ar-duplicates.a.bz2 b/tests/test-ar-duplicates.a.bz2
new file mode 100644
index 
..e8168c6bb813bb05292372d6b856e2ca1163a835
GIT binary patch
literal 783
zcmV+q1MvJpT4*^jL0KkKS*O-t`~U>U|NsC0^v!1T{dI5Uazg*-o^**RKmrI*1V9J?
z0RUpbpefJ;J^%m!H~;_|001&HG5`Po00EJZ00u$8007Vc0g<7Q042*yPG7bO$
z27mwzjSPSQ0001FWB>t>Z~y=_003lYWB>pF00Sc+01Sb>Qb4DoG|(YN(+YV{O*J>F
zZ891m=`tHrY3VgR0F2UlO|*o0s(PNL3ovPT-FWk=B;Jux%~F)fXQs4SQOt9gyiE>j
zn6Ga(89!@GYHl-L-dxbs$}&@+(DyVdX=!IB!kuQ))6nI$Da(vh*7{bCREV6#KNqm!
zdu&Mb+M`10By`;OIdVe?-cr&jN>kKstt5_(4hBpYI!%S0RYq!zoTdw-r};8m5SuEI
zu&BS|S>97Ll_Ks%qWX~|eNFX2+)(wDko6K&h@AyU+*7EjB0KJbX(i2-NWr+XprTIu
zB%S!mUnj<+n4}YmKId7>S^5_w!Fy?EWo4tTvdK6U;gV9C5?V$|i8mD;#XsVDVQ%g0=kvP>JW-B*XEa@sbQG}Le!OvXnKQ+tOr1|<*S=e&p2DS+rYDV*_vs1e6
zu1U>irf%B#9wjbP_72;9ds3ZsFn8O_*f5OUXClse^_%iUp6rvfsXM*Jq^>8z@U|Q6
z*J1OxE^e`H$%>n8wk=MZHxzfH?{*p^+fvGr{$BN;1J#puO|@+L>>t^ZP1xPOJkJDd
zG&~LmIfeUCo0!>Vz3RBGUhvbk7`3dlX0%-$MeRv3e^oYHE+>J;`q?pM6W?@SQ#QEc
ze~0GQ%}g2{wdp?SsT`$F_Z(%Ds5sw)!*Wg-`sD8<;cAYoXHLwT6*G9*M;hmeoNOvD
zUXsh63|o(x!uDHDwwJQbnE~C

literal 0
HcmV?d1

-- 
2.43.0.rc1.413.gea7ed67945-goog

[PATCH v2] libdwfl: Correctly handle corefile non-contiguous segments

2023-11-17 Thread Aaron Merey

v1: https://sourceware.org/pipermail/elfutils-devel/2023q4/006644.html

v2 changes:

The size of the uncompressed testcore-noncontig has been reduced from
736M to 54K.

dwfl_addrsegment tests have been added to verify correct handling of
non-contiguous segments.
---
 libdwfl/dwfl_segment_report_module.c |  37 ++-
 tests/.gitignore |   1 +
 tests/Makefile.am|   8 ++--
 tests/dwfl-core-noncontig.c  |  67 +++
 tests/run-dwfl-core-noncontig.sh |  63 +
 tests/testcore-noncontig.bz2 | Bin 0 -> 54684 bytes
 6 files changed, 162 insertions(+), 14 deletions(-)
 create mode 100644 tests/dwfl-core-noncontig.c
 create mode 100755 tests/run-dwfl-core-noncontig.sh
 create mode 100644 tests/testcore-noncontig.bz2

diff --git a/libdwfl/dwfl_segment_report_module.c 
b/libdwfl/dwfl_segment_report_module.c
index 3ef62a7d..09ee37b3 100644
--- a/libdwfl/dwfl_segment_report_module.c
+++ b/libdwfl/dwfl_segment_report_module.c
@@ -737,17 +737,34 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const 
char *name,
&& invalid_elf (module->elf, module->disk_file_has_build_id,
&build_id))
  {
-   elf_end (module->elf);
-   close (module->fd);
-   module->elf = NULL;
-   module->fd = -1;
+   /* If MODULE's build-id doesn't match the disk file's
+  build-id, close ELF only if MODULE and ELF refer to
+  different builds of files with the same name.  This
+  prevents premature closure of the correct ELF in cases
+  where segments of a module are non-contiguous in memory.  */
+   if (name != NULL && module->name[0] != '\0'
+   && strcmp (basename (module->name), basename (name)) == 0)
+ {
+   elf_end (module->elf);
+   close (module->fd);
+   module->elf = NULL;
+   module->fd = -1;
+ }
  }
-   if (module->elf != NULL)
+   else if (module->elf != NULL)
  {
-   /* Ignore this found module if it would conflict in address
-  space with any already existing module of DWFL.  */
+   /* This module has already been reported.  */
skip_this_module = true;
  }
+   else
+ {
+   /* Only report this module if we haven't already done so.  */
+   for (Dwfl_Module *mod = dwfl->modulelist; mod != NULL;
+mod = mod->next)
+ if (mod->low_addr == module_start
+ && mod->high_addr == module_end)
+   skip_this_module = true;
+ }
  }
   if (skip_this_module)
goto out;
@@ -781,10 +798,6 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const 
char *name,
}
 }
 
-  /* Our return value now says to skip the segments contained
- within the module.  */
-  ndx = addr_segndx (dwfl, segment, module_end, true);
-
   /* Examine its .dynamic section to get more interesting details.
  If it has DT_SONAME, we'll use that as the module name.
  If it has a DT_DEBUG, then it's actually a PIE rather than a DSO.
@@ -929,6 +942,8 @@ dwfl_segment_report_module (Dwfl *dwfl, int ndx, const char 
*name,
   ndx = -1;
   goto out;
 }
+  else
+ndx++;
 
   /* We have reported the module.  Now let the caller decide whether we
  should read the whole thing in right now.  */
diff --git a/tests/.gitignore b/tests/.gitignore
index b9aa22ba..5bebb2c4 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -49,6 +49,7 @@
 /dwfl-report-elf-align
 /dwfl-report-offline-memory
 /dwfl-report-segment-contiguous
+/dwfl-core-noncontig
 /dwfllines
 /dwflmodtest
 /dwflsyms
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 7fb8efb1..9f8f7698 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -42,7 +42,7 @@ check_PROGRAMS = arextract arsymtest newfile saridx scnnames 
sectiondump \
  dwfl-bug-addr-overflow arls dwfl-bug-fd-leak \
  dwfl-addr-sect dwfl-bug-report early-offscn \
  dwfl-bug-getmodules dwarf-getmacros dwarf-ranges addrcfi \
- dwarfcfi \
+ dwfl-core-noncontig dwarfcfi \
  test-flag-nobits dwarf-getstring rerequest_tag \
  alldts typeiter typeiter2 low_high_pc \
  test-elf_cntl_gelf_getshdr dwflsyms dwfllines \
@@ -212,7 +212,7 @@ TESTS = run-arextract.sh run-arsymtest.sh run-ar.sh newfile 
test-nlist \
$(asm_TESTS) run-disasm-bpf.sh run-low_high_pc-dw-form-indirect.sh \
run-nvidia-extended-linemap-libdw.sh 
run-nvidia-extended-linemap-readelf.sh \
run-readelf-dw-form-indirect.sh run-strip-largealign.sh \
-   run-

Re: [PATCH] libdw: check offset dwarf_formstring in all cases

☺ Buildbot (Sourceware): elfutils - build successful (main)

[PATCH 1/2] libdwfl: handle duplicate ELFs when reporting archives

[PATCH 2/2] tests: Add test for duplicate entries in archive

[PATCH v2] libdwfl: Correctly handle corefile non-contiguous segments

5 matches

Site Navigation

Mail list logo

Footer information