[PATCH 0/5] bfd: Add support for Cygwin x86_64 core dumps (v2)

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

[PATCH 0/5] bfd: Add support for Cygwin x86_64 core dumps (v2)

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

Changes since v1 to address review comments:

- Check note size before reading data everywhere
- Define some constants for note types

Jon Turney (5):
  Read tid from correct offset in win32pstatus NOTE_INFO_THREAD
  Define constants for win32pstatus ELF notes
  Don't hardcode CONTEXT size for a NOTE_INFO_THREAD win32pstatus note
  Refine size constraints applied to win32pstatus ELF notes
  Add handling for 64-bit module addresses in Cygwin core dumps

 bfd/ChangeLog | 27 ++++++++++++++++++++++
 bfd/elf.c     | 64 ++++++++++++++++++++++++++++++++++++++++-----------
 2 files changed, 77 insertions(+), 14 deletions(-)

--
2.27.0

Reply | Threaded
Open this post in threaded view
|

[PATCH 1/5] 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/5] Define constants for win32pstatus ELF notes

Jon TURNEY
In reply to this post by Jon TURNEY
Define constants for win32pstatus ELF notes, as they were prior to
4a6636fb, and say what specifies them.

bfd/ChangeLog:

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

        * elf.c (NOTE_INFO{_PROCESS,_THREAD,_MODULE}): Define.
        (elfcore_grok_win32pstatus): Use.
---
 bfd/ChangeLog |  5 +++++
 bfd/elf.c     | 14 +++++++++-----
 2 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/bfd/elf.c b/bfd/elf.c
index 0990be31f54..d4900dd3709 100644
--- a/bfd/elf.c
+++ b/bfd/elf.c
@@ -10127,6 +10127,12 @@ elfcore_grok_lwpstatus (bfd *abfd, Elf_Internal_Note *note)
 }
 #endif /* defined (HAVE_LWPSTATUS_T) */
 
+/* These constants, and the structure offsets used below, are defined by
+   Cygwin's core_dump.h */
+#define NOTE_INFO_PROCESS  1
+#define NOTE_INFO_THREAD   2
+#define NOTE_INFO_MODULE   3
+
 static bfd_boolean
 elfcore_grok_win32pstatus (bfd *abfd, Elf_Internal_Note *note)
 {
@@ -10148,15 +10154,13 @@ elfcore_grok_win32pstatus (bfd *abfd, Elf_Internal_Note *note)
 
   switch (type)
     {
-    case 1 /* NOTE_INFO_PROCESS */:
+    case NOTE_INFO_PROCESS:
       /* FIXME: need to add ->core->command.  */
-      /* process_info.pid */
       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 + 8);
       break;
 
-    case 2 /* NOTE_INFO_THREAD */:
+    case NOTE_INFO_THREAD:
       /* Make a ".reg/<tid>" section containing the Win32 API thread CONTEXT
          structure. */
       /* thread_info.tid */
@@ -10187,7 +10191,7 @@ elfcore_grok_win32pstatus (bfd *abfd, Elf_Internal_Note *note)
   return FALSE;
       break;
 
-    case 3 /* NOTE_INFO_MODULE */:
+    case NOTE_INFO_MODULE:
       /* Make a ".module/xxxxxxxx" section.  */
       /* module_info.base_address */
       base_addr = bfd_get_32 (abfd, note->descdata + 4);
--
2.27.0

Reply | Threaded
Open this post in threaded view
|

[PATCH 3/5] 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 d4900dd3709..61a7f0930e2 100644
--- a/bfd/elf.c
+++ b/bfd/elf.c
@@ -10178,7 +10178,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/5] Refine size constraints applied to 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).

Instead, apply the appropriate size constraint for each win32pstatus
note type.

bfd/ChangeLog:

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

        * elf.c (elfcore_grok_win32pstatus): Don't apply size constraint
        for NOTE_INFO_THREAD to all win32pstatus ELF notes, instead apply
        appropriate size constraint for each win32pstatus note type.
---
 bfd/ChangeLog |  6 ++++++
 bfd/elf.c     | 17 ++++++++++++++++-
 2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/bfd/elf.c b/bfd/elf.c
index 61a7f0930e2..b05d0d6c2db 100644
--- a/bfd/elf.c
+++ b/bfd/elf.c
@@ -10139,12 +10139,13 @@ elfcore_grok_win32pstatus (bfd *abfd, Elf_Internal_Note *note)
   char buf[30];
   char *name;
   size_t len;
+  size_t name_size;
   asection *sect;
   int type;
   int is_active_thread;
   bfd_vma base_addr;
 
-  if (note->descsz < 728)
+  if (note->descsz < 4)
     return TRUE;
 
   if (! CONST_STRNEQ (note->namedata, "win32"))
@@ -10155,12 +10156,18 @@ elfcore_grok_win32pstatus (bfd *abfd, Elf_Internal_Note *note)
   switch (type)
     {
     case NOTE_INFO_PROCESS:
+      if (note->descsz < 12)
+        return TRUE;
+
       /* FIXME: need to add ->core->command.  */
       elf_tdata (abfd)->core->pid = bfd_get_32 (abfd, note->descdata + 4);
       elf_tdata (abfd)->core->signal = bfd_get_32 (abfd, note->descdata + 8);
       break;
 
     case NOTE_INFO_THREAD:
+      if (note->descsz < 12)
+        return TRUE;
+
       /* Make a ".reg/<tid>" section containing the Win32 API thread CONTEXT
          structure. */
       /* thread_info.tid */
@@ -10192,6 +10199,9 @@ elfcore_grok_win32pstatus (bfd *abfd, Elf_Internal_Note *note)
       break;
 
     case NOTE_INFO_MODULE:
+      if (note->descsz < 12)
+        return TRUE;
+
       /* Make a ".module/xxxxxxxx" section.  */
       /* module_info.base_address */
       base_addr = bfd_get_32 (abfd, note->descdata + 4);
@@ -10209,6 +10219,11 @@ elfcore_grok_win32pstatus (bfd *abfd, Elf_Internal_Note *note)
       if (sect == NULL)
  return FALSE;
 
+      /* module_info.module_name_size */
+      name_size = bfd_get_32 (abfd, note->descdata + 8);
+      if (note->descsz < 12 + name_size)
+        return TRUE;
+
       sect->size = note->descsz;
       sect->filepos = note->descpos;
       sect->alignment_power = 2;
--
2.27.0

Reply | Threaded
Open this post in threaded view
|

[PATCH 5/5] 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     | 32 ++++++++++++++++++++++++--------
 2 files changed, 28 insertions(+), 8 deletions(-)

diff --git a/bfd/elf.c b/bfd/elf.c
index b05d0d6c2db..21de5df8ffe 100644
--- a/bfd/elf.c
+++ b/bfd/elf.c
@@ -10132,6 +10132,7 @@ elfcore_grok_lwpstatus (bfd *abfd, Elf_Internal_Note *note)
 #define NOTE_INFO_PROCESS  1
 #define NOTE_INFO_THREAD   2
 #define NOTE_INFO_MODULE   3
+#define NOTE_INFO_MODULE64 4
 
 static bfd_boolean
 elfcore_grok_win32pstatus (bfd *abfd, Elf_Internal_Note *note)
@@ -10199,13 +10200,30 @@ elfcore_grok_win32pstatus (bfd *abfd, Elf_Internal_Note *note)
       break;
 
     case NOTE_INFO_MODULE:
-      if (note->descsz < 12)
-        return TRUE;
-
+    case NOTE_INFO_MODULE64:
       /* Make a ".module/xxxxxxxx" section.  */
-      /* module_info.base_address */
-      base_addr = bfd_get_32 (abfd, note->descdata + 4);
-      sprintf (buf, ".module/%08lx", (unsigned long) base_addr);
+      if (type == NOTE_INFO_MODULE)
+        {
+          if (note->descsz < 12)
+            return TRUE;
+
+          /* module_info.base_address */
+          base_addr = bfd_get_32 (abfd, note->descdata + 4);
+          sprintf (buf, ".module/%08lx", (unsigned long) base_addr);
+          /* module_info.module_name_size */
+          name_size = bfd_get_32 (abfd, note->descdata + 8);
+        }
+      else
+        {
+          if (note->descsz < 16)
+            return TRUE;
+
+          /* module_info.base_address */
+          base_addr = bfd_get_64 (abfd, note->descdata + 4);
+          sprintf (buf, ".module/%016lx", (unsigned long) base_addr);
+          /* module_info.module_name_size */
+          name_size = bfd_get_32 (abfd, note->descdata + 12);
+        }
 
       len = strlen (buf) + 1;
       name = (char *) bfd_alloc (abfd, len);
@@ -10219,8 +10237,6 @@ elfcore_grok_win32pstatus (bfd *abfd, Elf_Internal_Note *note)
       if (sect == NULL)
  return FALSE;
 
-      /* module_info.module_name_size */
-      name_size = bfd_get_32 (abfd, note->descdata + 8);
       if (note->descsz < 12 + name_size)
         return TRUE;
 
--
2.27.0

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 4/5] Refine size constraints applied to win32pstatus ELF notes

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

>      case NOTE_INFO_PROCESS:
> +      if (note->descsz < 12)
> +        return TRUE;
> +

Shouldn't this return, and the later ones, be "return FALSE" ?
After all you have a note type but insufficient space for valid note data.

Actually it looks to me like almost all of the returns in this function
should be "return FALSE" as there is something wrong with the note...

Cheers
  Nick
 

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 4/5] Refine size constraints applied to win32pstatus ELF notes

Jon TURNEY
On 13/07/2020 16:04, Nick Clifton wrote:
> Hi Jon,
>
>>       case NOTE_INFO_PROCESS:
>> +      if (note->descsz < 12)
>> +        return TRUE;
>> +
>
> Shouldn't this return, and the later ones, be "return FALSE" ?
> After all you have a note type but insufficient space for valid note data.

Yes, you are right.  Revised patches to follow.

> Actually it looks to me like almost all of the returns in this function
> should be "return FALSE" as there is something wrong with the note...

Perhaps the way this is written at the moment is a bit awkward as we
don't really distinguish in the return code between (i) the contents of
the note are malformed, and (ii) an internal error occurred while
processing the note. Do we really want to stop with an error in both cases?
Reply | Threaded
Open this post in threaded view
|

[PATCH 1/2] Refine size constraints applied to win32pstatus ELF notes

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).

Instead, apply the appropriate size constraint for each win32pstatus
note type.

bfd/ChangeLog:

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

        * elf.c (elfcore_grok_win32pstatus): Don't apply size constraint
        for NOTE_INFO_THREAD to all win32pstatus ELF notes, instead apply
        appropriate size constraint for each win32pstatus note type.
---
 bfd/ChangeLog |  6 ++++++
 bfd/elf.c     | 17 ++++++++++++++++-
 2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/bfd/elf.c b/bfd/elf.c
index 61a7f0930e2..1d62523b120 100644
--- a/bfd/elf.c
+++ b/bfd/elf.c
@@ -10139,12 +10139,13 @@ elfcore_grok_win32pstatus (bfd *abfd, Elf_Internal_Note *note)
   char buf[30];
   char *name;
   size_t len;
+  size_t name_size;
   asection *sect;
   int type;
   int is_active_thread;
   bfd_vma base_addr;
 
-  if (note->descsz < 728)
+  if (note->descsz < 4)
     return TRUE;
 
   if (! CONST_STRNEQ (note->namedata, "win32"))
@@ -10155,12 +10156,18 @@ elfcore_grok_win32pstatus (bfd *abfd, Elf_Internal_Note *note)
   switch (type)
     {
     case NOTE_INFO_PROCESS:
+      if (note->descsz < 12)
+        return FALSE;
+
       /* FIXME: need to add ->core->command.  */
       elf_tdata (abfd)->core->pid = bfd_get_32 (abfd, note->descdata + 4);
       elf_tdata (abfd)->core->signal = bfd_get_32 (abfd, note->descdata + 8);
       break;
 
     case NOTE_INFO_THREAD:
+      if (note->descsz < 12)
+        return FALSE;
+
       /* Make a ".reg/<tid>" section containing the Win32 API thread CONTEXT
          structure. */
       /* thread_info.tid */
@@ -10192,6 +10199,9 @@ elfcore_grok_win32pstatus (bfd *abfd, Elf_Internal_Note *note)
       break;
 
     case NOTE_INFO_MODULE:
+      if (note->descsz < 12)
+        return FALSE;
+
       /* Make a ".module/xxxxxxxx" section.  */
       /* module_info.base_address */
       base_addr = bfd_get_32 (abfd, note->descdata + 4);
@@ -10209,6 +10219,11 @@ elfcore_grok_win32pstatus (bfd *abfd, Elf_Internal_Note *note)
       if (sect == NULL)
  return FALSE;
 
+      /* module_info.module_name_size */
+      name_size = bfd_get_32 (abfd, note->descdata + 8);
+      if (note->descsz < 12 + name_size)
+        return FALSE;
+
       sect->size = note->descsz;
       sect->filepos = note->descpos;
       sect->alignment_power = 2;
--
2.27.0

Reply | Threaded
Open this post in threaded view
|

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

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     | 32 ++++++++++++++++++++++++--------
 2 files changed, 28 insertions(+), 8 deletions(-)

diff --git a/bfd/elf.c b/bfd/elf.c
index 1d62523b120..b836f041724 100644
--- a/bfd/elf.c
+++ b/bfd/elf.c
@@ -10132,6 +10132,7 @@ elfcore_grok_lwpstatus (bfd *abfd, Elf_Internal_Note *note)
 #define NOTE_INFO_PROCESS  1
 #define NOTE_INFO_THREAD   2
 #define NOTE_INFO_MODULE   3
+#define NOTE_INFO_MODULE64 4
 
 static bfd_boolean
 elfcore_grok_win32pstatus (bfd *abfd, Elf_Internal_Note *note)
@@ -10199,13 +10200,30 @@ elfcore_grok_win32pstatus (bfd *abfd, Elf_Internal_Note *note)
       break;
 
     case NOTE_INFO_MODULE:
-      if (note->descsz < 12)
-        return FALSE;
-
+    case NOTE_INFO_MODULE64:
       /* Make a ".module/xxxxxxxx" section.  */
-      /* module_info.base_address */
-      base_addr = bfd_get_32 (abfd, note->descdata + 4);
-      sprintf (buf, ".module/%08lx", (unsigned long) base_addr);
+      if (type == NOTE_INFO_MODULE)
+        {
+          if (note->descsz < 12)
+            return FALSE;
+
+          /* module_info.base_address */
+          base_addr = bfd_get_32 (abfd, note->descdata + 4);
+          sprintf (buf, ".module/%08lx", (unsigned long) base_addr);
+          /* module_info.module_name_size */
+          name_size = bfd_get_32 (abfd, note->descdata + 8);
+        }
+      else /* NOTE_INFO_MODULE64 */
+        {
+          if (note->descsz < 16)
+            return FALSE;
+
+          /* module_info.base_address */
+          base_addr = bfd_get_64 (abfd, note->descdata + 4);
+          sprintf (buf, ".module/%016lx", (unsigned long) base_addr);
+          /* module_info.module_name_size */
+          name_size = bfd_get_32 (abfd, note->descdata + 12);
+        }
 
       len = strlen (buf) + 1;
       name = (char *) bfd_alloc (abfd, len);
@@ -10219,8 +10237,6 @@ elfcore_grok_win32pstatus (bfd *abfd, Elf_Internal_Note *note)
       if (sect == NULL)
  return FALSE;
 
-      /* module_info.module_name_size */
-      name_size = bfd_get_32 (abfd, note->descdata + 8);
       if (note->descsz < 12 + name_size)
         return FALSE;
 
--
2.27.0

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 4/5] Refine size constraints applied to win32pstatus ELF notes

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

>> Actually it looks to me like almost all of the returns in this function
>> should be "return FALSE" as there is something wrong with the note...
>
> Perhaps the way this is written at the moment is a bit awkward as we don't really distinguish in the return code between (i) the contents of the note are malformed, and (ii) an internal error occurred while processing the note. Do we really want to stop with an error in both cases?

Hmmm - generate an error/warning message of some kind - definitely.
Stop on the error - probably not.  After all these are core dumps
that we are dealing with and the user is going to want to get as
much information out of them as they can.

Cheers
  Nick

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 4/5] Refine size constraints applied to win32pstatus ELF notes

Jon TURNEY
On 15/07/2020 15:30, Nick Clifton wrote:

>
>>> Actually it looks to me like almost all of the returns in this function
>>> should be "return FALSE" as there is something wrong with the note...
>>
>> Perhaps the way this is written at the moment is a bit awkward as we don't really distinguish in the return code between (i) the contents of the note are malformed, and (ii) an internal error occurred while processing the note. Do we really want to stop with an error in both cases?
>
> Hmmm - generate an error/warning message of some kind - definitely.
> Stop on the error - probably not.  After all these are core dumps
> that we are dealing with and the user is going to want to get as
> much information out of them as they can.

So, that seems to be back to 'return TRUE, but with a warning'.

I attempted to address that with the following patch [6/5]
Reply | Threaded
Open this post in threaded view
|

[PATCH 6/5] Only warn about malformed win32pstatus notes

Jon TURNEY
bfd/ChangeLog:

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

        * elf.c (elfcore_grok_win32pstatus): Warn on malformed
        win32pstatus notes, and return TRUE so we continue rather than
        stopping as if it was an error.
---
 bfd/ChangeLog |  6 ++++++
 bfd/elf.c     | 39 ++++++++++++++++++++++++++-------------
 2 files changed, 32 insertions(+), 13 deletions(-)

diff --git a/bfd/elf.c b/bfd/elf.c
index b836f041724..f28b553b67b 100644
--- a/bfd/elf.c
+++ b/bfd/elf.c
@@ -10154,21 +10154,36 @@ elfcore_grok_win32pstatus (bfd *abfd, Elf_Internal_Note *note)
 
   type = bfd_get_32 (abfd, note->descdata);
 
+  struct {
+    const char *type_name;
+    unsigned long min_size;
+  } size_check[] =
+      {
+       { "NOTE_INFO_PROCESS", 12 },
+       { "NOTE_INFO_THREAD", 12 },
+       { "NOTE_INFO_MODULE", 12 },
+       { "NOTE_INFO_MODULE64", 16 },
+      };
+
+  if (type > (sizeof(size_check)/sizeof(size_check[0])))
+      return TRUE;
+
+  if (note->descsz < size_check[type - 1].min_size)
+    {
+      _bfd_error_handler (_("%pB: warning: win32pstatus %s of size %lu bytes is too small"),
+                          abfd, size_check[type - 1].type_name, note->descsz);
+      return TRUE;
+    }
+
   switch (type)
     {
     case NOTE_INFO_PROCESS:
-      if (note->descsz < 12)
-        return FALSE;
-
       /* FIXME: need to add ->core->command.  */
       elf_tdata (abfd)->core->pid = bfd_get_32 (abfd, note->descdata + 4);
       elf_tdata (abfd)->core->signal = bfd_get_32 (abfd, note->descdata + 8);
       break;
 
     case NOTE_INFO_THREAD:
-      if (note->descsz < 12)
-        return FALSE;
-
       /* Make a ".reg/<tid>" section containing the Win32 API thread CONTEXT
          structure. */
       /* thread_info.tid */
@@ -10204,9 +10219,6 @@ elfcore_grok_win32pstatus (bfd *abfd, Elf_Internal_Note *note)
       /* Make a ".module/xxxxxxxx" section.  */
       if (type == NOTE_INFO_MODULE)
         {
-          if (note->descsz < 12)
-            return FALSE;
-
           /* module_info.base_address */
           base_addr = bfd_get_32 (abfd, note->descdata + 4);
           sprintf (buf, ".module/%08lx", (unsigned long) base_addr);
@@ -10215,9 +10227,6 @@ elfcore_grok_win32pstatus (bfd *abfd, Elf_Internal_Note *note)
         }
       else /* NOTE_INFO_MODULE64 */
         {
-          if (note->descsz < 16)
-            return FALSE;
-
           /* module_info.base_address */
           base_addr = bfd_get_64 (abfd, note->descdata + 4);
           sprintf (buf, ".module/%016lx", (unsigned long) base_addr);
@@ -10238,7 +10247,11 @@ elfcore_grok_win32pstatus (bfd *abfd, Elf_Internal_Note *note)
  return FALSE;
 
       if (note->descsz < 12 + name_size)
-        return FALSE;
+        {
+          _bfd_error_handler (_("%pB: win32pstatus NOTE_INFO_MODULE of size %lu is too small to contain a name of size %zu"),
+                              abfd, note->descsz, name_size);
+          return TRUE;
+        }
 
       sect->size = note->descsz;
       sect->filepos = note->descpos;
--
2.27.0

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 6/5] Only warn about malformed win32pstatus notes

Jon TURNEY
On 21/07/2020 14:15, Jon Turney wrote:
> bfd/ChangeLog:
>
> 2020-07-21  Jon Turney  <[hidden email]>
>
> * elf.c (elfcore_grok_win32pstatus): Warn on malformed
> win32pstatus notes, and return TRUE so we continue rather than
> stopping as if it was an error.
> ---

Ping?