Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* [PATCH/RFC] Reorganize osabi.c:generic_elf_osabi_sniff_abi_tag_sections()
@ 2004-01-18 20:40 Mark Kettenis
  2004-01-18 22:52 ` Daniel Jacobowitz
  0 siblings, 1 reply; 6+ messages in thread
From: Mark Kettenis @ 2004-01-18 20:40 UTC (permalink / raw)
  To: gdb-patches

While adding support for OpenBSD ELF, I found myself duplicating the
same bit of code for checking ELF notes again.  This patch puts that
bit of code in its own function.  The new function adds a check
whether the nore section is large enough to contain the note that
we're checked for.

If nobody objects, I'll commit this in a few days,

Mark


Index: ChangeLog
from  Mark Kettenis  <kettenis@gnu.org>

	* osabi.c (MAX_NOTESZ): New define.
	(check_note): New function.
	(generic_elf_osabi_sniff_abi_tag_sections): Reorganize code using
	check_note.

Index: osabi.c
===================================================================
RCS file: /cvs/src/src/gdb/osabi.c,v
retrieving revision 1.25
diff -u -p -r1.25 osabi.c
--- osabi.c 17 Jan 2004 00:13:46 -0000 1.25
+++ osabi.c 18 Jan 2004 20:39:29 -0000
@@ -355,119 +355,126 @@ gdbarch_init_osabi (struct gdbarch_info 
      info.bfd_arch_info->printable_name);
 }
 \f
+/* Limit on the amount of data to be read.  */
+#define MAX_NOTESZ	128
+
+/* Return non-zero if NOTE matches NAME, DESCSZ and TYPE.  */
+
+static int
+check_note (bfd *abfd, asection *sect, const char *note,
+	    const char *name, unsigned long descsz, unsigned long type)
+{
+  unsigned long notesz;
+
+  /* Calculate the size of this note...  */
+  notesz = strlen (name) + 1;
+  notesz = ((notesz + 3) & ~3);
+  notesz += descsz;
+  notesz = ((notesz + 3) & ~3);
+
+  /* ...and check it.  */
+  gdb_assert (notesz <= MAX_NOTESZ);
+  if (notesz > bfd_section_size (abfd, sect))
+    return 0;
+
+  /* Check the note name.  */
+  if (bfd_h_get_32 (abfd, note) != (strlen (name) + 1)
+      || strcmp (note + 12, name) != 0)
+    return 0;
+
+  /* Check the descriptor size.  */
+  if (bfd_h_get_32 (abfd, note + 4) != descsz)
+    return 0;
+
+  /* Check the note type.  */
+  if (bfd_h_get_32 (abfd, note + 8) != type)
+    return 0;
+
+  return 1;
+}
 
 /* Generic sniffer for ELF flavoured files.  */
 
 void
 generic_elf_osabi_sniff_abi_tag_sections (bfd *abfd, asection *sect, void *obj)
 {
-  enum gdb_osabi *os_ident_ptr = obj;
+  enum gdb_osabi *osabi = obj;
   const char *name;
   unsigned int sectsize;
+  char *note;
 
   name = bfd_get_section_name (abfd, sect);
   sectsize = bfd_section_size (abfd, sect);
 
-  /* .note.ABI-tag notes, used by GNU/Linux and FreeBSD.  */
-  if (strcmp (name, ".note.ABI-tag") == 0 && sectsize > 0)
-    {
-      unsigned int name_length, data_length, note_type;
-      char *note;
+  /* Limit the amount of data to read.  */
+  if (sectsize > MAX_NOTESZ)
+    sectsize = MAX_NOTESZ;
 
-      /* If the section is larger than this, it's probably not what we are
-	 looking for.  */
-      if (sectsize > 128)
-	sectsize = 128;
+  note = alloca (sectsize);
+  bfd_get_section_contents (abfd, sect, note, 0, sectsize);
 
-      note = alloca (sectsize);
-
-      bfd_get_section_contents (abfd, sect, note,
-				(file_ptr) 0, (bfd_size_type) sectsize);
-
-      name_length = bfd_h_get_32 (abfd, note);
-      data_length = bfd_h_get_32 (abfd, note + 4);
-      note_type   = bfd_h_get_32 (abfd, note + 8);
-
-      if (name_length == 4 && data_length == 16 && note_type == NT_GNU_ABI_TAG
-	  && strcmp (note + 12, "GNU") == 0)
+  /* .note.ABI-tag notes, used by GNU/Linux and FreeBSD.  */
+  if (strcmp (name, ".note.ABI-tag") == 0)
+    {
+      /* GNU.  */
+      if (check_note (abfd, sect, note, "GNU", 16, NT_GNU_ABI_TAG))
 	{
-	  int os_number = bfd_h_get_32 (abfd, note + 16);
+	  unsigned int abi_tag = bfd_h_get_32 (abfd, note + 16);
 
-	  switch (os_number)
+	  switch (abi_tag)
 	    {
 	    case GNU_ABI_TAG_LINUX:
-	      *os_ident_ptr = GDB_OSABI_LINUX;
+	      *osabi = GDB_OSABI_LINUX;
 	      break;
 
 	    case GNU_ABI_TAG_HURD:
-	      *os_ident_ptr = GDB_OSABI_HURD;
+	      *osabi = GDB_OSABI_HURD;
 	      break;
 
 	    case GNU_ABI_TAG_SOLARIS:
-	      *os_ident_ptr = GDB_OSABI_SOLARIS;
+	      *osabi = GDB_OSABI_SOLARIS;
 	      break;
 
 	    case GNU_ABI_TAG_FREEBSD:
-	      *os_ident_ptr = GDB_OSABI_FREEBSD_ELF;
+	      *osabi = GDB_OSABI_FREEBSD_ELF;
 	      break;
-	      
+
 	    case GNU_ABI_TAG_NETBSD:
-	      *os_ident_ptr = GDB_OSABI_NETBSD_ELF;
+	      *osabi = GDB_OSABI_NETBSD_ELF;
 	      break;
-	      
+
 	    default:
-	      internal_error
-		(__FILE__, __LINE__,
-		 "generic_elf_osabi_sniff_abi_tag_sections: unknown OS number %d",
-		 os_number);
+	      internal_error (__FILE__, __LINE__, "\
+generic_elf_osabi_sniff_abi_tag_sections: unknown OS number %d",
+			      abi_tag);
 	    }
 	  return;
 	}
-      else if (name_length == 8 && data_length == 4
-	       && note_type == NT_FREEBSD_ABI_TAG
-	       && strcmp (note + 12, "FreeBSD") == 0)
+
+      /* FreeBSD.  */
+      if (check_note (abfd, sect, note, "FreeBSD", 4, NT_FREEBSD_ABI_TAG))
 	{
-	  /* XXX Should we check the version here?  Probably not
-	     necessary yet.  */
-	  *os_ident_ptr = GDB_OSABI_FREEBSD_ELF;
+	  /* There is no need to check the version yet.  */
+	  *osabi = GDB_OSABI_FREEBSD_ELF;
+	  return;
 	}
+
       return;
     }
-
+      
   /* .note.netbsd.ident notes, used by NetBSD.  */
-  if (strcmp (name, ".note.netbsd.ident") == 0 && sectsize > 0)
+  if (strcmp (name, ".note.netbsd.ident") == 0
+      && check_note (abfd, sect, note, "NetBSD", 4, NT_NETBSD_IDENT))
     {
-      unsigned int name_length, data_length, note_type;
-      char *note;
-
-      /* If the section is larger than this, it's probably not what we are
-	 looking for.  */
-      if (sectsize > 128) 
-	sectsize = 128;
-
-      note = alloca (sectsize);
-
-      bfd_get_section_contents (abfd, sect, note,
-				(file_ptr) 0, (bfd_size_type) sectsize);
-      
-      name_length = bfd_h_get_32 (abfd, note);
-      data_length = bfd_h_get_32 (abfd, note + 4);
-      note_type   = bfd_h_get_32 (abfd, note + 8);
-
-      if (name_length == 7 && data_length == 4 && note_type == NT_NETBSD_IDENT
-	  && strcmp (note + 12, "NetBSD") == 0)
-	{
-	  /* XXX Should we check the version here?  Probably not
-	     necessary yet.  */
-	  *os_ident_ptr = GDB_OSABI_NETBSD_ELF;
-	}
+      /* There is no need to check the version yet.  */
+      *osabi = GDB_OSABI_NETBSD_ELF;
       return;
     }
 
   /* .note.netbsdcore.procinfo notes, used by NetBSD.  */
-  if (strcmp (name, ".note.netbsdcore.procinfo") == 0 && sectsize > 0)
+  if (strcmp (name, ".note.netbsdcore.procinfo") == 0)
     {
-      *os_ident_ptr = GDB_OSABI_NETBSD_ELF;
+      *osabi = GDB_OSABI_NETBSD_ELF;
       return;
     }
 }


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH/RFC] Reorganize osabi.c:generic_elf_osabi_sniff_abi_tag_sections()
  2004-01-18 20:40 [PATCH/RFC] Reorganize osabi.c:generic_elf_osabi_sniff_abi_tag_sections() Mark Kettenis
@ 2004-01-18 22:52 ` Daniel Jacobowitz
  2004-01-19 18:39   ` Mark Kettenis
  0 siblings, 1 reply; 6+ messages in thread
From: Daniel Jacobowitz @ 2004-01-18 22:52 UTC (permalink / raw)
  To: gdb-patches

On Sun, Jan 18, 2004 at 09:40:40PM +0100, Mark Kettenis wrote:
> +  /* ...and check it.  */
> +  gdb_assert (notesz <= MAX_NOTESZ);
> +  if (notesz > bfd_section_size (abfd, sect))
> +    return 0;

I'd rather not add assertions based on the input file.  Can we complain
and fail instead?

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH/RFC] Reorganize osabi.c:generic_elf_osabi_sniff_abi_tag_sections()
  2004-01-18 22:52 ` Daniel Jacobowitz
@ 2004-01-19 18:39   ` Mark Kettenis
  2004-01-19 19:15     ` Daniel Jacobowitz
  0 siblings, 1 reply; 6+ messages in thread
From: Mark Kettenis @ 2004-01-19 18:39 UTC (permalink / raw)
  To: drow; +Cc: gdb-patches

   Date: Sun, 18 Jan 2004 17:52:54 -0500
   From: Daniel Jacobowitz <drow@mvista.com>

   On Sun, Jan 18, 2004 at 09:40:40PM +0100, Mark Kettenis wrote:
   > +  /* ...and check it.  */
   > +  gdb_assert (notesz <= MAX_NOTESZ);
   > +  if (notesz > bfd_section_size (abfd, sect))
   > +    return 0;

   I'd rather not add assertions based on the input file.  Can we complain
   and fail instead?

Ah, but the assert is an internal consistency check.  It checks
whether the MAX_NOTESZ limit is large enough for the note the caller
of check_note() is checking for.  Basically the assert triggers if
someone adds a check_note() call with a long name, or a large
descriptor size.  In that case the person in question should increase
MAX_NOTESZ.  The assert is guaranteed not to be triggered for the
check_note() calls in my patch.

The following if-statement does a sanity check on the input file.  We
simply return 0 is the section is too small.

I'll add some comments spelling this out before I check this in.

Mark


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH/RFC] Reorganize osabi.c:generic_elf_osabi_sniff_abi_tag_sections()
  2004-01-19 18:39   ` Mark Kettenis
@ 2004-01-19 19:15     ` Daniel Jacobowitz
  2004-01-21 23:08       ` Mark Kettenis
  0 siblings, 1 reply; 6+ messages in thread
From: Daniel Jacobowitz @ 2004-01-19 19:15 UTC (permalink / raw)
  To: gdb-patches

On Mon, Jan 19, 2004 at 07:39:03PM +0100, Mark Kettenis wrote:
>    Date: Sun, 18 Jan 2004 17:52:54 -0500
>    From: Daniel Jacobowitz <drow@mvista.com>
> 
>    On Sun, Jan 18, 2004 at 09:40:40PM +0100, Mark Kettenis wrote:
>    > +  /* ...and check it.  */
>    > +  gdb_assert (notesz <= MAX_NOTESZ);
>    > +  if (notesz > bfd_section_size (abfd, sect))
>    > +    return 0;
> 
>    I'd rather not add assertions based on the input file.  Can we complain
>    and fail instead?
> 
> Ah, but the assert is an internal consistency check.  It checks
> whether the MAX_NOTESZ limit is large enough for the note the caller
> of check_note() is checking for.  Basically the assert triggers if
> someone adds a check_note() call with a long name, or a large
> descriptor size.  In that case the person in question should increase
> MAX_NOTESZ.  The assert is guaranteed not to be triggered for the
> check_note() calls in my patch.

Oh, you're right - I leapt to the conclusion that this was based on the
size of notes in the input.  Sorry.

-- 
Daniel Jacobowitz
MontaVista Software                         Debian GNU/Linux Developer


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH/RFC] Reorganize osabi.c:generic_elf_osabi_sniff_abi_tag_sections()
  2004-01-19 19:15     ` Daniel Jacobowitz
@ 2004-01-21 23:08       ` Mark Kettenis
  2004-01-22  7:35         ` Eli Zaretskii
  0 siblings, 1 reply; 6+ messages in thread
From: Mark Kettenis @ 2004-01-21 23:08 UTC (permalink / raw)
  To: gdb-patches

Daniel's confusement (can I say that in english?) caused me to tweak
some comments.  The attached is what I committed.

Mark


Index: ChangeLog
from  Mark Kettenis  <kettenis@gnu.org>

	* osabi.c (MAX_NOTESZ): New define.
	(check_note): New function.
	(generic_elf_osabi_sniff_abi_tag_sections): Reorganize code using
	check_note.

Index: osabi.c
===================================================================
RCS file: /cvs/src/src/gdb/osabi.c,v
retrieving revision 1.25
diff -u -p -r1.25 osabi.c
--- osabi.c 17 Jan 2004 00:13:46 -0000 1.25
+++ osabi.c 21 Jan 2004 23:02:22 -0000
@@ -355,119 +355,128 @@ gdbarch_init_osabi (struct gdbarch_info 
      info.bfd_arch_info->printable_name);
 }
 \f
+/* Limit on the amount of data to be read.  */
+#define MAX_NOTESZ	128
+
+/* Return non-zero if NOTE matches NAME, DESCSZ and TYPE.  */
+
+static int
+check_note (bfd *abfd, asection *sect, const char *note,
+	    const char *name, unsigned long descsz, unsigned long type)
+{
+  unsigned long notesz;
+
+  /* Calculate the size of this note.  */
+  notesz = strlen (name) + 1;
+  notesz = ((notesz + 3) & ~3);
+  notesz += descsz;
+  notesz = ((notesz + 3) & ~3);
+
+  /* If this assertion triggers, increase MAX_NOTESZ.  */
+  gdb_assert (notesz <= MAX_NOTESZ);
+
+  /* Check whether SECT is big enough to comtain the complete note.  */
+  if (notesz > bfd_section_size (abfd, sect))
+    return 0;
+
+  /* Check the note name.  */
+  if (bfd_h_get_32 (abfd, note) != (strlen (name) + 1)
+      || strcmp (note + 12, name) != 0)
+    return 0;
+
+  /* Check the descriptor size.  */
+  if (bfd_h_get_32 (abfd, note + 4) != descsz)
+    return 0;
+
+  /* Check the note type.  */
+  if (bfd_h_get_32 (abfd, note + 8) != type)
+    return 0;
+
+  return 1;
+}
 
 /* Generic sniffer for ELF flavoured files.  */
 
 void
 generic_elf_osabi_sniff_abi_tag_sections (bfd *abfd, asection *sect, void *obj)
 {
-  enum gdb_osabi *os_ident_ptr = obj;
+  enum gdb_osabi *osabi = obj;
   const char *name;
   unsigned int sectsize;
+  char *note;
 
   name = bfd_get_section_name (abfd, sect);
   sectsize = bfd_section_size (abfd, sect);
 
-  /* .note.ABI-tag notes, used by GNU/Linux and FreeBSD.  */
-  if (strcmp (name, ".note.ABI-tag") == 0 && sectsize > 0)
-    {
-      unsigned int name_length, data_length, note_type;
-      char *note;
+  /* Limit the amount of data to read.  */
+  if (sectsize > MAX_NOTESZ)
+    sectsize = MAX_NOTESZ;
 
-      /* If the section is larger than this, it's probably not what we are
-	 looking for.  */
-      if (sectsize > 128)
-	sectsize = 128;
+  note = alloca (sectsize);
+  bfd_get_section_contents (abfd, sect, note, 0, sectsize);
 
-      note = alloca (sectsize);
-
-      bfd_get_section_contents (abfd, sect, note,
-				(file_ptr) 0, (bfd_size_type) sectsize);
-
-      name_length = bfd_h_get_32 (abfd, note);
-      data_length = bfd_h_get_32 (abfd, note + 4);
-      note_type   = bfd_h_get_32 (abfd, note + 8);
-
-      if (name_length == 4 && data_length == 16 && note_type == NT_GNU_ABI_TAG
-	  && strcmp (note + 12, "GNU") == 0)
+  /* .note.ABI-tag notes, used by GNU/Linux and FreeBSD.  */
+  if (strcmp (name, ".note.ABI-tag") == 0)
+    {
+      /* GNU.  */
+      if (check_note (abfd, sect, note, "GNU", 16, NT_GNU_ABI_TAG))
 	{
-	  int os_number = bfd_h_get_32 (abfd, note + 16);
+	  unsigned int abi_tag = bfd_h_get_32 (abfd, note + 16);
 
-	  switch (os_number)
+	  switch (abi_tag)
 	    {
 	    case GNU_ABI_TAG_LINUX:
-	      *os_ident_ptr = GDB_OSABI_LINUX;
+	      *osabi = GDB_OSABI_LINUX;
 	      break;
 
 	    case GNU_ABI_TAG_HURD:
-	      *os_ident_ptr = GDB_OSABI_HURD;
+	      *osabi = GDB_OSABI_HURD;
 	      break;
 
 	    case GNU_ABI_TAG_SOLARIS:
-	      *os_ident_ptr = GDB_OSABI_SOLARIS;
+	      *osabi = GDB_OSABI_SOLARIS;
 	      break;
 
 	    case GNU_ABI_TAG_FREEBSD:
-	      *os_ident_ptr = GDB_OSABI_FREEBSD_ELF;
+	      *osabi = GDB_OSABI_FREEBSD_ELF;
 	      break;
-	      
+
 	    case GNU_ABI_TAG_NETBSD:
-	      *os_ident_ptr = GDB_OSABI_NETBSD_ELF;
+	      *osabi = GDB_OSABI_NETBSD_ELF;
 	      break;
-	      
+
 	    default:
-	      internal_error
-		(__FILE__, __LINE__,
-		 "generic_elf_osabi_sniff_abi_tag_sections: unknown OS number %d",
-		 os_number);
+	      internal_error (__FILE__, __LINE__, "\
+generic_elf_osabi_sniff_abi_tag_sections: unknown OS number %d",
+			      abi_tag);
 	    }
 	  return;
 	}
-      else if (name_length == 8 && data_length == 4
-	       && note_type == NT_FREEBSD_ABI_TAG
-	       && strcmp (note + 12, "FreeBSD") == 0)
+
+      /* FreeBSD.  */
+      if (check_note (abfd, sect, note, "FreeBSD", 4, NT_FREEBSD_ABI_TAG))
 	{
-	  /* XXX Should we check the version here?  Probably not
-	     necessary yet.  */
-	  *os_ident_ptr = GDB_OSABI_FREEBSD_ELF;
+	  /* There is no need to check the version yet.  */
+	  *osabi = GDB_OSABI_FREEBSD_ELF;
+	  return;
 	}
+
       return;
     }
-
+      
   /* .note.netbsd.ident notes, used by NetBSD.  */
-  if (strcmp (name, ".note.netbsd.ident") == 0 && sectsize > 0)
+  if (strcmp (name, ".note.netbsd.ident") == 0
+      && check_note (abfd, sect, note, "NetBSD", 4, NT_NETBSD_IDENT))
     {
-      unsigned int name_length, data_length, note_type;
-      char *note;
-
-      /* If the section is larger than this, it's probably not what we are
-	 looking for.  */
-      if (sectsize > 128) 
-	sectsize = 128;
-
-      note = alloca (sectsize);
-
-      bfd_get_section_contents (abfd, sect, note,
-				(file_ptr) 0, (bfd_size_type) sectsize);
-      
-      name_length = bfd_h_get_32 (abfd, note);
-      data_length = bfd_h_get_32 (abfd, note + 4);
-      note_type   = bfd_h_get_32 (abfd, note + 8);
-
-      if (name_length == 7 && data_length == 4 && note_type == NT_NETBSD_IDENT
-	  && strcmp (note + 12, "NetBSD") == 0)
-	{
-	  /* XXX Should we check the version here?  Probably not
-	     necessary yet.  */
-	  *os_ident_ptr = GDB_OSABI_NETBSD_ELF;
-	}
+      /* There is no need to check the version yet.  */
+      *osabi = GDB_OSABI_NETBSD_ELF;
       return;
     }
 
   /* .note.netbsdcore.procinfo notes, used by NetBSD.  */
-  if (strcmp (name, ".note.netbsdcore.procinfo") == 0 && sectsize > 0)
+  if (strcmp (name, ".note.netbsdcore.procinfo") == 0)
     {
-      *os_ident_ptr = GDB_OSABI_NETBSD_ELF;
+      *osabi = GDB_OSABI_NETBSD_ELF;
       return;
     }
 }


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH/RFC] Reorganize osabi.c:generic_elf_osabi_sniff_abi_tag_sections()
  2004-01-21 23:08       ` Mark Kettenis
@ 2004-01-22  7:35         ` Eli Zaretskii
  0 siblings, 0 replies; 6+ messages in thread
From: Eli Zaretskii @ 2004-01-22  7:35 UTC (permalink / raw)
  To: Mark Kettenis; +Cc: gdb-patches

> Date: Thu, 22 Jan 2004 00:08:09 +0100 (CET)
> From: Mark Kettenis <kettenis@chello.nl>
> 
> Daniel's confusement (can I say that in english?)

I believe you meant to say ``confusion''.


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2004-01-22  7:35 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2004-01-18 20:40 [PATCH/RFC] Reorganize osabi.c:generic_elf_osabi_sniff_abi_tag_sections() Mark Kettenis
2004-01-18 22:52 ` Daniel Jacobowitz
2004-01-19 18:39   ` Mark Kettenis
2004-01-19 19:15     ` Daniel Jacobowitz
2004-01-21 23:08       ` Mark Kettenis
2004-01-22  7:35         ` Eli Zaretskii

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox