diff mbox series

[05/10,v3] libdw: make dwarf_getalt and dwarf_setalt thread-safe

Message ID 20240802233847.690564-5-amerey@redhat.com
State Committed
Delegated to: Mark Wielaard
Headers
Series [01/10,v3] libelf: Fix deadlock in __libelf_readall |

Commit Message

Aaron Merey Aug. 2, 2024, 11:38 p.m. UTC 
  From: Heather McIntyre <hsm2@rice.edu>

	* libdw/dwarf_getalt.c (dwarf_getalt): Add locking.
	* libdw/dwarf_setalt.c (dwarf_setalt): Ditto.

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:

Check for NULL argument before acquiring lock. Assign result before
releasing lock.  Include dwarf_setalt.c changes that were previously
included in patch 5/9 v2.

 libdw/dwarf_getalt.c | 31 +++++++++++++++++++++++++++----
 libdw/dwarf_setalt.c |  3 +++
 2 files changed, 30 insertions(+), 4 deletions(-)

Comments

Mark Wielaard Aug. 16, 2024, 1:45 p.m. UTC | #1 
On Fri, 2024-08-02 at 19:38 -0400, Aaron Merey wrote:
> From: Heather McIntyre <hsm2@rice.edu>
> 
> 	* libdw/dwarf_getalt.c (dwarf_getalt): Add locking.
> 	* libdw/dwarf_setalt.c (dwarf_setalt): Ditto.
> 
> 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:
> 
> Check for NULL argument before acquiring lock. Assign result before
> releasing lock.  Include dwarf_setalt.c changes that were previously
> included in patch 5/9 v2.

Cleanups look good. This patch looks good to go.

Thanks,

Mark
Aaron Merey Aug. 21, 2024, 12:31 a.m. UTC | #2 
Hi Mark,

On Fri, Aug 16, 2024 at 9:45 AM Mark Wielaard <mark@klomp.org> wrote:
>
> On Fri, 2024-08-02 at 19:38 -0400, Aaron Merey wrote:
> > From: Heather McIntyre <hsm2@rice.edu>
> >
> >       * libdw/dwarf_getalt.c (dwarf_getalt): Add locking.
> >       * libdw/dwarf_setalt.c (dwarf_setalt): Ditto.
> >
> > 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:
> >
> > Check for NULL argument before acquiring lock. Assign result before
> > releasing lock.  Include dwarf_setalt.c changes that were previously
> > included in patch 5/9 v2.
>
> Cleanups look good. This patch looks good to go.

Thanks, pushed as commit 7c4fcff44ae.

Aaron
diff mbox series

Patch

diff --git a/libdw/dwarf_getalt.c b/libdw/dwarf_getalt.c
index 0a12dfae..26433b81 100644
--- a/libdw/dwarf_getalt.c
+++ b/libdw/dwarf_getalt.c
@@ -160,15 +160,34 @@  find_debug_altlink (Dwarf *dbg)
     }
 }
 
+/* find_debug_altlink() modifies "dbg->alt_dwarf".
+   dwarf_getalt() reads "main->alt_dwarf".
+   Mutual exclusion is enforced to prevent a race. */
+
 Dwarf *
 dwarf_getalt (Dwarf *main)
 {
-  /* Only try once.  */
-  if (main == NULL || main->alt_dwarf == (void *) -1)
+  if (main == NULL)
     return NULL;
 
+  rwlock_wrlock(main->dwarf_lock);
+
+  /* Only try once.  */
+  if (main->alt_dwarf == (void *) -1)
+    {
+      rwlock_unlock (main->dwarf_lock);
+      return NULL;
+    }
+
+  /* Assign result before releasing the lock.  */
+  Dwarf *result;
+
   if (main->alt_dwarf != NULL)
-    return main->alt_dwarf;
+    {
+      result = main->alt_dwarf;
+      rwlock_unlock (main->dwarf_lock);
+      return result;
+    }
 
   find_debug_altlink (main);
 
@@ -176,9 +195,13 @@  dwarf_getalt (Dwarf *main)
   if (main->alt_dwarf == NULL)
     {
       main->alt_dwarf = (void *) -1;
+      rwlock_unlock (main->dwarf_lock);
       return NULL;
     }
 
-  return main->alt_dwarf;
+  result = main->alt_dwarf;
+  rwlock_unlock (main->dwarf_lock);
+
+  return result;
 }
 INTDEF (dwarf_getalt)
diff --git a/libdw/dwarf_setalt.c b/libdw/dwarf_setalt.c
index dc9b61cb..f98a457c 100644
--- a/libdw/dwarf_setalt.c
+++ b/libdw/dwarf_setalt.c
@@ -35,6 +35,8 @@ 
 void
 dwarf_setalt (Dwarf *main, Dwarf *alt)
 {
+  rwlock_wrlock (main->dwarf_lock);
+
   if (main->alt_fd != -1)
     {
       INTUSE(dwarf_end) (main->alt_dwarf);
@@ -43,5 +45,6 @@  dwarf_setalt (Dwarf *main, Dwarf *alt)
     }
 
   main->alt_dwarf = alt;
+  rwlock_unlock (main->dwarf_lock);
 }
 INTDEF (dwarf_setalt)