[binutils-gdb] gdb/corelow.c: fix use-after-free in build_file_mappings

[Lancelot SIX]

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=9b3a1001c8405965b3142d7361c13049714e543f

commit 9b3a1001c8405965b3142d7361c13049714e543f
Author: Lancelot SIX <lancelot.six@amd.com>
Date:   Wed May 31 11:35:32 2023 +0100

    gdb/corelow.c: fix use-after-free in build_file_mappings
    
    In core_target::build_file_mappings, GDB tries to open files referenced
    in the core dump.
    
    The process goes like this:
    
        struct bfd *bfd = bfd_map[filename];
        if (bfd == nullptr)
          {
            bfd = bfd_map[filename]
              = bfd_openr (expanded_fname.get (), "binary");
            if (bfd == nullptr || !bfd_check_format (bfd, bfd_object))
              {
                if (bfd != nullptr)
                  bfd_close (bfd);
                return;
              }
          }
        asection *sec = bfd_make_section_anyway (bfd, "load");
        ...
    
    The problem is that if bfd_check_format fails, we close the bfd but keep
    a reference to it in the bfd_map.
    
    If the same filename appears another time in the NT_FILE note, we enter
    this code again.  The second time, bfd_map[filename] is not nullptr and
    we try to call bfd_make_section_anyway on an already closed BFD, which
    is a use-after-free error.
    
    This patch makes sure that the bfd is only saved in the bfd_map if it
    got opened successfully.
    
    This error got exposed by a recent change in BFD (014a602b86f "Don't
    optimise bfd_seek to same position").  Since this change, opening a
    coredump which contains mapping to some special files such as a DRI
    render node (/dev/dri/renderD$NUM) exposes the issue.  This happens for
    example for processes using AMDGPU devices to offload compute tasks.
    
    Reviewed-By: John Baldwin <jhb@FreeBSD.org>
    Approved-By: Andrew Burgess <aburgess@redhat.com>

Diff:
---
 gdb/corelow.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/gdb/corelow.c b/gdb/corelow.c
index db489b4280e..54def4198bc 100644
--- a/gdb/corelow.c
+++ b/gdb/corelow.c
@@ -258,8 +258,7 @@ core_target::build_file_mappings ()
 		return;
 	      }
 
-	    bfd = bfd_map[filename] = bfd_openr (expanded_fname.get (),
-						 "binary");
+	    bfd = bfd_openr (expanded_fname.get (), "binary");
 
 	    if (bfd == nullptr || !bfd_check_format (bfd, bfd_object))
 	      {
@@ -284,6 +283,7 @@ core_target::build_file_mappings ()
 	       This can be checked before/after a core file detach via
 	       "maint info bfds".  */
 	    gdb_bfd_record_inclusion (core_bfd, bfd);
+	    bfd_map[filename] = bfd;
 	  }
 
 	/* Make new BFD section.  All sections have the same name,