Re: [committed, PATCH] Check file size before getting section contents

["H.J. Lu" <hjl dot tools at gmail dot com>]

On Mon, Jun 26, 2017 at 2:48 PM, Pedro Alves <palves@redhat.com> wrote:
> Hi H.J.,
>
> This patch caused many regressions in GDB's testsuite:
>
>   https://sourceware.org/ml/gdb-testers/2017-q2/msg05045.html
>
> x86 buildslaves haven't caught up with the patch yet, but I
> see the same thing locally, on my x86-64 machine.  (git bisect
> pointed me here.)
>
> Here's quick way to run only a few of the tests that are now failing:
>
>  $ make check TESTS="gdb.base/break-interp.exp gdb.base/corefile.exp gdb.base/solib-search.exp gdb.reverse/sigall-precsave.exp"
>
> Any idea what's going on?
>
> BTW, this looks very suspicious:
>
>  @@ -867,7 +876,13 @@ _bfd_generic_get_section_contents_in_window
>       sz = section->rawsize;
>     else
>       sz = section->size;
>  +  filesz = bfd_get_file_size (abfd);
>  +    {
>  +      /* This should never happen.  */
>  +      abort ();
>  +    }
>
> ... because that abort is unconditional.  Did you intend
> to guard it with:
>    if (filesz < 0)
> like in _bfd_generic_get_section_contents?

I checked in this patch:

commit 1f473e3d0ad285195934e6a077c7ed32afe66437
Author: H.J. Lu <hjl.tools@gmail.com>
Date:   Mon Jun 26 15:47:16 2017 -0700

    Add a missing line to _bfd_generic_get_section_contents_in_window

            PR binutils/21665
            * libbfd.c (_bfd_generic_get_section_contents_in_window): Add
            a missing line.

diff --git a/bfd/ChangeLog b/bfd/ChangeLog
index cd5cb57..fbe2049 100644
--- a/bfd/ChangeLog
+++ b/bfd/ChangeLog
@@ -1,3 +1,9 @@
+2017-06-26  H.J. Lu  <hongjiu.lu@intel.com>
+
+ PR binutils/21665
+ * libbfd.c (_bfd_generic_get_section_contents_in_window): Add
+ a missing line.
+
 2017-06-26  Maciej W. Rozycki  <macro@imgtec.com>

  * cpu-mips.c (arch_info_struct): Mark the 4010 32-bit.
diff --git a/bfd/libbfd.c b/bfd/libbfd.c
index b903328..7b270ef 100644
--- a/bfd/libbfd.c
+++ b/bfd/libbfd.c
@@ -877,6 +877,7 @@ _bfd_generic_get_section_contents_in_window
   else
     sz = section->size;
   filesz = bfd_get_file_size (abfd);
+  if (filesz < 0)
     {
       /* This should never happen.  */
       abort ();

> Thanks,
> Pedro Alves
>
> On 06/26/2017 05:31 PM, H.J. Lu wrote:
>> Don't check the section size in bfd_get_full_section_contents since
>> the size of a decompressed section may be larger than the file size.
>> Instead, check file size in _bfd_generic_get_section_contents.
>>
>>       PR binutils/21665
>>       * compress.c (bfd_get_full_section_contents): Don't check the
>>       file size here.
>>       * libbfd.c (_bfd_generic_get_section_contents): Check for and
>>       reject a section whoes size + offset is greater than the size
>>       of the entire file.
>>       (_bfd_generic_get_section_contents_in_window): Likewise.
>
>



-- 
H.J.