[PATCH 01/10 v3] libelf: Fix deadlock in __libelf_readall

Aaron Merey amerey@redhat.com
Fri Aug 2 23:38:00 GMT 2024


From: Heather McIntyre <hsm2@rice.edu>

Apply locking during __libelf_readall.

Signed-off-by: Heather S. McIntyre <hsm2@rice.edu>
Signed-off-by: Aaron Merey <amerey@redhat.com>
Signed-off-by: Mark Wielaard <mark@klomp.org>

---
v3 changes:

Update comments and change order or child lock aquisition.

 libelf/common.h      | 24 ++++++++++++++----------
 libelf/elf_readall.c |  4 ++--
 2 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/libelf/common.h b/libelf/common.h
index 9b2a856d..8e33a70e 100644
--- a/libelf/common.h
+++ b/libelf/common.h
@@ -89,30 +89,34 @@ allocate_elf (int fildes, void *map_address, int64_t offset, size_t maxsize,
 }
 
 
-/* Acquire lock for the descriptor and all children.  */
+/* Caller must hold a lock for ELF. If there are children then a lock
+   will be acquired for each of them (recursively).  */
 static void
 __attribute__ ((unused))
-libelf_acquire_all (Elf *elf)
+libelf_acquire_all_children (Elf *elf)
 {
-  rwlock_wrlock (elf->lock);
-
   if (elf->kind == ELF_K_AR)
     {
       Elf *child = elf->state.ar.children;
 
       while (child != NULL)
 	{
+	  rwlock_wrlock (child->lock);
+
 	  if (child->ref_count != 0)
-	    libelf_acquire_all (child);
+	    libelf_acquire_all_children (child);
+
 	  child = child->next;
 	}
     }
 }
 
-/* Release own lock and those of the children.  */
+
+/* Caller must hold a lock for ELF. If there are children then a lock
+   will be released for each of them (recursively).  */
 static void
 __attribute__ ((unused))
-libelf_release_all (Elf *elf)
+libelf_release_all_children (Elf *elf)
 {
   if (elf->kind == ELF_K_AR)
     {
@@ -121,12 +125,12 @@ libelf_release_all (Elf *elf)
       while (child != NULL)
 	{
 	  if (child->ref_count != 0)
-	    libelf_release_all (child);
+	    libelf_release_all_children (child);
+
+	  rwlock_unlock (child->lock);
 	  child = child->next;
 	}
     }
-
-  rwlock_unlock (elf->lock);
 }
 
 
diff --git a/libelf/elf_readall.c b/libelf/elf_readall.c
index d0f9a28c..4ef8fe97 100644
--- a/libelf/elf_readall.c
+++ b/libelf/elf_readall.c
@@ -84,7 +84,7 @@ __libelf_readall (Elf *elf)
 
       /* If this is an archive and we have derived descriptors get the
 	 locks for all of them.  */
-      libelf_acquire_all (elf);
+      libelf_acquire_all_children (elf);
 
       if (elf->maximum_size == ~((size_t) 0))
 	{
@@ -141,7 +141,7 @@ __libelf_readall (Elf *elf)
 	__libelf_seterrno (ELF_E_NOMEM);
 
       /* Free the locks on the children.  */
-      libelf_release_all (elf);
+      libelf_release_all_children (elf);
     }
 
   rwlock_unlock (elf->lock);
-- 
2.45.2



More information about the Elfutils-devel mailing list