__libdw_getabbrev may reparse a cached abbrev when argument lengthp is
non-NULL.  This reparsing is the same as for an uncached abbrev seen for
the first time.  The fields of the abbrev are written to whether or
not they've already been set.

This can create a data race for multithreaded use cases.  Fix this by
not writing to the abbrev if it is already cached.

Signed-off-by: Aaron Merey <[email protected]>
---
 libdw/dwarf_getabbrev.c | 24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/libdw/dwarf_getabbrev.c b/libdw/dwarf_getabbrev.c
index d9a6c022..26cf38e8 100644
--- a/libdw/dwarf_getabbrev.c
+++ b/libdw/dwarf_getabbrev.c
@@ -107,18 +107,26 @@ __libdw_getabbrev (Dwarf *dbg, struct Dwarf_CU *cu, 
Dwarf_Off offset,
        goto out;
     }
 
-  /* If there is already a value in the hash table we are going to
-     overwrite its content.  This must not be a problem, since the
-     content better be the same.  */
-  abb->code = code;
   if (abbrevp >= end)
     goto invalid;
-  get_uleb128 (abb->tag, abbrevp, end);
+
+  unsigned int tag;
+  get_uleb128 (tag, abbrevp, end);
+
   if (abbrevp + 1 >= end)
     goto invalid;
-  abb->has_children = *abbrevp++ == DW_CHILDREN_yes;
-  abb->attrp = (unsigned char *) abbrevp;
-  abb->offset = offset;
+
+  bool has_children = *abbrevp++ == DW_CHILDREN_yes;
+
+  /* Set the entry's fields if it was just allocated.  */
+  if (! foundit)
+    {
+      abb->code = code;
+      abb->tag = tag;
+      abb->has_children = has_children;
+      abb->attrp = (unsigned char *) abbrevp;
+      abb->offset = offset;
+    }
 
   /* Skip over all the attributes and check rest of the abbrev is valid.  */
   unsigned int attrname;
-- 
2.55.0

Reply via email to