diff mbox series

[02/10,v3] libelf: Fix deadlock in elf_cntl

Message ID 20240802233847.690564-2-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>

        * libelf/elf_cntl.c (elf_cntl): Move rwlock_wrlock, rwlock_unlock,
        inside case switch statements.  Remove unnecessary early return.

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:
Remove unnecessary early return that would require locking to
check condition.

 libelf/elf_cntl.c | 14 +++-----------
 1 file changed, 3 insertions(+), 11 deletions(-)

Comments

Mark Wielaard Aug. 15, 2024, 4:16 p.m. UTC | #1 
Hi Aaron,

On Fri, 2024-08-02 at 19:38 -0400, Aaron Merey wrote:
> From: Heather McIntyre <hsm2@rice.edu>
> 
>         * libelf/elf_cntl.c (elf_cntl): Move rwlock_wrlock, rwlock_unlock,
>         inside case switch statements.  Remove unnecessary early return.
> 
> 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:
> Remove unnecessary early return that would require locking to
> check condition.

The code looks good and clean now.

Thanks,

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

On Thu, Aug 15, 2024 at 12:16 PM Mark Wielaard <mark@klomp.org> wrote:
>
> On Fri, 2024-08-02 at 19:38 -0400, Aaron Merey wrote:
> > From: Heather McIntyre <hsm2@rice.edu>
> >
> >         * libelf/elf_cntl.c (elf_cntl): Move rwlock_wrlock, rwlock_unlock,
> >         inside case switch statements.  Remove unnecessary early return.
> >
> > 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:
> > Remove unnecessary early return that would require locking to
> > check condition.
>
> The code looks good and clean now.

Thanks, pushed as commit 97b72c00603.

Aaron
diff mbox series

Patch

diff --git a/libelf/elf_cntl.c b/libelf/elf_cntl.c
index 04aa9132..da4ea999 100644
--- a/libelf/elf_cntl.c
+++ b/libelf/elf_cntl.c
@@ -42,19 +42,11 @@  elf_cntl (Elf *elf, Elf_Cmd cmd)
   if (elf == NULL)
     return -1;
 
-  if (elf->fildes == -1)
-    {
-      __libelf_seterrno (ELF_E_INVALID_HANDLE);
-      return -1;
-    }
-
-  rwlock_wrlock (elf->lock);
-
   switch (cmd)
     {
     case ELF_C_FDREAD:
       /* If not all of the file is in the memory read it now.  */
-      if (elf->map_address == NULL && __libelf_readall (elf) == NULL)
+      if (__libelf_readall (elf) == NULL)
 	{
 	  /* We were not able to read everything.  */
 	  result = -1;
@@ -64,7 +56,9 @@  elf_cntl (Elf *elf, Elf_Cmd cmd)
 
     case ELF_C_FDDONE:
       /* Mark the file descriptor as not usable.  */
+      rwlock_wrlock (elf->lock);
       elf->fildes = -1;
+      rwlock_unlock (elf->lock);
       break;
 
     default:
@@ -73,7 +67,5 @@  elf_cntl (Elf *elf, Elf_Cmd cmd)
       break;
     }
 
-  rwlock_unlock (elf->lock);
-
   return result;
 }