libelf: Fix elf_newdata when raw ELF file/image data is available.

When ELF data for a section has been read by elf_rawdata, data_read
and rawdata_base are set, but data_list_rear will not be set until the
data will be converted (by elf_getdata). elf_newdata would overwrite
the existing data in that case. Both elf_getdata and elf_update rely
on the fact that when data_list_rear is set they don't have to look
at the raw data anymore. So make sure we update the data list properly
before adding any new data and raw data is available in elf_newdata.

Add newdata test that calls elf_newdata before and after elf_rawdata
and elf_getdata and checks the new size and contents of the section.

Signed-off-by: Mark Wielaard <[email protected]>

4 files changed, 89 insertions, 41 deletions

diff --git a/libelf/ChangeLog b/libelf/ChangeLog
index 0ee94d4e..15485029 100644
--- a/libelf/ChangeLog
+++ b/libelf/ChangeLog
@@ -1,5 +1,14 @@
 2015-01-20  Mark Wielaard  <[email protected]>
 
+	* libelfP.h (__elf_strptr_internal): New function declaration.
+	* elf_getdata.c (__libelf_set_data_list_rdlock): New internal
+	function extracted from...
+	(__elf_getdata_rdlock): ... here.
+	* elf_newdata.c (elf_newdata): Check scn->rawdata_base and update
+	datalist if necessary.
+
+2015-01-20  Mark Wielaard  <[email protected]>
+
 	* elf_strptr.c (elf_strptr): Call __elf[32|64]_getshdr_rdlock if
 	necessary.
 
diff --git a/libelf/elf_getdata.c b/libelf/elf_getdata.c
index 0aeb9972..e8f022f9 100644
--- a/libelf/elf_getdata.c
+++ b/libelf/elf_getdata.c
@@ -337,6 +337,44 @@ __libelf_set_rawdata (Elf_Scn *scn)
   return result;
 }
 
+void
+internal_function
+__libelf_set_data_list_rdlock (Elf_Scn *scn, int wrlocked)
+{
+  if (scn->rawdata.d.d_buf != NULL && scn->rawdata.d.d_size > 0)
+    {
+      Elf *elf = scn->elf;
+
+      /* Upgrade the lock to a write lock if necessary and check
+	 nobody else already did the work.  */
+      if (!wrlocked)
+	{
+	  rwlock_unlock (elf->lock);
+	  rwlock_wrlock (elf->lock);
+	  if (scn->data_list_rear != NULL)
+	    return;
+	}
+
+      /* Convert according to the version and the type.   */
+      convert_data (scn, __libelf_version, elf->class,
+		    (elf->class == ELFCLASS32
+		     || (offsetof (struct Elf, state.elf32.ehdr)
+			 == offsetof (struct Elf, state.elf64.ehdr))
+		     ? elf->state.elf32.ehdr->e_ident[EI_DATA]
+		     : elf->state.elf64.ehdr->e_ident[EI_DATA]),
+		    scn->rawdata.d.d_size, scn->rawdata.d.d_type);
+    }
+  else
+    {
+      /* This is an empty or NOBITS section.  There is no buffer but
+	 the size information etc is important.  */
+      scn->data_list.data.d = scn->rawdata.d;
+      scn->data_list.data.s = scn;
+    }
+
+  scn->data_list_rear = &scn->data_list;
+}
+
 Elf_Data *
 internal_function
 __elf_getdata_rdlock (scn, data)
@@ -427,42 +465,10 @@ __elf_getdata_rdlock (scn, data)
      empty in case the section has size zero (for whatever reason).
      Now create the converted data in case this is necessary.  */
   if (scn->data_list_rear == NULL)
-    {
-      if (scn->rawdata.d.d_buf != NULL && scn->rawdata.d.d_size > 0)
-	{
-	  if (!locked)
-	    {
-	      rwlock_unlock (elf->lock);
-	      rwlock_wrlock (elf->lock);
-	      if (scn->data_list_rear != NULL)
-		goto pass;
-	    }
-
-	  /* Convert according to the version and the type.   */
-	  convert_data (scn, __libelf_version, elf->class,
-			(elf->class == ELFCLASS32
-			 || (offsetof (struct Elf, state.elf32.ehdr)
-			     == offsetof (struct Elf, state.elf64.ehdr))
-			 ? elf->state.elf32.ehdr->e_ident[EI_DATA]
-			 : elf->state.elf64.ehdr->e_ident[EI_DATA]),
-			scn->rawdata.d.d_size, scn->rawdata.d.d_type);
-	}
-      else
-	{
-	  /* This is an empty or NOBITS section.  There is no buffer but
-	     the size information etc is important.  */
-	  scn->data_list.data.d = scn->rawdata.d;
-	  scn->data_list.data.s = scn;
-	}
-
-      scn->data_list_rear = &scn->data_list;
-    }
+    __libelf_set_data_list_rdlock (scn, locked);
 
-  /* If no data is present we cannot return any.  */
-  if (scn->data_list_rear != NULL)
-  pass:
-    /* Return the first data element in the list.  */
-    result = &scn->data_list.data.d;
+  /* Return the first data element in the list.  */
+  result = &scn->data_list.data.d;
 
  out:
   return result;
diff --git a/libelf/elf_newdata.c b/libelf/elf_newdata.c
index 90d18133..f6609a80 100644
--- a/libelf/elf_newdata.c
+++ b/libelf/elf_newdata.c
@@ -1,5 +1,5 @@
 /* Create new, empty section data.
-   Copyright (C) 1998, 1999, 2000, 2001, 2002 Red Hat, Inc.
+   Copyright (C) 1998, 1999, 2000, 2001, 2002, 2015 Red Hat, Inc.
    This file is part of elfutils.
    Contributed by Ulrich Drepper <[email protected]>, 1998.
 
@@ -64,6 +64,25 @@ elf_newdata (Elf_Scn *scn)
 
   rwlock_wrlock (scn->elf->lock);
 
+  /* data_read is set when data has been read from the ELF image or
+     when a new section has been created by elf_newscn.  If data has
+     been read from the ELF image, then rawdata_base will point to raw
+     data.  If data_read has been set by elf_newscn, then rawdata_base
+     will be NULL.  data_list_rear will be set by elf_getdata if the
+     data has been converted, or by this function, elf_newdata, when
+     new data has been added.
+
+     Currently elf_getdata and elf_update rely on the fact that when
+     data_list_read is not NULL all they have to do is walk the data
+     list. They will ignore any (unread) raw data in that case.
+
+     So we need to make sure the data list is setup if there is
+     already data available.  */
+  if (scn->data_read
+      && scn->rawdata_base != NULL
+      && scn->data_list_rear == NULL)
+    __libelf_set_data_list_rdlock (scn, 1);
+
   if (scn->data_read && scn->data_list_rear == NULL)
     {
       /* This means the section was created by the user and this is the
@@ -73,6 +92,19 @@ elf_newdata (Elf_Scn *scn)
     }
   else
     {
+      /* It would be more efficient to create new data without
+	 reading/converting the data from the file.  But then we
+	 have to remember this.  Currently elf_getdata and
+	 elf_update rely on the fact that they don't have to
+	 load/convert any data if data_list_rear is set.  */
+      if (scn->data_read == 0)
+	{
+	  if (__libelf_set_rawdata_wrlock (scn) != 0)
+	    /* Something went wrong.  The error value is already set.  */
+	    goto out;
+	  __libelf_set_data_list_rdlock (scn, 1);
+	}
+
       /* Create a new, empty data descriptor.  */
       result = (Elf_Data_List *) calloc (1, sizeof (Elf_Data_List));
       if (result == NULL)
@@ -82,11 +114,6 @@ elf_newdata (Elf_Scn *scn)
 	}
 
       result->flags = ELF_F_DIRTY | ELF_F_MALLOCED;
-
-      if (scn->data_list_rear == NULL)
-	/* We create new data without reading/converting the data from the
-	   file.  That is fine but we have to remember this.  */
-	scn->data_list_rear = &scn->data_list;
     }
 
   /* Set the predefined values.  */
diff --git a/libelf/libelfP.h b/libelf/libelfP.h
index 3b24e75c..0ad4071d 100644
--- a/libelf/libelfP.h
+++ b/libelf/libelfP.h
@@ -532,6 +532,12 @@ extern Elf_Data *__elf_getdata_rdlock (Elf_Scn *__scn, Elf_Data *__data)
      internal_function;
 extern Elf_Data *__elf_rawdata_internal (Elf_Scn *__scn, Elf_Data *__data)
      attribute_hidden;
+/* Should be called to setup first section data element if
+   data_list_rear is NULL and we know data_read is set and there is
+   raw data available.  Might upgrade the ELF lock from a read to a
+   write lock.  If the lock is already a write lock set wrlocked.  */
+extern void __libelf_set_data_list_rdlock (Elf_Scn *scn, int wrlocked)
+  attribute_hidden;
 extern char *__elf_strptr_internal (Elf *__elf, size_t __index,
 				    size_t __offset) attribute_hidden;
 extern Elf_Data *__elf32_xlatetom_internal (Elf_Data *__dest,