[Bug tools/25082] Multiple crashes in eu-unstrip
https://sourceware.org/bugzilla/show_bug.cgi?id=25082 Mark Wielaard changed: What|Removed |Added Status|UNCONFIRMED |ASSIGNED Last reconfirmed||2019-10-21 CC||mark at klomp dot org Assignee|unassigned at sourceware dot org |mark at klomp dot org Ever confirmed|0 |1 --- Comment #3 from Mark Wielaard --- Created attachment 12047 --> https://sourceware.org/bugzilla/attachment.cgi?id=12047&action=edit unstrip: Add various checks for bad input data eu-unstrip was clearly not written for bad ELF input files. Not surprisingly because it would be slightly odd to run it on untrusted input, which wasn't just stripped in two. But I have added a couple of robustness fixed that should at least not make it crash and give an error message that will hopefully explain what is wrong with the input files. -- You are receiving this mail because: You are on the CC list for the bug.
Re: [PATCH 2/3] libdw: Rewrite the memory handler to be thread-safe.
Hi,
Finally back to this patch series.
On Thu, 2019-08-29 at 15:16 +0200, Mark Wielaard wrote:
> diff --git a/libdw/dwarf_end.c b/libdw/dwarf_end.c
> index 29795c10..fc573cb3 100644
> --- a/libdw/dwarf_end.c
> +++ b/libdw/dwarf_end.c
> @@ -94,14 +94,15 @@ dwarf_end (Dwarf *dwarf)
>/* And the split Dwarf. */
>tdestroy (dwarf->split_tree, noop_free);
>
> - struct libdw_memblock *memp = dwarf->mem_tail;
> - /* The first block is allocated together with the Dwarf object. */
> - while (memp->prev != NULL)
> + /* Free the internally allocated memory. */
> + struct libdw_memblock *memp = (struct libdw_memblock *)dwarf->mem_tail;
> + while (memp != NULL)
> {
> struct libdw_memblock *prevp = memp->prev;
> free (memp);
> memp = prevp;
> }
> + pthread_key_delete (dwarf->mem_key);
>
>/* Free the pubnames helper structure. */
>free (dwarf->pubnames_sets);
This doesn't build on older GCCs (I am using 4.8.5) with this compile error:
libdw/dwarf_end.c: In function ‘dwarf_end’:
libdw/dwarf_end.c:98:45: error: cannot convert to a pointer type
struct libdw_memblock *memp = (struct libdw_memblock *)dwarf->mem_tail;
^
This is because mem_tail is defined as:
> diff --git a/libdw/libdwP.h b/libdw/libdwP.h
> index eebb7d12..ad2599eb 100644
> --- a/libdw/libdwP.h
> +++ b/libdw/libdwP.h
> @@ -218,16 +231,11 @@ struct Dwarf
>/* Similar for addrx/constx, which will come from .debug_addr section. */
>struct Dwarf_CU *fake_addr_cu;
>
> - /* Internal memory handling. This is basically a simplified
> - reimplementation of obstacks. Unfortunately the standard obstack
> - implementation is not usable in libraries. */
> - struct libdw_memblock
> - {
> -size_t size;
> -size_t remaining;
> -struct libdw_memblock *prev;
> -char mem[0];
> - } *mem_tail;
> + /* Internal memory handling. Each thread allocates separately and only
> + allocates from its own blocks, while all the blocks are pushed
> atomically
> + onto a unified stack for easy deallocation. */
> + pthread_key_t mem_key;
> + atomic_uintptr_t mem_tail;
>
>/* Default size of allocated memory blocks. */
>size_t mem_default_size;
And for older compilers, without stdatomic.h, this means atomic_uintptr_t is
really:
typedef _Atomic(uintptr_t) atomic_uintptr_t;
#define _Atomic(T) struct { volatile __typeof__(T) __val; }
And you cannot cast a struct to a pointer type directly.
To make this work on both older and newer gcc versions I changed this to:
diff --git a/libdw/dwarf_end.c b/libdw/dwarf_end.c
index fc573cb3..76ab9954 100644
--- a/libdw/dwarf_end.c
+++ b/libdw/dwarf_end.c
@@ -95,7 +95,8 @@ dwarf_end (Dwarf *dwarf)
tdestroy (dwarf->split_tree, noop_free);
/* Free the internally allocated memory. */
- struct libdw_memblock *memp = (struct libdw_memblock *)dwarf->mem_tail;
+ struct libdw_memblock *memp;
+ memp = (struct libdw_memblock *) atomic_load (&dwarf->mem_tail);
while (memp != NULL)
{
struct libdw_memblock *prevp = memp->prev;
Does that look reasonable?
Thanks,
Mark
Re: [PATCH 2/3] libdw: Rewrite the memory handler to be thread-safe.
On Mon, Oct 21, 2019 at 18:13, Mark Wielaard wrote:
Hi,
Finally back to this patch series.
On Thu, 2019-08-29 at 15:16 +0200, Mark Wielaard wrote:
diff --git a/libdw/dwarf_end.c b/libdw/dwarf_end.c
index 29795c10..fc573cb3 100644
--- a/libdw/dwarf_end.c
+++ b/libdw/dwarf_end.c
@@ -94,14 +94,15 @@ dwarf_end (Dwarf *dwarf)
/* And the split Dwarf. */
tdestroy (dwarf->split_tree, noop_free);
- struct libdw_memblock *memp = dwarf->mem_tail;
- /* The first block is allocated together with the Dwarf
object. */
- while (memp->prev != NULL)
+ /* Free the internally allocated memory. */
+ struct libdw_memblock *memp = (struct libdw_memblock
*)dwarf->mem_tail;
+ while (memp != NULL)
{
struct libdw_memblock *prevp = memp->prev;
free (memp);
memp = prevp;
}
+ pthread_key_delete (dwarf->mem_key);
/* Free the pubnames helper structure. */
free (dwarf->pubnames_sets);
This doesn't build on older GCCs (I am using 4.8.5) with this compile
error:
libdw/dwarf_end.c: In function ‘dwarf_end’:
libdw/dwarf_end.c:98:45: error: cannot convert to a pointer type
struct libdw_memblock *memp = (struct libdw_memblock
*)dwarf->mem_tail;
^
Ah, whoops. Thanks for catching that one.
This is because mem_tail is defined as:
diff --git a/libdw/libdwP.h b/libdw/libdwP.h
index eebb7d12..ad2599eb 100644
--- a/libdw/libdwP.h
+++ b/libdw/libdwP.h
@@ -218,16 +231,11 @@ struct Dwarf
/* Similar for addrx/constx, which will come from .debug_addr
section. */
struct Dwarf_CU *fake_addr_cu;
- /* Internal memory handling. This is basically a simplified
- reimplementation of obstacks. Unfortunately the standard
obstack
- implementation is not usable in libraries. */
- struct libdw_memblock
- {
-size_t size;
-size_t remaining;
-struct libdw_memblock *prev;
-char mem[0];
- } *mem_tail;
+ /* Internal memory handling. Each thread allocates separately
and only
+ allocates from its own blocks, while all the blocks are
pushed atomically
+ onto a unified stack for easy deallocation. */
+ pthread_key_t mem_key;
+ atomic_uintptr_t mem_tail;
/* Default size of allocated memory blocks. */
size_t mem_default_size;
And for older compilers, without stdatomic.h, this means
atomic_uintptr_t is really:
typedef _Atomic(uintptr_t) atomic_uintptr_t;
#define _Atomic(T) struct { volatile
__typeof__(T) __val; }
And you cannot cast a struct to a pointer type directly.
To make this work on both older and newer gcc versions I changed this
to:
diff --git a/libdw/dwarf_end.c b/libdw/dwarf_end.c
index fc573cb3..76ab9954 100644
--- a/libdw/dwarf_end.c
+++ b/libdw/dwarf_end.c
@@ -95,7 +95,8 @@ dwarf_end (Dwarf *dwarf)
tdestroy (dwarf->split_tree, noop_free);
/* Free the internally allocated memory. */
- struct libdw_memblock *memp = (struct libdw_memblock
*)dwarf->mem_tail;
+ struct libdw_memblock *memp;
+ memp = (struct libdw_memblock *) atomic_load
(&dwarf->mem_tail);
while (memp != NULL)
{
struct libdw_memblock *prevp = memp->prev;
Does that look reasonable?
It does, although I would prefer:
diff --git a/libdw/dwarf_end.c b/libdw/dwarf_end.c
index 9ca17212..6da9e0cd 100644
--- a/libdw/dwarf_end.c
+++ b/libdw/dwarf_end.c
@@ -95,7 +95,9 @@ dwarf_end (Dwarf *dwarf)
tdestroy (dwarf->split_tree, noop_free);
/* Free the internally allocated memory. */
- struct libdw_memblock *memp = (struct libdw_memblock
*)dwarf->mem_tail;
+ struct libdw_memblock *memp;
+ memp = (struct libdw_memblock *)atomic_load(&dwarf->mem_tail,
+
memory_order_relaxed);
while (memp != NULL)
{
struct libdw_memblock *prevp = memp->prev;
Because some idiot thought making seq_cst the default was a good idea.
And this way it notes in the code that this load is non-synchronizing.
-Jonathon
Thanks,
Mark
Re: [PATCH 2/3] libdw: Rewrite the memory handler to be thread-safe.
Hi,
On Mon, 2019-10-21 at 11:28 -0500, Jonathon Anderson wrote:
> On Mon, Oct 21, 2019 at 18:13, Mark Wielaard wrote:
> > Does that look reasonable?
>
> It does, although I would prefer:
>
> diff --git a/libdw/dwarf_end.c b/libdw/dwarf_end.c
> index 9ca17212..6da9e0cd 100644
> --- a/libdw/dwarf_end.c
> +++ b/libdw/dwarf_end.c
> @@ -95,7 +95,9 @@ dwarf_end (Dwarf *dwarf)
>tdestroy (dwarf->split_tree, noop_free);
>
>/* Free the internally allocated memory. */
> - struct libdw_memblock *memp = (struct libdw_memblock
> *)dwarf->mem_tail;
> + struct libdw_memblock *memp;
> + memp = (struct libdw_memblock *)atomic_load(&dwarf->mem_tail,
> + memory_order_relaxed);
>while (memp != NULL)
> {
>struct libdw_memblock *prevp = memp->prev;
>
> Because some idiot thought making seq_cst the default was a good idea.
> And this way it notes in the code that this load is non-synchronizing.
Lets avoid the "strong" language about people. But lets see if we can
make the load less "strong" for the atomics :)
I think we cannot use the atomic_load () function, but have to use
atomic_load_explicit. So it would become:
diff --git a/libdw/dwarf_end.c b/libdw/dwarf_end.c
index fc573cb3..a2e94436 100644
--- a/libdw/dwarf_end.c
+++ b/libdw/dwarf_end.c
@@ -95,7 +95,10 @@ dwarf_end (Dwarf *dwarf)
tdestroy (dwarf->split_tree, noop_free);
/* Free the internally allocated memory. */
- struct libdw_memblock *memp = (struct libdw_memblock *)dwarf->mem_tail;
+ struct libdw_memblock *memp;
+ memp = (struct libdw_memblock *) (atomic_load_explicit
+ (&dwarf->mem_tail,
+memory_order_relaxed));
while (memp != NULL)
{
struct libdw_memblock *prevp = memp->prev;
Cheers,
Mark
[Bug tools/25077] unstrip bad handling of sh_entsize of the symver section
https://sourceware.org/bugzilla/show_bug.cgi?id=25077 Mark Wielaard changed: What|Removed |Added Component|libelf |tools Assignee|unassigned at sourceware dot org |mark at klomp dot org Summary|AddressSanitizer: |unstrip bad handling of |heap-buffer-overflow at |sh_entsize of the symver |libelf/elf32_updatefile.c:7 |section |72 | -- You are receiving this mail because: You are on the CC list for the bug.
[Bug tools/25083] unstrip tries to write out an enormous amount of data
https://sourceware.org/bugzilla/show_bug.cgi?id=25083 Mark Wielaard changed: What|Removed |Added Component|libelf |tools Assignee|unassigned at sourceware dot org |mark at klomp dot org Summary|Unexpected hangs at |unstrip tries to write out |elf32_updatefile.c:518 |an enormous amount of data -- You are receiving this mail because: You are on the CC list for the bug.
[Bug tools/25077] unstrip bad handling of sh_entsize of the symver section
https://sourceware.org/bugzilla/show_bug.cgi?id=25077 --- Comment #2 from leftcopy.chx at gmail dot com --- Since this crashes occurs when calling `elf_update`, which resides in libelf, I suppose this is a libelf library issue. I'd suggest adding some documents to warn that it is the developers' duty to ensure that the claimed symver size matches with the actual one, or better resolves the size purely inside libelf code? -- You are receiving this mail because: You are on the CC list for the bug.
