Loading ...
Sorry, an error occurred while loading the content.

Re: vim-7.2-245 segfaults on startup with glibc fortify

Expand Messages
  • Karsten Hopp
    ... Hello, The build log is available at http://kojipkgs.fedoraproject.org/packages/vim/7.2.245/1.fc12/data/logs/i686/build.log and we were using
    Message 1 of 9 , Aug 4, 2009
    • 0 Attachment
      Am 04.08.2009 14:59, schrieb Dominique Pellé:
      Bram Moolenaar wrote:
      
        
      Karsten Hopp wrote:
      
          
      We've rebuilt all Fedora-12 packages and vim was one of the programs
      that failed to
      run with enhanced glibc fortify features:
      
      vi
      *** buffer overflow detected ***: vim terminated
      ======= Backtrace: =========
      /lib64/libc.so.6(__fortify_fail+0x37)[0x7f780f90df47]
      /lib64/libc.so.6[0x7f780f90bee0]
      vim(eval_init+0x6f)[0x45a5cf]
      vim(main+0x82)[0x4ac952]
      /lib64/libc.so.6(__libc_start_main+0xfd)[0x7f780f835aed]
      vim[0x422069]
      
      
      
      https://bugzilla.redhat.com/show_bug.cgi?id=514717 has all the details
      and 2 suggested fixes.
            
      What Vim does is completely fine.  So your library is broken.  I
      understand that it's intentionally broken to find buffer overflows.
      
      Now, we could work around the broken library.  The diff shows one
      solution.  But this would actually be needed in many more places, it
      just fixes the one found on startup.  When running Vim there are more.
      Takes a long time to find them all.
      
      A proper fix would be:
      - Add configure check for the broken library.
      - When the library is broken use another function instead of STRCPY() to
       avoid using the broken strcpy().  It's slower but should work.
      
      A better solution would be to tell the library not to have this check
      for what it guessed the destination size would be.  Is there an option
      or #define for this?
          
      
      There is such a compilation option:  -D_FORTIFY_SOURCE=0
      
      Patch 7.2.044 already fixed crash which happened  because of
      the Fortify feature.  It was fixed by compiling Vim with
      -D_FORTIFY_SOURCE=1.   Some versions of gcc compile
      by default with -D_FORTIFY_SOURCE=2 (that's the case for
      Ubuntu at least).  -D_FORTIFY_SOURCE=2 can in some cases
      cause valid programs to crash, which is the case for Vim.
      As far as I understand, -D_FORTIFY_SOURCE=1 still has
      some checks but they are relaxed a bit from -D_FORTIFY_SOURCE=2.
      Compiling with -D_FORTIFY_SOURCE=1 should not break
      Vim.  I never see any problem with Vim using -D_FORTIFY_SOURCE=1.
      But if you want to completely disable the Fortify feature, you can compile
      with -D_FORTIFY_SOURCE=0  (or -U_FORTIFY_SOURCE)
      
      This is the Patch in which introduces -D_FORTIFY_SOURCE=1:
      
      ===============
      Patch 7.2.044
      Problem:    Crash because of STRCPY() being over protective of the destination
                 size. (Dominique Pelle)
      Solution:   Add -D_FORTIFY_SOURCE=1 to CFLAGS.  Use an intermediate variable
                 for the pointer to avoid a warning.
      Files:      src/auto/configure, src/configure.in, src/eval.c
      ===============
      
      Is Fedora compiling Vim with -D_FORTIFY_SOURCE=1 or
      -D_FORTIFY_SOURCE=2?
      
      In the absense of -D_FORTIFY_SOURCE compilation option,
      gcc might using -D_FORTIFY_SOURCE=2 by default (as it is the
      case on Ubuntu) which is known to break vim and was fixed
      in patch 7.2.044.
      
      The configure script checks whether to add -D_FORTIFY_SOURCE=1
      to the compilation options. Perhaps this is not happing on Fedora?
      
      Can you give the compilation log to see what compilation options
      were used?
      
      -- Dominique
        


      Hello,

      The build log is available at http://kojipkgs.fedoraproject.org/packages/vim/7.2.245/1.fc12/data/logs/i686/build.log and we were using -D_FORTIFY_SOURCE=2 to compile vim as you've already suspected.

      This compile option comes from the RPM compile options and seems to take precedence over your fortify changes from patch 44 as we now have '-D_FORTIFY_SOURCE=2 -D_FORTIFY_SOURCE=1' on the gcc commandline.


         Karsten



      --~--~---------~--~----~------------~-------~--~----~
      You received this message from the "vim_dev" maillist.
      For more information, visit http://www.vim.org/maillist.php
      -~----------~----~----~----~------~----~------~--~---

    • Bram Moolenaar
      ... Ah, so the configure script is working, but not as expected. Where is the -D_FORTIFY_SOURCE=2 flag coming from? Is it in $CFLAGS? If so then configure
      Message 2 of 9 , Aug 4, 2009
      • 0 Attachment
        Karsten Hopp wrote:


        > Am 04.08.2009 14:59, schrieb Dominique Pellé:
        > > Bram Moolenaar wrote:
        > >
        > >
        > >> Karsten Hopp wrote:
        > >>
        > >>
        > >>> We've rebuilt all Fedora-12 packages and vim was one of the programs
        > >>> that failed to
        > >>> run with enhanced glibc fortify features:
        > >>>
        > >>> vi
        > >>> *** buffer overflow detected ***: vim terminated
        > >>> ======= Backtrace: =========
        > >>> /lib64/libc.so.6(__fortify_fail+0x37)[0x7f780f90df47]
        > >>> /lib64/libc.so.6[0x7f780f90bee0]
        > >>> vim(eval_init+0x6f)[0x45a5cf]
        > >>> vim(main+0x82)[0x4ac952]
        > >>> /lib64/libc.so.6(__libc_start_main+0xfd)[0x7f780f835aed]
        > >>> vim[0x422069]
        > >>>
        > >>>
        > >>>
        > >>> https://bugzilla.redhat.com/show_bug.cgi?id=514717 has all the details
        > >>> and 2 suggested fixes.
        > >>>
        > >> What Vim does is completely fine. So your library is broken. I
        > >> understand that it's intentionally broken to find buffer overflows.
        > >>
        > >> Now, we could work around the broken library. The diff shows one
        > >> solution. But this would actually be needed in many more places, it
        > >> just fixes the one found on startup. When running Vim there are more.
        > >> Takes a long time to find them all.
        > >>
        > >> A proper fix would be:
        > >> - Add configure check for the broken library.
        > >> - When the library is broken use another function instead of STRCPY() to
        > >> avoid using the broken strcpy(). It's slower but should work.
        > >>
        > >> A better solution would be to tell the library not to have this check
        > >> for what it guessed the destination size would be. Is there an option
        > >> or #define for this?
        > >>
        > >
        > >
        > > There is such a compilation option: -D_FORTIFY_SOURCE=0
        > >
        > > Patch 7.2.044 already fixed crash which happened because of
        > > the Fortify feature. It was fixed by compiling Vim with
        > > -D_FORTIFY_SOURCE=1. Some versions of gcc compile
        > > by default with -D_FORTIFY_SOURCE=2 (that's the case for
        > > Ubuntu at least). -D_FORTIFY_SOURCE=2 can in some cases
        > > cause valid programs to crash, which is the case for Vim.
        > > As far as I understand, -D_FORTIFY_SOURCE=1 still has
        > > some checks but they are relaxed a bit from -D_FORTIFY_SOURCE=2.
        > > Compiling with -D_FORTIFY_SOURCE=1 should not break
        > > Vim. I never see any problem with Vim using -D_FORTIFY_SOURCE=1.
        > > But if you want to completely disable the Fortify feature, you can compile
        > > with -D_FORTIFY_SOURCE=0 (or -U_FORTIFY_SOURCE)
        > >
        > > This is the Patch in which introduces -D_FORTIFY_SOURCE=1:
        > >
        > > ===============
        > > Patch 7.2.044
        > > Problem: Crash because of STRCPY() being over protective of the destination
        > > size. (Dominique Pelle)
        > > Solution: Add -D_FORTIFY_SOURCE=1 to CFLAGS. Use an intermediate variable
        > > for the pointer to avoid a warning.
        > > Files: src/auto/configure, src/configure.in, src/eval.c
        > > ===============
        > >
        > > Is Fedora compiling Vim with -D_FORTIFY_SOURCE=1 or
        > > -D_FORTIFY_SOURCE=2?
        > >
        > > In the absense of -D_FORTIFY_SOURCE compilation option,
        > > gcc might using -D_FORTIFY_SOURCE=2 by default (as it is the
        > > case on Ubuntu) which is known to break vim and was fixed
        > > in patch 7.2.044.
        > >
        > > The configure script checks whether to add -D_FORTIFY_SOURCE=1
        > > to the compilation options. Perhaps this is not happing on Fedora?
        > >
        > > Can you give the compilation log to see what compilation options
        > > were used?
        > >
        > > -- Dominique
        > >
        >
        >
        > Hello,
        >
        > The build log is available at
        > http://kojipkgs.fedoraproject.org/packages/vim/7.2.245/1.fc12/data/logs/i686/build.log
        > and we were using -D_FORTIFY_SOURCE=2 to compile vim as you've already
        > suspected.
        >
        > This compile option comes from the RPM compile options and seems to take
        > precedence over your fortify changes from patch 44 as we now have
        > '-D_FORTIFY_SOURCE=2 -D_FORTIFY_SOURCE=1' on the gcc commandline.

        Ah, so the configure script is working, but not as expected.

        Where is the -D_FORTIFY_SOURCE=2 flag coming from? Is it in $CFLAGS?
        If so then configure could filter it out.

        --
        hundred-and-one symptoms of being an internet addict:
        145. You e-mail your boss, informing him you'll be late.

        /// Bram Moolenaar -- Bram@... -- http://www.Moolenaar.net \\\
        /// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
        \\\ download, build and distribute -- http://www.A-A-P.org ///
        \\\ help me help AIDS victims -- http://ICCF-Holland.org ///

        --~--~---------~--~----~------------~-------~--~----~
        You received this message from the "vim_dev" maillist.
        For more information, visit http://www.vim.org/maillist.php
        -~----------~----~----~----~------~----~------~--~---
      • James Vega
        ... Actually, you re using -Wp,-D_FORTIFY_SOURCE=2 instead of simply -D_FORTIFY_SOURCE=2. I d take a guess that s the issue because otherwise the later
        Message 3 of 9 , Aug 4, 2009
        • 0 Attachment
          On Tue, Aug 04, 2009 at 05:54:58PM +0200, Karsten Hopp wrote:
          > and we were using -D_FORTIFY_SOURCE=2 to compile vim as you've already
          > suspected.

          Actually, you're using -Wp,-D_FORTIFY_SOURCE=2 instead of simply
          -D_FORTIFY_SOURCE=2. I'd take a guess that's the issue because
          otherwise the later -D_FORTIFY_SOURCE=1 should override the earlier
          definition.

          GCC's documentation recommends not using -Wp since it bypasses the
          compiler driver and directly passes the options down to the
          preprocessor. Maybe that'd be good advice to heed.

          --
          James
          GPG Key: 1024D/61326D40 2003-09-02 James Vega <jamessan@...>
        • Dominique Pellé
          ... It seems so, in the build log at http://kojipkgs.fedoraproject.org/packages/vim/7.2.245/1.fc12/data/logs/i686/build.log, I see... === begin quote === +
          Message 4 of 9 , Aug 4, 2009
          • 0 Attachment
            Bram Moolenaar<Bram@...> wrote:

            > Karsten Hopp wrote:
            >
            >
            >> Am 04.08.2009 14:59, schrieb Dominique Pellé:
            >> > Bram Moolenaar wrote:
            >> >
            >> >
            >> >> Karsten Hopp wrote:
            >> >>
            >> >>
            >> >>> We've rebuilt all Fedora-12 packages and vim was one of the programs
            >> >>> that failed to
            >> >>> run with enhanced glibc fortify features:
            >> >>>
            >> >>> vi
            >> >>> *** buffer overflow detected ***: vim terminated
            >> >>> ======= Backtrace: =========
            >> >>> /lib64/libc.so.6(__fortify_fail+0x37)[0x7f780f90df47]
            >> >>> /lib64/libc.so.6[0x7f780f90bee0]
            >> >>> vim(eval_init+0x6f)[0x45a5cf]
            >> >>> vim(main+0x82)[0x4ac952]
            >> >>> /lib64/libc.so.6(__libc_start_main+0xfd)[0x7f780f835aed]
            >> >>> vim[0x422069]
            >> >>>
            >> >>>
            >> >>>
            >> >>> https://bugzilla.redhat.com/show_bug.cgi?id=514717 has all the details
            >> >>> and 2 suggested fixes.
            >> >>>
            >> >> What Vim does is completely fine.  So your library is broken.  I
            >> >> understand that it's intentionally broken to find buffer overflows.
            >> >>
            >> >> Now, we could work around the broken library.  The diff shows one
            >> >> solution.  But this would actually be needed in many more places, it
            >> >> just fixes the one found on startup.  When running Vim there are more.
            >> >> Takes a long time to find them all.
            >> >>
            >> >> A proper fix would be:
            >> >> - Add configure check for the broken library.
            >> >> - When the library is broken use another function instead of STRCPY() to
            >> >>   avoid using the broken strcpy().  It's slower but should work.
            >> >>
            >> >> A better solution would be to tell the library not to have this check
            >> >> for what it guessed the destination size would be.  Is there an option
            >> >> or #define for this?
            >> >>
            >> >
            >> >
            >> > There is such a compilation option:  -D_FORTIFY_SOURCE=0
            >> >
            >> > Patch 7.2.044 already fixed crash which happened  because of
            >> > the Fortify feature.  It was fixed by compiling Vim with
            >> > -D_FORTIFY_SOURCE=1.   Some versions of gcc compile
            >> > by default with -D_FORTIFY_SOURCE=2 (that's the case for
            >> > Ubuntu at least).  -D_FORTIFY_SOURCE=2 can in some cases
            >> > cause valid programs to crash, which is the case for Vim.
            >> > As far as I understand, -D_FORTIFY_SOURCE=1 still has
            >> > some checks but they are relaxed a bit from -D_FORTIFY_SOURCE=2.
            >> > Compiling with -D_FORTIFY_SOURCE=1 should not break
            >> > Vim.  I never see any problem with Vim using -D_FORTIFY_SOURCE=1.
            >> > But if you want to completely disable the Fortify feature, you can compile
            >> > with -D_FORTIFY_SOURCE=0  (or -U_FORTIFY_SOURCE)
            >> >
            >> > This is the Patch in which introduces -D_FORTIFY_SOURCE=1:
            >> >
            >> > ===============
            >> > Patch 7.2.044
            >> > Problem:    Crash because of STRCPY() being over protective of the destination
            >> >             size. (Dominique Pelle)
            >> > Solution:   Add -D_FORTIFY_SOURCE=1 to CFLAGS.  Use an intermediate variable
            >> >             for the pointer to avoid a warning.
            >> > Files:      src/auto/configure, src/configure.in, src/eval.c
            >> > ===============
            >> >
            >> > Is Fedora compiling Vim with -D_FORTIFY_SOURCE=1 or
            >> > -D_FORTIFY_SOURCE=2?
            >> >
            >> > In the absense of -D_FORTIFY_SOURCE compilation option,
            >> > gcc might using -D_FORTIFY_SOURCE=2 by default (as it is the
            >> > case on Ubuntu) which is known to break vim and was fixed
            >> > in patch 7.2.044.
            >> >
            >> > The configure script checks whether to add -D_FORTIFY_SOURCE=1
            >> > to the compilation options. Perhaps this is not happing on Fedora?
            >> >
            >> > Can you give the compilation log to see what compilation options
            >> > were used?
            >> >
            >> > -- Dominique
            >> >
            >>
            >>
            >> Hello,
            >>
            >> The build log is available at
            >> http://kojipkgs.fedoraproject.org/packages/vim/7.2.245/1.fc12/data/logs/i686/build.log
            >> and we were using -D_FORTIFY_SOURCE=2 to compile vim as you've already
            >> suspected.
            >>
            >> This compile option comes from the RPM compile options and seems to take
            >> precedence over your fortify changes from patch 44 as we now have
            >> '-D_FORTIFY_SOURCE=2 -D_FORTIFY_SOURCE=1' on the gcc commandline.
            >
            > Ah, so the configure script is working, but not as expected.
            >
            > Where is the -D_FORTIFY_SOURCE=2 flag coming from?  Is it in $CFLAGS?
            > If so then configure could filter it out.


            It seems so, in the build log at
            http://kojipkgs.fedoraproject.org/packages/vim/7.2.245/1.fc12/data/logs/i686/build.log,
            I see...

            === begin quote ===
            + export 'CFLAGS=-O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2
            -fexceptions -fstack-protector --param=ssp-buffer-size=4 -m32
            -march=i686 -mtune=atom -fasynchronous-unwind-tables -D_GNU_SOURCE
            -D_FILE_OFFSET_BITS=64 -D_FORTIFY_SOURCE=2'
            ...[snip]...
            gcc -O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=2 -fexceptions
            -fstack-protector --param=ssp-buffer-size=4 -m32 -march=i686
            -mtune=atom -fasynchronous-unwind-tables -D_GNU_SOURCE
            -D_FILE_OFFSET_BITS=64 -D_FORTIFY_SOURCE=2 -D_FORTIFY_SOURCE=1 -DUNIX
            -o xxd xxd.c
            === end quote ===

            Hmmm, -D_FORTIFY_SOURCE=2 is even defined twice in CFLAGS:
            * once with -Wp,-D_FORTIFY_SOURCE=2
            * once with -D_FORTIFY_SOURCE=2

            In my opinion, compiling with -D_FORTIFY_SOURCE=2 is asking
            for trouble since it is known to break valid C programs. See this link:

            http://gcc.gnu.org/ml/gcc-patches/2004-09/msg02055.html

            === begin quote ===
            The intended use in glibc is that by default no protection is
            done, when the above GCC 4.0+ and -D_FORTIFY_SOURCE=1 is used
            at optimization level 1 and above, security measures that
            shouldn't change behaviour of conforming programs are taken.
            With -D_FORTIFY_SOURCE=2 some more checking is added, but
            some conforming programs might fail.
            === end quote ===

            -D_FORTIFY_SOURCE=2 sounds useful for developers but
            compiling a release of Fedora with it is risky in my opinion.
            How many other spurious crash will it cause that will be less
            obvious to detect than this one in other programs?

            -- Dominique

            --~--~---------~--~----~------------~-------~--~----~
            You received this message from the "vim_dev" maillist.
            For more information, visit http://www.vim.org/maillist.php
            -~----------~----~----~----~------~----~------~--~---
          • Dominique Pellé
            ... The attached patch does what you suggest. I tested with: $ export CFLAGS= -Wp,-D_FORTIFY_SOURCE=2 -D_FORTIFY_SOURCE=2 $ cd vim7/src $ make distclean $
            Message 5 of 9 , Aug 4, 2009
            • 0 Attachment
              On Tue, Aug 4, 2009 at 9:13 PM, Bram Moolenaar wrote:

              >> Hello,
              >>
              >> The build log is available at
              >> http://kojipkgs.fedoraproject.org/packages/vim/7.2.245/1.fc12/data/logs/i686/build.log
              >> and we were using -D_FORTIFY_SOURCE=2 to compile vim as you've already
              >> suspected.
              >>
              >> This compile option comes from the RPM compile options and seems to take
              >> precedence over your fortify changes from patch 44 as we now have
              >> '-D_FORTIFY_SOURCE=2 -D_FORTIFY_SOURCE=1' on the gcc commandline.
              >
              > Ah, so the configure script is working, but not as expected.
              >
              > Where is the -D_FORTIFY_SOURCE=2 flag coming from?  Is it in $CFLAGS?
              > If so then configure could filter it out.


              The attached patch does what you suggest.

              I tested with:

              $ export CFLAGS="-Wp,-D_FORTIFY_SOURCE=2 -D_FORTIFY_SOURCE=2"
              $ cd vim7/src
              $ make distclean
              $ autoconf
              $ ./configure
              $ make

              ... and -D_FORTIFY_SOURCE=1 is used when compiling.

              -- Dominique

              --~--~---------~--~----~------------~-------~--~----~
              You received this message from the "vim_dev" maillist.
              For more information, visit http://www.vim.org/maillist.php
              -~----------~----~----~----~------~----~------~--~---
            • Bram Moolenaar
              ... Thanks, I ll include this. -- hundred-and-one symptoms of being an internet addict: 158. You get a tuner card so you can watch TV while surfing. /// Bram
              Message 6 of 9 , Aug 7, 2009
              • 0 Attachment
                Dominique Pelle wrote:

                > >> The build log is available at
                > >> http://kojipkgs.fedoraproject.org/packages/vim/7.2.245/1.fc12/data/logs/i686/build.log
                > >> and we were using -D_FORTIFY_SOURCE=2 to compile vim as you've already
                > >> suspected.
                > >>
                > >> This compile option comes from the RPM compile options and seems to take
                > >> precedence over your fortify changes from patch 44 as we now have
                > >> '-D_FORTIFY_SOURCE=2 -D_FORTIFY_SOURCE=1' on the gcc commandline.
                > >
                > > Ah, so the configure script is working, but not as expected.
                > >
                > > Where is the -D_FORTIFY_SOURCE=2 flag coming from? Is it in $CFLAGS?
                > > If so then configure could filter it out.
                >
                >
                > The attached patch does what you suggest.
                >
                > I tested with:
                >
                > $ export CFLAGS="-Wp,-D_FORTIFY_SOURCE=2 -D_FORTIFY_SOURCE=2"
                > $ cd vim7/src
                > $ make distclean
                > $ autoconf
                > $ ./configure
                > $ make
                >
                > ... and -D_FORTIFY_SOURCE=1 is used when compiling.

                Thanks, I'll include this.

                --
                hundred-and-one symptoms of being an internet addict:
                158. You get a tuner card so you can watch TV while surfing.

                /// Bram Moolenaar -- Bram@... -- http://www.Moolenaar.net \\\
                /// sponsor Vim, vote for features -- http://www.Vim.org/sponsor/ \\\
                \\\ download, build and distribute -- http://www.A-A-P.org ///
                \\\ help me help AIDS victims -- http://ICCF-Holland.org ///

                --~--~---------~--~----~------------~-------~--~----~
                You received this message from the "vim_dev" maillist.
                For more information, visit http://www.vim.org/maillist.php
                -~----------~----~----~----~------~----~------~--~---
              Your message has been successfully submitted and would be delivered to recipients shortly.