diff mbox series

[RFC] PR 34481 arbitrary limit on decompressed size of .dwo files

Message ID ao0kpuAp4iFRJAKQ@squeak.grove.modra.org
State New
Headers
Series [RFC] PR 34481 arbitrary limit on decompressed size of .dwo files |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_binutils_build--master-arm warning Skipped because it is an RFC
linaro-tcwg-bot/tcwg_binutils_build--master-aarch64 warning Skipped because it is an RFC

Commit Message

Alan Modra Aug. 25, 2026, 5:14 a.m. UTC 
  PR34481 exposes a failure of a heuristic in bfd_section_size_insane
dealing with compressed sections.  The assumption there is that a
compressed section would not be more than ten times the total file
size.  This of course is foolish since one highly compressed section
can easily exceed ten times the total file size.  However, it mostly
worked for real object files.  The claim in pr34481 is that the limit
has been hit for .dwo files in a real C++ codebase.  I don't find that
claim unbelievable.

While we could work around the .dwo problem with the following patch,
I'm inclined to simply remove the whole "size / 10 > filesize" block.

My reasoning is that this is anti-fuzzer code.  If we allow a .dwo
hole then the anti-fuzzer code may as well not be there.  Fuzzers will
soon find the hole.

What do you all think?

	* section.c (bfd_section_size_insane): Do not attempt to limit
	.dwo section sizes.

Comments

Jan Beulich Aug. 25, 2026, 7:05 a.m. UTC | #1 
On 25.08.2026 07:14, Alan Modra wrote:
> PR34481 exposes a failure of a heuristic in bfd_section_size_insane
> dealing with compressed sections.  The assumption there is that a
> compressed section would not be more than ten times the total file
> size.  This of course is foolish since one highly compressed section
> can easily exceed ten times the total file size.  However, it mostly
> worked for real object files.  The claim in pr34481 is that the limit
> has been hit for .dwo files in a real C++ codebase.  I don't find that
> claim unbelievable.
> 
> While we could work around the .dwo problem with the following patch,
> I'm inclined to simply remove the whole "size / 10 > filesize" block.
> 
> My reasoning is that this is anti-fuzzer code.  If we allow a .dwo
> hole then the anti-fuzzer code may as well not be there.  Fuzzers will
> soon find the hole.
> 
> What do you all think?

I agree, fwiw. (I don't like such arbitrary limits anyway.)

Jan
Alan Modra Aug. 27, 2026, 1:20 p.m. UTC | #2 
This is what I'm about to commit.

int aaaa..a;  where 'a' is repeated a million times, produces a
-g -gsplit-dwarf -gz .dwo file of only 2200 bytes.  This might be a
silly testcase, but it demonstrates the ten times file size limit when
decompressing .debug_str.dwo is easily exceeded.

	PR 26946
	PR 28834
	PR 34481
bfd/
	* section.c (bfd_section_size_insane): Do not attempt to sanity
	check compressed sections.
binutils/
	* readelf.c (uncompress_section_contents): Do not limit uncompressed
	section size.  Remove now unused file_size param.  Adjust callers.

diff --git a/bfd/section.c b/bfd/section.c
index 457486b0f89..fb2cc830dbd 100644
--- a/bfd/section.c
+++ b/bfd/section.c
@@ -1765,23 +1765,7 @@ bfd_section_size_insane (bfd *abfd, asection *sec)
 
   if (sec->compress_status == DECOMPRESS_SECTION_ZSTD
       || sec->compress_status == DECOMPRESS_SECTION_ZLIB)
-    {
-      /* PR26946, PR28834: Sanity check compress header uncompressed
-	 size against the original file size, and check that the
-	 compressed section can be read from file.  We choose an
-	 arbitrary uncompressed size of 10x the file size, rather than
-	 a compress ratio.  The reason being that compiling
-	 "int aaa..a;" with "a" repeated enough times can result in
-	 compression ratios without limit for .debug_str, whereas such
-	 a file will usually also have the enormous symbol
-	 uncompressed in .symtab.  */
-     if (size / 10 > filesize)
-       {
-	 bfd_set_error (bfd_error_bad_value);
-	 return true;
-       }
-     size = sec->compressed_size;
-    }
+    size = sec->compressed_size;
 
   if ((ufile_ptr) sec->filepos > filesize || size > filesize - sec->filepos)
     {
diff --git a/binutils/readelf.c b/binutils/readelf.c
index b5ccc675af6..aa472947cde 100644
--- a/binutils/readelf.c
+++ b/binutils/readelf.c
@@ -16597,8 +16597,7 @@ static bool
 uncompress_section_contents (bool              is_zstd,
 			     unsigned char **  buffer,
 			     uint64_t          uncompressed_size,
-			     uint64_t *        size,
-			     uint64_t          file_size)
+			     uint64_t *        size)
 {
   uint64_t compressed_size = *size;
   unsigned char *compressed_buffer = *buffer;
@@ -16606,16 +16605,6 @@ uncompress_section_contents (bool              is_zstd,
   z_stream strm;
   int rc;
 
-  /* Similar to bfd_section_size_insane() in the BFD library we expect an
-     upper limit of ~10x compression.  Any compression larger than that is
-     thought to be due to fuzzing of the compression header.  */
-  if (uncompressed_size > file_size * 10)
-    {
-      error (_("Uncompressed section size is suspiciously large: 0x%" PRIu64 "\n"),
-	       uncompressed_size);
-      goto fail;
-    }
-
   uncompressed_buffer = xmalloc (uncompressed_size);
 
   if (is_zstd)
@@ -16732,7 +16721,7 @@ maybe_expand_or_relocate_section (Elf_Internal_Shdr *  section,
       if (uncompressed_size)
 	{
 	  if (uncompress_section_contents (is_zstd, &start, uncompressed_size,
-					   &new_size, filedata->file_size))
+					   &new_size))
 	    {
 	      *decomp_buf = start;
 	      section_size = new_size;
@@ -17315,7 +17304,7 @@ load_specific_debug_section (enum dwarf_section_display_enum  debug,
       if (uncompressed_size)
 	{
 	  if (uncompress_section_contents (is_zstd, &start, uncompressed_size,
-					   &size, filedata->file_size))
+					   &size))
 	    {
 	      /* Free the compressed buffer, update the section buffer
 		 and the section size if uncompress is successful.  */
diff mbox series

Patch

diff --git a/bfd/section.c b/bfd/section.c
index 457486b0f89..13cb2fdeb2d 100644
--- a/bfd/section.c
+++ b/bfd/section.c
@@ -1774,11 +1774,17 @@  bfd_section_size_insane (bfd *abfd, asection *sec)
 	 "int aaa..a;" with "a" repeated enough times can result in
 	 compression ratios without limit for .debug_str, whereas such
 	 a file will usually also have the enormous symbol
-	 uncompressed in .symtab.  */
+	 uncompressed in .symtab.  PR34481: For separare dwarf info
+	 files we won't have a .symtab section so can't make any
+	 assumptions about decompressed section sizes.  */
      if (size / 10 > filesize)
        {
-	 bfd_set_error (bfd_error_bad_value);
-	 return true;
+	 size_t len = strlen (sec->name);
+	 if (len < 4 || memcmp (sec->name + len - 4, ".dwo", 4) != 0)
+	   {
+	     bfd_set_error (bfd_error_bad_value);
+	     return true;
+	   }
        }
      size = sec->compressed_size;
     }