[PATCH 0/4] bfd: Add support for Cygwin x86_64 core dumps

classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH 0/4] bfd: Add support for Cygwin x86_64 core dumps

Jon TURNEY
Fixes and additions support x86_64 in reading the NT_WIN32PSTATUS ELF notes
in a Cygwin "core dump".

As far as I know, the only way to generate these "core dumps" is to use
Cygwin's 'dumper' tool, which requires some fixes on x86_64 [1].

[1] https://cygwin.com/pipermail/cygwin-patches/2020q3/010313.html

Jon Turney (4):
  Read tid from correct offset in win32pstatus NOTE_INFO_THREAD
  Don't apply size constraint to all win32pstatus ELF notes.
  Don't hardcode CONTEXT size for a NOTE_INFO_THREAD win32pstatus note
  Add handling for 64-bit module addresses in Cygwin core dumps

 bfd/ChangeLog | 21 +++++++++++++++++++++
 bfd/elf.c     | 29 ++++++++++++++++++-----------
 2 files changed, 39 insertions(+), 11 deletions(-)

--
2.27.0

Reply | Threaded
Open this post in threaded view
|

[PATCH 1/4] Read tid from correct offset in win32pstatus NOTE_INFO_THREAD

Jon TURNEY
Fix the offset used to read the tid from a win32pstatus ELF note.

This probably meant that registers were only being correctly recovered
from the core dump for the current thread.

It looks like this has beeen incorrect since 4a6636fb.

Also fix offsets used in NOTE_INFO_PROCESS (which is not actually
generated by the Cygwin dumper tool).

Also improve comment.

bfd/ChangeLog:

2020-07-01  Jon Turney  <[hidden email]>

        * elf.c (elfcore_grok_win32pstatus): Fix the offset used to read
        the tid from a win32pstatus NOTE_INFO_THREAD ELF note.  Fix
        offsets used to read NOTE_INFO_PROCESS.
---
 bfd/ChangeLog | 6 ++++++
 bfd/elf.c     | 9 +++++----
 2 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/bfd/elf.c b/bfd/elf.c
index 9ca42e10d8e..0990be31f54 100644
--- a/bfd/elf.c
+++ b/bfd/elf.c
@@ -10151,15 +10151,16 @@ elfcore_grok_win32pstatus (bfd *abfd, Elf_Internal_Note *note)
     case 1 /* NOTE_INFO_PROCESS */:
       /* FIXME: need to add ->core->command.  */
       /* process_info.pid */
-      elf_tdata (abfd)->core->pid = bfd_get_32 (abfd, note->descdata + 8);
+      elf_tdata (abfd)->core->pid = bfd_get_32 (abfd, note->descdata + 4);
       /* process_info.signal */
-      elf_tdata (abfd)->core->signal = bfd_get_32 (abfd, note->descdata + 12);
+      elf_tdata (abfd)->core->signal = bfd_get_32 (abfd, note->descdata + 8);
       break;
 
     case 2 /* NOTE_INFO_THREAD */:
-      /* Make a ".reg/999" section.  */
+      /* Make a ".reg/<tid>" section containing the Win32 API thread CONTEXT
+         structure. */
       /* thread_info.tid */
-      sprintf (buf, ".reg/%ld", (long) bfd_get_32 (abfd, note->descdata + 8));
+      sprintf (buf, ".reg/%ld", (long) bfd_get_32 (abfd, note->descdata + 4));
 
       len = strlen (buf) + 1;
       name = (char *) bfd_alloc (abfd, len);
--
2.27.0

Reply | Threaded
Open this post in threaded view
|

[PATCH 2/4] Don't apply size constraint to all win32pstatus ELF notes.

Jon TURNEY
In reply to this post by Jon TURNEY
Don't reject any win32pstatus notes smaller than minimum size for a
NOTE_INFO_THREAD.

This only happens to work because the Cygwin dumper tool currently
writes all these notes as the largest size of the union, (which wastes
lots of space in the core dump).

bfd/ChangeLog:

2020-07-01  Jon Turney  <[hidden email]>

        * elf.c (elfcore_grok_win32pstatus): Don't apply size constraint
        for NOTE_INFO_THREAD to all win32pstatus ELF notes.
---
 bfd/ChangeLog | 5 +++++
 bfd/elf.c     | 3 ---
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/bfd/elf.c b/bfd/elf.c
index 0990be31f54..373e285ca4f 100644
--- a/bfd/elf.c
+++ b/bfd/elf.c
@@ -10138,9 +10138,6 @@ elfcore_grok_win32pstatus (bfd *abfd, Elf_Internal_Note *note)
   int is_active_thread;
   bfd_vma base_addr;
 
-  if (note->descsz < 728)
-    return TRUE;
-
   if (! CONST_STRNEQ (note->namedata, "win32"))
     return TRUE;
 
--
2.27.0

Reply | Threaded
Open this post in threaded view
|

[PATCH 3/4] Don't hardcode CONTEXT size for a NOTE_INFO_THREAD win32pstatus note

Jon TURNEY
In reply to this post by Jon TURNEY
Don't hardcode the size of the Win32 API thread CONTEXT type read from a
NOTE_INFO_THREAD win32pstatus note (since it's different on different
architectures).

bfd/ChangeLog:

2020-07-01  Jon Turney  <[hidden email]>

        * elf.c (elfcore_grok_win32pstatus): Don't hardcode the size of
        the Win32 API thread CONTEXT type read from a NOTE_INFO_THREAD
        win32pstatus note.
---
 bfd/ChangeLog | 6 ++++++
 bfd/elf.c     | 2 +-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/bfd/elf.c b/bfd/elf.c
index 373e285ca4f..a3b8abf2410 100644
--- a/bfd/elf.c
+++ b/bfd/elf.c
@@ -10171,7 +10171,7 @@ elfcore_grok_win32pstatus (bfd *abfd, Elf_Internal_Note *note)
  return FALSE;
 
       /* sizeof (thread_info.thread_context) */
-      sect->size = 716;
+      sect->size = note->descsz - 12;
       /* offsetof (thread_info.thread_context) */
       sect->filepos = note->descpos + 12;
       sect->alignment_power = 2;
--
2.27.0

Reply | Threaded
Open this post in threaded view
|

[PATCH 4/4] Add handling for 64-bit module addresses in Cygwin core dumps

Jon TURNEY
In reply to this post by Jon TURNEY
bfd/ChangeLog:

2020-07-01  Jon Turney  <[hidden email]>

        * elf.c (elfcore_grok_win32pstatus): Handle NOTE_INFO_MODULE64.
---
 bfd/ChangeLog |  4 ++++
 bfd/elf.c     | 15 ++++++++++++---
 2 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/bfd/elf.c b/bfd/elf.c
index a3b8abf2410..513ccc79c76 100644
--- a/bfd/elf.c
+++ b/bfd/elf.c
@@ -10185,10 +10185,19 @@ elfcore_grok_win32pstatus (bfd *abfd, Elf_Internal_Note *note)
       break;
 
     case 3 /* NOTE_INFO_MODULE */:
-      /* Make a ".module/xxxxxxxx" section.  */
+    case 4 /* NOTE_INFO_MODULE64 */:
+      /* Make a ".module/<base_address>" section.  */
       /* module_info.base_address */
-      base_addr = bfd_get_32 (abfd, note->descdata + 4);
-      sprintf (buf, ".module/%08lx", (unsigned long) base_addr);
+      if (type == 3)
+        {
+          base_addr = bfd_get_32 (abfd, note->descdata + 4);
+          sprintf (buf, ".module/%08lx", (unsigned long) base_addr);
+        }
+      else
+        {
+          base_addr = bfd_get_64 (abfd, note->descdata + 4);
+          sprintf (buf, ".module/%016lx", (unsigned long) base_addr);
+        }
 
       len = strlen (buf) + 1;
       name = (char *) bfd_alloc (abfd, len);
--
2.27.0

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 0/4] bfd: Add support for Cygwin x86_64 core dumps

Sourceware - binutils list mailing list
In reply to this post by Jon TURNEY
Hi Jon,

> Fixes and additions support x86_64 in reading the NT_WIN32PSTATUS ELF notes
> in a Cygwin "core dump".

The patch series looks fine to me apart from one thing:

From patch 2/4:

  -  if (note->descsz < 728)
  -    return TRUE;

Without this check it will be possible for a corrupt core file
to trigger invalid reads beyond the end of the note section.
(Binary fuzzers love triggering this kind of bug).  So I think
that everywhere you read data from a note you should make sure
that there actually is data present first.

Cheers
  Nick

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 0/4] bfd: Add support for Cygwin x86_64 core dumps

Jon TURNEY
On 09/07/2020 14:38, Nick Clifton via Binutils wrote:

> Hi Jon,
>
>> Fixes and additions support x86_64 in reading the NT_WIN32PSTATUS ELF notes
>> in a Cygwin "core dump".
>
> The patch series looks fine to me apart from one thing:
>
>  From patch 2/4:
>
>    -  if (note->descsz < 728)
>    -    return TRUE;
>
> Without this check it will be possible for a corrupt core file
> to trigger invalid reads beyond the end of the note section.
> (Binary fuzzers love triggering this kind of bug).  So I think
> that everywhere you read data from a note you should make sure
> that there actually is data present first.

Yes, that should be done.  I posted a revised patch set with that added.