[v4,08/14] Use NT_FILE note section for reading core target memory

Message ID 20200705225807.2264705-9-kevinb@redhat.com
State Superseded
Headers show
Series
  • Fix BZ 25631 - core file memory access problem
Related show

Commit Message

Simon Marchi via Gdb-patches July 5, 2020, 10:58 p.m.
In his reviews of my v1 and v2 corefile related patches, Pedro
identified two cases which weren't handled by those patches.

In https://sourceware.org/pipermail/gdb-patches/2020-May/168826.html,
Pedro showed that debugging a core file in which mmap() is used to
create a new mapping over an existing file-backed mapping will
produce incorrect results.  I.e, for his example, GDB would
show:

(gdb) disassemble main
Dump of assembler code for function main:
   0x00000000004004e6 <+0>:	push   %rbp
   0x00000000004004e7 <+1>:	mov    %rsp,%rbp
=> 0x00000000004004ea <+4>:	callq  0x4003f0 <abort@plt>
End of assembler dump.

This sort of looks like it might be correct, but is not due to the
fact that mmap(...MAP_FIXED...) was used to create a mapping (of all
zeros) on top of the .text section.  So, the correct result should be:

(gdb) disassemble main
Dump of assembler code for function main:
   0x00000000004004e6 <+0>:	add    %al,(%rax)
   0x00000000004004e8 <+2>:	add    %al,(%rax)
=> 0x00000000004004ea <+4>:	add    %al,(%rax)
   0x00000000004004ec <+6>:	add    %al,(%rax)
   0x00000000004004ee <+8>:	add    %al,(%rax)
End of assembler dump.

The other case that Pedro found involved an attempted examination of a
particular section in the test case from gdb.base/corefile.exp.  On
Fedora 27 or 28, the following behavior may be observed:

(gdb) info proc mappings
Mapped address spaces:

          Start Addr           End Addr       Size     Offset objfile
...
      0x7ffff7839000     0x7ffff7a38000   0x1ff000   0x1b5000 /usr/lib64/libc-2.27.so
...
(gdb) x/4x 0x7ffff7839000
0x7ffff7839000:	Cannot access memory at address 0x7ffff7839000

FYI, this section appears to be unrelocated vtable data.  See
https://sourceware.org/pipermail/gdb-patches/2020-May/168331.html for
a detailed analysis.

The important thing here is that GDB should be able to access this
address since it should be backed by the shared library.  I.e. it
should do this:

(gdb) x/4x 0x7ffff7839000
0x7ffff7839000:	0x0007ddf0	0x00000000	0x0007dba0	0x00000000

Both of these cases are fixed with this commit.

In a nutshell, this commit opens a "binary" target BFD for each of the
files that are mentioned in an NT_FILE / .note.linuxcore.file note
section.  It then uses these mappings instead of the file stratum
mappings that GDB has used in the past.

If this note section doesn't exist or is mangled for some reason, then
GDB will use the file stratum as before.  Should this happen, then
we can expect both of the above problems to again be present.

See the code comments in the commit for other details.

gdb/ChangeLog:

	* corelow.c (solist.h, unordered_map): Include.
	(class core_target): Add field m_core_file_mappings and
	method build_file_mappings.
	(core_target::core_target): Call build_file_mappings.
	(core_target::~core_target): Free memory associated with
	m_core_file_mappings.
	(core_target::build_file_mappings): New method.
	(core_target::xfer_partial): Use m_core_file_mappings
	for memory transfers.
	* linux-tdep.c (linux_read_core_file_mappings): New
	function.
	(linux_core_info_proc_mappings): Rewrite to use
	linux_read_core_file_mappings.
	(linux_init_abi): Register linux_read_core_file_mappings.
---
 gdb/corelow.c    | 138 +++++++++++++++++++++++++++++++-
 gdb/linux-tdep.c | 203 +++++++++++++++++++++++++++++++----------------
 2 files changed, 270 insertions(+), 71 deletions(-)

-- 
2.26.2

Comments

Pedro Alves July 10, 2020, 8:08 p.m. | #1
On 7/5/20 11:58 PM, Kevin Buettner via Gdb-patches wrote:

> +void

> +core_target::build_file_mappings ()

> +{

> +  std::unordered_map<std::string, struct bfd *> bfd_map;

> +

> +  /* See linux_read_core_file_mappings() in linux-tdep.c for an example

> +     read_core_file_mappings method.  */

> +  gdbarch_read_core_file_mappings (m_core_gdbarch, core_bfd,

> +

> +    /* After determining the number of mappings, read_core_file_mappings

> +       will invoke this lambda which allocates target_section storage for

> +       the mappings.  */

> +    [&] (ULONGEST count)

> +      {

> +	m_core_file_mappings.sections = XNEWVEC (struct target_section, count);

> +	m_core_file_mappings.sections_end = m_core_file_mappings.sections;

> +      },

> +

> +    /* read_core_file_mappings will invoke this lambda for each mapping

> +       that it finds.  */

> +    [&] (int num, ULONGEST start, ULONGEST end, ULONGEST file_ofs,

> +         const char *filename, const void *other)

> +      {

> +	/* Architecture-specific read_core_mapping methods are expected to

> +	   weed out non-file-backed mappings.  */

> +	gdb_assert (filename != nullptr);

> +

> +	struct bfd *bfd = bfd_map[filename];

> +	if (bfd == nullptr) {


Brace on next line.

> +

> +	  /* Use exec_file_find() to do sysroot expansion.  It'll also strip

> +	     the potential sysroot target: prefix.  If there is no sysroot, an

> +	     equivalent (possibly more canonical) pathname will be provided.  */

> +	  gdb::unique_xmalloc_ptr<char> expanded_fname

> +	    = exec_file_find (filename, NULL);

> +	  if (expanded_fname == nullptr)

> +	    {

> +	      warning (_("Can't open file %s during file-backed mapping "

> +	                 "note processing"),

> +		       expanded_fname.get ());

> +	      return;

> +	    }

> +

> +	  bfd = bfd_map[filename] = bfd_openr (expanded_fname.get (), "binary");

> +

> +	  if (bfd == nullptr || !bfd_check_format (bfd, bfd_object))

> +	    {

> +	      /* If we get here, there's a good chance that it's due to

> +		 an internal error.  We issue a warning instead of an

> +		 internal error because of the possibility that the

> +		 file was removed in between checking for its

> +		 existence during the expansion in exec_file_find()

> +		 and the calls to bfd_openr() / bfd_check_format(). 

> +		 Output both the path from the core file note along

> +		 with its expansion to make debugging this problem

> +		 easier.  */

> +	      warning (_("Can't open file %s which was expanded to %s "

> +			 "during file-backed mapping note processing"),

> +		       filename, expanded_fname.get ());

> +	      return;


Don't we have to close the bfd if it was opened, but failed the
bfd_check_format check?

> +	    }

> +	    /* Ensure that the bfd will be closed when core_bfd is closed. 

> +	       This can be checked before/after a core file detach via

> +	       "maint info bfds".  */

> +	    gdb_bfd_record_inclusion (core_bfd, bfd);

> +	}

> +

> +	/* Make new BFD section.  */

> +	char secnamebuf[64];

> +	sprintf (secnamebuf, "S%04d", num);

> +	char *secname = (char *) bfd_alloc (bfd, strlen (secnamebuf) + 1);

> +	if (secname == nullptr)

> +	  error (_("Out of memory"));

> +	strcpy (secname, secnamebuf);

> +	asection *sec = bfd_make_section_anyway (bfd, secname);

> +	if (sec == nullptr)

> +	  error (_("Can't make section"));


SECNAME leaks if you throw here.  Use a unique pointer?
Also, should these be malloc_failure instead?

> +	sec->filepos = file_ofs;

> +	bfd_set_section_flags (sec, SEC_READONLY | SEC_HAS_CONTENTS);

> +	bfd_set_section_size (sec, end - start);

> +	bfd_set_section_vma (sec, start);

> +	bfd_set_section_lma (sec, start);

> +	bfd_set_section_alignment (sec, 2);

> +

> +	/* Set target_section fields.  */

> +	struct target_section *ts = m_core_file_mappings.sections_end++;

> +	ts->addr = start;

> +	ts->endaddr = end;

> +	ts->owner = nullptr;

> +	ts->the_bfd_section = sec;

> +      });

>  }

>  


Otherwise LGTM.
Simon Marchi via Gdb-patches July 14, 2020, 7:53 a.m. | #2
On Fri, 10 Jul 2020 21:08:27 +0100
Pedro Alves <pedro@palves.net> wrote:

> > +	/* Make new BFD section.  */

> > +	char secnamebuf[64];

> > +	sprintf (secnamebuf, "S%04d", num);

> > +	char *secname = (char *) bfd_alloc (bfd, strlen (secnamebuf) + 1);

> > +	if (secname == nullptr)

> > +	  error (_("Out of memory"));

> > +	strcpy (secname, secnamebuf);

> > +	asection *sec = bfd_make_section_anyway (bfd, secname);

> > +	if (sec == nullptr)

> > +	  error (_("Can't make section"));  

> 

> SECNAME leaks if you throw here.  Use a unique pointer?


It turns out that this code will leak even if the error pathway is not
taken.  After studying bfd/section.c, it appears that nothing ever
frees the section name.  bfd_make_section_anyway{,with_flags} is
usually (though not always) called with a constant string like
".dynstr", ".plt", ".buildid", etc.  In those cases where it is passed
a dynamically allocated string, I see no provision for freeing the
section name.  I strongly suspect that there are leaks where this
occurs.

It turns out that bfd_make_section_anyway() will create a new
section even if passed a name of an already existing section.  I'm
experimenting with just passing in "load" as the name for each
needed section.  So far, it seems to work.

Kevin
Simon Marchi via Gdb-patches July 21, 2020, 2:09 a.m. | #3
On Fri, 10 Jul 2020 21:08:27 +0100
Pedro Alves <pedro@palves.net> wrote:

> > +	/* Make new BFD section.  */

> > +	char secnamebuf[64];

> > +	sprintf (secnamebuf, "S%04d", num);

> > +	char *secname = (char *) bfd_alloc (bfd, strlen (secnamebuf) + 1);

> > +	if (secname == nullptr)

> > +	  error (_("Out of memory"));

> > +	strcpy (secname, secnamebuf);

> > +	asection *sec = bfd_make_section_anyway (bfd, secname);

> > +	if (sec == nullptr)

> > +	  error (_("Can't make section"));  

> 

[...]
> Also, should these be malloc_failure instead?


I think that malloc_failure would have been appropriate for a failed
bfd_alloc() call.  I've removed this call now, so it's no longer
relevant.

I think that calling error() is still appropriate for a failed call to
bfd_make_section_anyway().  I did take a look to see how other code
within GDB handled a failed call to bfd_make_section_anyway.* :

- record_full_base_target::save_record in record.c calls error().
- write_gcore_file_1 and gcore_create_callback in gcore.c both call
  error().
- dump_bfd_file in cli/cli-dump.c does NOT handle failure from
  bfd_make_section_anyway.  I.e. it'll segfault from a NULL pointer
  access!

Kevin
Tom Tromey July 21, 2020, 6:21 p.m. | #4
>>>>> "Kevin" == Kevin Buettner via Gdb-patches <gdb-patches@sourceware.org> writes:


>> > +	char *secname = (char *) bfd_alloc (bfd, strlen (secnamebuf) + 1);


>> SECNAME leaks if you throw here.  Use a unique pointer?


Kevin> It turns out that this code will leak even if the error pathway is not
Kevin> taken.  After studying bfd/section.c, it appears that nothing ever
Kevin> frees the section name.  bfd_make_section_anyway{,with_flags} is
Kevin> usually (though not always) called with a constant string like
Kevin> ".dynstr", ".plt", ".buildid", etc.  In those cases where it is passed
Kevin> a dynamically allocated string, I see no provision for freeing the
Kevin> section name.  I strongly suspect that there are leaks where this
Kevin> occurs.

bfd_alloc allocates on the BFD's objalloc (which is like an obstack, but
"optimized").  So, it's a leak in the "lingerer" sense (memory allocated
that isn't useful), but not in the ordinary sense, because it will be
freed when the BFD is closed.

Presumably if we had ASAN and/or valgrind annotations for obstack and
objalloc, this would show up as a real leak.  But we don't, so it won't.

I didn't look through BFD to see how section names might be allocated,
but if bfd_alloc is used, then it's fine.

Tom

Patch

diff --git a/gdb/corelow.c b/gdb/corelow.c
index 8304d60129..ac52a1019e 100644
--- a/gdb/corelow.c
+++ b/gdb/corelow.c
@@ -37,6 +37,7 @@ 
 #include "exec.h"
 #include "readline/tilde.h"
 #include "solib.h"
+#include "solist.h"
 #include "filenames.h"
 #include "progspace.h"
 #include "objfiles.h"
@@ -45,6 +46,7 @@ 
 #include "gdbsupport/filestuff.h"
 #include "build-id.h"
 #include "gdbsupport/pathstuff.h"
+#include <unordered_map>
 
 #ifndef O_LARGEFILE
 #define O_LARGEFILE 0
@@ -121,6 +123,13 @@  class core_target final : public process_stratum_target
      targets.  */
   target_section_table m_core_section_table {};
 
+  /* File-backed address space mappings: some core files include
+     information about memory mapped files.  */
+  target_section_table m_core_file_mappings {};
+
+  /* Build m_core_file_mappings.  Called from the constructor.  */
+  void build_file_mappings ();
+
   /* FIXME: kettenis/20031023: Eventually this field should
      disappear.  */
   struct gdbarch *m_core_gdbarch = NULL;
@@ -141,11 +150,121 @@  core_target::core_target ()
 			   &m_core_section_table.sections_end))
     error (_("\"%s\": Can't find sections: %s"),
 	   bfd_get_filename (core_bfd), bfd_errmsg (bfd_get_error ()));
+
+  build_file_mappings ();
 }
 
 core_target::~core_target ()
 {
   xfree (m_core_section_table.sections);
+  xfree (m_core_file_mappings.sections);
+}
+
+/* Construct the target_section_table for file-backed mappings if
+   they exist.
+
+   For each unique path in the note, we'll open a BFD with a bfd
+   target of "binary".  This is an unstructured bfd target upon which
+   we'll impose a structure from the mappings in the architecture-specific
+   mappings note.  A BFD section is allocated and initialized for each
+   file-backed mapping.
+
+   We take care to not share already open bfds with other parts of
+   GDB; in particular, we don't want to add new sections to existing
+   BFDs.  We do, however, ensure that the BFDs that we allocate here
+   will go away (be deallocated) when the core target is detached.  */
+
+void
+core_target::build_file_mappings ()
+{
+  std::unordered_map<std::string, struct bfd *> bfd_map;
+
+  /* See linux_read_core_file_mappings() in linux-tdep.c for an example
+     read_core_file_mappings method.  */
+  gdbarch_read_core_file_mappings (m_core_gdbarch, core_bfd,
+
+    /* After determining the number of mappings, read_core_file_mappings
+       will invoke this lambda which allocates target_section storage for
+       the mappings.  */
+    [&] (ULONGEST count)
+      {
+	m_core_file_mappings.sections = XNEWVEC (struct target_section, count);
+	m_core_file_mappings.sections_end = m_core_file_mappings.sections;
+      },
+
+    /* read_core_file_mappings will invoke this lambda for each mapping
+       that it finds.  */
+    [&] (int num, ULONGEST start, ULONGEST end, ULONGEST file_ofs,
+         const char *filename, const void *other)
+      {
+	/* Architecture-specific read_core_mapping methods are expected to
+	   weed out non-file-backed mappings.  */
+	gdb_assert (filename != nullptr);
+
+	struct bfd *bfd = bfd_map[filename];
+	if (bfd == nullptr) {
+
+	  /* Use exec_file_find() to do sysroot expansion.  It'll also strip
+	     the potential sysroot target: prefix.  If there is no sysroot, an
+	     equivalent (possibly more canonical) pathname will be provided.  */
+	  gdb::unique_xmalloc_ptr<char> expanded_fname
+	    = exec_file_find (filename, NULL);
+	  if (expanded_fname == nullptr)
+	    {
+	      warning (_("Can't open file %s during file-backed mapping "
+	                 "note processing"),
+		       expanded_fname.get ());
+	      return;
+	    }
+
+	  bfd = bfd_map[filename] = bfd_openr (expanded_fname.get (), "binary");
+
+	  if (bfd == nullptr || !bfd_check_format (bfd, bfd_object))
+	    {
+	      /* If we get here, there's a good chance that it's due to
+		 an internal error.  We issue a warning instead of an
+		 internal error because of the possibility that the
+		 file was removed in between checking for its
+		 existence during the expansion in exec_file_find()
+		 and the calls to bfd_openr() / bfd_check_format(). 
+		 Output both the path from the core file note along
+		 with its expansion to make debugging this problem
+		 easier.  */
+	      warning (_("Can't open file %s which was expanded to %s "
+			 "during file-backed mapping note processing"),
+		       filename, expanded_fname.get ());
+	      return;
+	    }
+	    /* Ensure that the bfd will be closed when core_bfd is closed. 
+	       This can be checked before/after a core file detach via
+	       "maint info bfds".  */
+	    gdb_bfd_record_inclusion (core_bfd, bfd);
+	}
+
+	/* Make new BFD section.  */
+	char secnamebuf[64];
+	sprintf (secnamebuf, "S%04d", num);
+	char *secname = (char *) bfd_alloc (bfd, strlen (secnamebuf) + 1);
+	if (secname == nullptr)
+	  error (_("Out of memory"));
+	strcpy (secname, secnamebuf);
+	asection *sec = bfd_make_section_anyway (bfd, secname);
+	if (sec == nullptr)
+	  error (_("Can't make section"));
+	sec->filepos = file_ofs;
+	bfd_set_section_flags (sec, SEC_READONLY | SEC_HAS_CONTENTS);
+	bfd_set_section_size (sec, end - start);
+	bfd_set_section_vma (sec, start);
+	bfd_set_section_lma (sec, start);
+	bfd_set_section_alignment (sec, 2);
+
+	/* Set target_section fields.  */
+	struct target_section *ts = m_core_file_mappings.sections_end++;
+	ts->addr = start;
+	ts->endaddr = end;
+	ts->owner = nullptr;
+	ts->the_bfd_section = sec;
+      });
 }
 
 static void add_to_thread_list (bfd *, asection *, void *);
@@ -630,10 +749,21 @@  core_target::xfer_partial (enum target_object object, const char *annex,
 	if (xfer_status == TARGET_XFER_OK)
 	  return TARGET_XFER_OK;
 
-	/* Now check the stratum beneath us; this should be file_stratum.  */
-	xfer_status = this->beneath ()->xfer_partial (object, annex, readbuf,
-						      writebuf, offset, len,
-						      xfered_len);
+	/* Check file backed mappings.  If they're available, use
+	   core file provided mappings (e.g. from .note.linuxcore.file
+	   or the like) as this should provide a more accurate
+	   result.  If not, check the stratum beneath us, which should
+	   be the file stratum.  */
+	if (m_core_file_mappings.sections != nullptr)
+	  xfer_status = section_table_xfer_memory_partial
+			  (readbuf, writebuf,
+			   offset, len, xfered_len,
+			   m_core_file_mappings.sections,
+			   m_core_file_mappings.sections_end);
+	else
+	  xfer_status = this->beneath ()->xfer_partial (object, annex, readbuf,
+							writebuf, offset, len,
+							xfered_len);
 	if (xfer_status == TARGET_XFER_OK)
 	  return TARGET_XFER_OK;
 
diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
index fd4337f100..fa59b09eec 100644
--- a/gdb/linux-tdep.c
+++ b/gdb/linux-tdep.c
@@ -1018,106 +1018,174 @@  linux_info_proc (struct gdbarch *gdbarch, const char *args,
     }
 }
 
-/* Implement "info proc mappings" for a corefile.  */
+/* Implementation of `gdbarch_read_core_file_mappings', as defined in
+   gdbarch.h.
+   
+   This function reads the NT_FILE note (which BFD turns into the
+   section ".note.linuxcore.file").  The format of this note / section
+   is described as follows in the Linux kernel sources in
+   fs/binfmt_elf.c:
+   
+      long count     -- how many files are mapped
+      long page_size -- units for file_ofs
+      array of [COUNT] elements of
+	long start
+	long end
+	long file_ofs
+      followed by COUNT filenames in ASCII: "FILE1" NUL "FILE2" NUL...
+      
+   CBFD is the BFD of the core file.
+
+   PRE_LOOP_CB is the callback function to invoke prior to starting
+   the loop which process individual entries.  This callback will
+   only be executed after the note has been examined in enough
+   detail to verify that it's not malformed in some way.
+   
+   LOOP_CB is the callback function that will be executed once
+   for each mapping.  */
 
 static void
-linux_core_info_proc_mappings (struct gdbarch *gdbarch, const char *args)
+linux_read_core_file_mappings (struct gdbarch *gdbarch,
+			       struct bfd *cbfd,
+			       gdb::function_view<void (ULONGEST count)>
+			         pre_loop_cb,
+			       gdb::function_view<void (int num,
+			                                ULONGEST start,
+							ULONGEST end,
+							ULONGEST file_ofs,
+							const char *filename,
+							const void *other)>
+				 loop_cb)
 {
-  asection *section;
-  ULONGEST count, page_size;
-  unsigned char *descdata, *filenames, *descend;
-  size_t note_size;
-  unsigned int addr_size_bits, addr_size;
-  struct gdbarch *core_gdbarch = gdbarch_from_bfd (core_bfd);
-  /* We assume this for reading 64-bit core files.  */
+  /* Ensure that ULONGEST is big enough for reading 64-bit core files.  */
   gdb_static_assert (sizeof (ULONGEST) >= 8);
 
-  section = bfd_get_section_by_name (core_bfd, ".note.linuxcore.file");
-  if (section == NULL)
-    {
-      warning (_("unable to find mappings in core file"));
-      return;
-    }
+  /* It's not required that the NT_FILE note exists, so return silently
+     if it's not found.  Beyond this point though, we'll complain
+     if problems are found.  */
+  asection *section = bfd_get_section_by_name (cbfd, ".note.linuxcore.file");
+  if (section == nullptr)
+    return;
 
-  addr_size_bits = gdbarch_addr_bit (core_gdbarch);
-  addr_size = addr_size_bits / 8;
-  note_size = bfd_section_size (section);
+  unsigned int addr_size_bits = gdbarch_addr_bit (gdbarch);
+  unsigned int addr_size = addr_size_bits / 8;
+  size_t note_size = bfd_section_size (section);
 
   if (note_size < 2 * addr_size)
-    error (_("malformed core note - too short for header"));
+    {
+      warning (_("malformed core note - too short for header"));
+      return;
+    }
 
-  gdb::def_vector<unsigned char> contents (note_size);
+  gdb::def_vector<gdb_byte> contents (note_size);
   if (!bfd_get_section_contents (core_bfd, section, contents.data (),
 				 0, note_size))
-    error (_("could not get core note contents"));
+    {
+      warning (_("could not get core note contents"));
+      return;
+    }
 
-  descdata = contents.data ();
-  descend = descdata + note_size;
+  gdb_byte *descdata = contents.data ();
+  char *descend = (char *) descdata + note_size;
 
   if (descdata[note_size - 1] != '\0')
-    error (_("malformed note - does not end with \\0"));
+    {
+      warning (_("malformed note - does not end with \\0"));
+      return;
+    }
 
-  count = bfd_get (addr_size_bits, core_bfd, descdata);
+  ULONGEST count = bfd_get (addr_size_bits, core_bfd, descdata);
   descdata += addr_size;
 
-  page_size = bfd_get (addr_size_bits, core_bfd, descdata);
+  ULONGEST page_size = bfd_get (addr_size_bits, core_bfd, descdata);
   descdata += addr_size;
 
   if (note_size < 2 * addr_size + count * 3 * addr_size)
-    error (_("malformed note - too short for supplied file count"));
-
-  printf_filtered (_("Mapped address spaces:\n\n"));
-  if (gdbarch_addr_bit (gdbarch) == 32)
-    {
-      printf_filtered ("\t%10s %10s %10s %10s %s\n",
-		       "Start Addr",
-		       "  End Addr",
-		       "      Size", "    Offset", "objfile");
-    }
-  else
     {
-      printf_filtered ("  %18s %18s %10s %10s %s\n",
-		       "Start Addr",
-		       "  End Addr",
-		       "      Size", "    Offset", "objfile");
+      warning (_("malformed note - too short for supplied file count"));
+      return;
     }
 
-  filenames = descdata + count * 3 * addr_size;
-  while (--count > 0)
+  char *filenames = (char *) descdata + count * 3 * addr_size;
+
+  /* Make sure that the correct number of filenames exist.  Complain
+     if there aren't enough or are too many.  */
+  char *f = filenames;
+  for (int i = 0; i < count; i++)
     {
-      ULONGEST start, end, file_ofs;
+      if (f >= descend)
+        {
+	  warning (_("malformed note - filename area is too small"));
+	  return;
+	}
+      f += strnlen (f, descend - f) + 1;
+    }
+  /* Complain, but don't return early if the filename area is too big.  */
+  if (f != descend)
+    warning (_("malformed note - filename area is too big"));
 
-      if (filenames == descend)
-	error (_("malformed note - filenames end too early"));
+  pre_loop_cb (count);
 
-      start = bfd_get (addr_size_bits, core_bfd, descdata);
+  for (int i = 0; i < count; i++)
+    {
+      ULONGEST start = bfd_get (addr_size_bits, core_bfd, descdata);
       descdata += addr_size;
-      end = bfd_get (addr_size_bits, core_bfd, descdata);
+      ULONGEST end = bfd_get (addr_size_bits, core_bfd, descdata);
       descdata += addr_size;
-      file_ofs = bfd_get (addr_size_bits, core_bfd, descdata);
+      ULONGEST file_ofs
+        = bfd_get (addr_size_bits, core_bfd, descdata) * page_size;
       descdata += addr_size;
+      char * filename = filenames;
+      filenames += strlen ((char *) filenames) + 1;
 
-      file_ofs *= page_size;
-
-      if (gdbarch_addr_bit (gdbarch) == 32)
-	printf_filtered ("\t%10s %10s %10s %10s %s\n",
-			 paddress (gdbarch, start),
-			 paddress (gdbarch, end),
-			 hex_string (end - start),
-			 hex_string (file_ofs),
-			 filenames);
-      else
-	printf_filtered ("  %18s %18s %10s %10s %s\n",
-			 paddress (gdbarch, start),
-			 paddress (gdbarch, end),
-			 hex_string (end - start),
-			 hex_string (file_ofs),
-			 filenames);
-
-      filenames += 1 + strlen ((char *) filenames);
+      loop_cb (i, start, end, file_ofs, filename, nullptr);
     }
 }
 
+/* Implement "info proc mappings" for a corefile.  */
+
+static void
+linux_core_info_proc_mappings (struct gdbarch *gdbarch, const char *args)
+{
+  linux_read_core_file_mappings (gdbarch, core_bfd,
+    [=] (ULONGEST count)
+      {
+	printf_filtered (_("Mapped address spaces:\n\n"));
+	if (gdbarch_addr_bit (gdbarch) == 32)
+	  {
+	    printf_filtered ("\t%10s %10s %10s %10s %s\n",
+			     "Start Addr",
+			     "  End Addr",
+			     "      Size", "    Offset", "objfile");
+	  }
+	else
+	  {
+	    printf_filtered ("  %18s %18s %10s %10s %s\n",
+			     "Start Addr",
+			     "  End Addr",
+			     "      Size", "    Offset", "objfile");
+	  }
+      },
+    [=] (int num, ULONGEST start, ULONGEST end, ULONGEST file_ofs,
+         const char *filename, const void *other)
+      {
+	if (gdbarch_addr_bit (gdbarch) == 32)
+	  printf_filtered ("\t%10s %10s %10s %10s %s\n",
+			   paddress (gdbarch, start),
+			   paddress (gdbarch, end),
+			   hex_string (end - start),
+			   hex_string (file_ofs),
+			   filename);
+	else
+	  printf_filtered ("  %18s %18s %10s %10s %s\n",
+			   paddress (gdbarch, start),
+			   paddress (gdbarch, end),
+			   hex_string (end - start),
+			   hex_string (file_ofs),
+			   filename);
+      });
+}
+
 /* Implement "info proc" for a corefile.  */
 
 static void
@@ -2472,6 +2540,7 @@  linux_init_abi (struct gdbarch_info info, struct gdbarch *gdbarch)
   set_gdbarch_info_proc (gdbarch, linux_info_proc);
   set_gdbarch_core_info_proc (gdbarch, linux_core_info_proc);
   set_gdbarch_core_xfer_siginfo (gdbarch, linux_core_xfer_siginfo);
+  set_gdbarch_read_core_file_mappings (gdbarch, linux_read_core_file_mappings);
   set_gdbarch_find_memory_regions (gdbarch, linux_find_memory_regions);
   set_gdbarch_make_corefile_notes (gdbarch, linux_make_corefile_notes);
   set_gdbarch_has_shared_address_space (gdbarch,