src/readelf.c: Close skel_fd

Message ID 20250125013258.2694909-1-amerey@redhat.com
State Committed
Delegated to: Mark Wielaard
Headers
Series src/readelf.c: Close skel_fd |

Commit Message

Aaron Merey Jan. 25, 2025, 1:32 a.m. UTC
  skel_fd is passed to create_dwfl, which calls dup() on skel_fd.
create_dwfl handles closing the dup'ed fd but not the original.

Ensure the original skel_fd is closed after it's passed to create_dwfl.

Signed-off-by: Aaron Merey <amerey@redhat.com>
---
 src/readelf.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)
  

Comments

Mark Wielaard Jan. 25, 2025, 11:40 p.m. UTC | #1
Hi Aaron,

On Fri, Jan 24, 2025 at 08:32:58PM -0500, Aaron Merey wrote:
> skel_fd is passed to create_dwfl, which calls dup() on skel_fd.
> create_dwfl handles closing the dup'ed fd but not the original.
> 
> Ensure the original skel_fd is closed after it's passed to create_dwfl.

Nice find.

We should add --track-fds=yes to our valgrind testing.  It would have
found this earlier. Two of our tests currently fail with:

-==75005== Open file descriptor 5: testfile-splitdwarf-4
-==75005==    at 0x4A1E023: open (in /usr/lib64/libc.so.6)
-==75005==    by 0x41392B: open (fcntl2.h:55)
-==75005==    by 0x41392B: print_debug (readelf.c:11919)
-==75005==    by 0x416451: process_elf_file (readelf.c:1084)
-==75005==    by 0x4170CC: process_dwflmod (readelf.c:840)
-==75005==    by 0x489CAE0: dwfl_getmodules (dwfl_getmodules.c:86)
-==75005==    by 0x406964: process_file (readelf.c:948)
-==75005==    by 0x401D11: main (readelf.c:417)

This file descriptor leak disappears with your patch.

Thanks,

Mark

> Signed-off-by: Aaron Merey <amerey@redhat.com>
> ---
>  src/readelf.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/src/readelf.c b/src/readelf.c
> index 3e97b64c..6526db07 100644
> --- a/src/readelf.c
> +++ b/src/readelf.c
> @@ -11921,7 +11921,13 @@ print_debug (Dwfl_Module *dwflmod, Ebl *ebl, GElf_Ehdr *ehdr)
>  		fprintf (stderr, "Warning: Couldn't open DWARF skeleton file"
>  			 " '%s'\n", skel_name);
>  	      else
> -		skel_dwfl = create_dwfl (skel_fd, skel_name);
> +		{
> +		  skel_dwfl = create_dwfl (skel_fd, skel_name);
> +
> +		  /* skel_fd was dup'ed by create_dwfl.  We can close the
> +		     original now.  */
> +		  close (skel_fd);
> +		}
>  
>  	      if (skel_dwfl != NULL)
>  		{
> -- 
> 2.47.1
>
  

Patch

diff --git a/src/readelf.c b/src/readelf.c
index 3e97b64c..6526db07 100644
--- a/src/readelf.c
+++ b/src/readelf.c
@@ -11921,7 +11921,13 @@  print_debug (Dwfl_Module *dwflmod, Ebl *ebl, GElf_Ehdr *ehdr)
 		fprintf (stderr, "Warning: Couldn't open DWARF skeleton file"
 			 " '%s'\n", skel_name);
 	      else
-		skel_dwfl = create_dwfl (skel_fd, skel_name);
+		{
+		  skel_dwfl = create_dwfl (skel_fd, skel_name);
+
+		  /* skel_fd was dup'ed by create_dwfl.  We can close the
+		     original now.  */
+		  close (skel_fd);
+		}
 
 	      if (skel_dwfl != NULL)
 		{