public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
From: Alan Modra <amodra@gmail.com>
To: binutils@sourceware.org
Subject: Pool section entries for DWP version 1
Date: Sun, 30 Oct 2022 20:25:33 +1030	[thread overview]
Message-ID: <Y15KFY32CyFhh/+u@squeak.grove.modra.org> (raw)

Ref: https://gcc.gnu.org/wiki/DebugFissionDWP?action=recall&rev=3

Fuzzers have found a weakness in the code stashing pool section
entries.  With random nonsensical values in the index entries (rather
than each index pointing to its own set distinct from other sets),
it's possible to overflow the space allocated, losing the NULL
terminator.  Without a terminator, find_section_in_set can run off the
end of the shndx_pool buffer.  Fix this by scanning the pool directly.

Does anyone still have dwp version 1 files they can use to test my
change?

binutils/
	* dwarf.c (add_shndx_to_cu_tu_entry): Delete range check.
	(end_cu_tu_entry): Likewise.
	(process_cu_tu_index): Fill shndx_pool by directly scanning
	pool, rather than indirectly from index entries.

diff --git a/binutils/dwarf.c b/binutils/dwarf.c
index c6340a28906..7730293326a 100644
--- a/binutils/dwarf.c
+++ b/binutils/dwarf.c
@@ -10652,22 +10652,12 @@ prealloc_cu_tu_list (unsigned int nshndx)
 static void
 add_shndx_to_cu_tu_entry (unsigned int shndx)
 {
-  if (shndx_pool_used >= shndx_pool_size)
-    {
-      error (_("Internal error: out of space in the shndx pool.\n"));
-      return;
-    }
   shndx_pool [shndx_pool_used++] = shndx;
 }
 
 static void
 end_cu_tu_entry (void)
 {
-  if (shndx_pool_used >= shndx_pool_size)
-    {
-      error (_("Internal error: out of space in the shndx pool.\n"));
-      return;
-    }
   shndx_pool [shndx_pool_used++] = 0;
 }
 
@@ -10773,53 +10763,55 @@ process_cu_tu_index (struct dwarf_section *section, int do_display)
 
   if (version == 1)
     {
+      unsigned char *shndx_list;
+      unsigned int shndx;
+
       if (!do_display)
-	prealloc_cu_tu_list ((limit - ppool) / 4);
-      for (i = 0; i < nslots; i++)
 	{
-	  unsigned char *shndx_list;
-	  unsigned int shndx;
-
-	  SAFE_BYTE_GET (signature, phash, 8, limit);
-	  if (signature != 0)
+	  prealloc_cu_tu_list ((limit - ppool) / 4);
+	  for (shndx_list = ppool + 4; shndx_list <= limit - 4; shndx_list += 4)
 	    {
-	      SAFE_BYTE_GET (j, pindex, 4, limit);
-	      shndx_list = ppool + j * 4;
-	      /* PR 17531: file: 705e010d.  */
-	      if (shndx_list < ppool)
-		{
-		  warn (_("Section index pool located before start of section\n"));
-		  return 0;
-		}
+	      shndx = byte_get (shndx_list, 4);
+	      add_shndx_to_cu_tu_entry (shndx);
+	    }
+	  end_cu_tu_entry ();
+	}
+      else
+	for (i = 0; i < nslots; i++)
+	  {
+	    SAFE_BYTE_GET (signature, phash, 8, limit);
+	    if (signature != 0)
+	      {
+		SAFE_BYTE_GET (j, pindex, 4, limit);
+		shndx_list = ppool + j * 4;
+		/* PR 17531: file: 705e010d.  */
+		if (shndx_list < ppool)
+		  {
+		    warn (_("Section index pool located before start of section\n"));
+		    return 0;
+		  }
 
-	      if (do_display)
 		printf (_("  [%3d] Signature:  %#" PRIx64 "  Sections: "),
 			i, signature);
-	      for (;;)
-		{
-		  if (shndx_list >= limit)
-		    {
-		      warn (_("Section %s too small for shndx pool\n"),
-			    section->name);
-		      return 0;
-		    }
-		  SAFE_BYTE_GET (shndx, shndx_list, 4, limit);
-		  if (shndx == 0)
-		    break;
-		  if (do_display)
+		for (;;)
+		  {
+		    if (shndx_list >= limit)
+		      {
+			warn (_("Section %s too small for shndx pool\n"),
+			      section->name);
+			return 0;
+		      }
+		    SAFE_BYTE_GET (shndx, shndx_list, 4, limit);
+		    if (shndx == 0)
+		      break;
 		    printf (" %d", shndx);
-		  else
-		    add_shndx_to_cu_tu_entry (shndx);
-		  shndx_list += 4;
-		}
-	      if (do_display)
+		    shndx_list += 4;
+		  }
 		printf ("\n");
-	      else
-		end_cu_tu_entry ();
-	    }
-	  phash += 8;
-	  pindex += 4;
-	}
+	      }
+	    phash += 8;
+	    pindex += 4;
+	  }
     }
   else if (version == 2)
     {

-- 
Alan Modra
Australia Development Lab, IBM

             reply	other threads:[~2022-10-30  9:55 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-10-30  9:55 Alan Modra [this message]
2022-11-11  0:54 ` Cary Coutant
2022-11-11  6:33   ` Fangrui Song

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Y15KFY32CyFhh/+u@squeak.grove.modra.org \
    --to=amodra@gmail.com \
    --cc=binutils@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).