[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Bacula-devel] "buffer overflow detected" error on fedora distributions.


On Friday 08 February 2008 23.16:43 Josh Fisher wrote:
> Kern Sibbald wrote:
> > Please Note the following quote from the documentation on the
> > implementation of FORTIFY_SOURCE=2
> >
> >     With -D_FORTIFY_SOURCE=2 some more checking is added, but
> >     some conforming programs might fail.
> >
> > Bacula is a conforming program (i.e. there is no buffer overrun at that
> > point).
>
> I agree. The checking is too simplistic (for performance reasons I
> suppose) and so hits too many false positives. Putting
> -DFORTIFY_SOURCE=2  into the default CFLAGS was a big mistake, because
> it will break perfectly safe code, like Bacula and probably many others.
> I could see it as a compile-time error, maybe, but not a run-time error.

I can understand that they (rpm guys) would make FORTIFY_SOURCE=2 a default, 
even though like you it is not something I would do, because according to 
their own documentation (see above) it doesn't always work, but where I have 
a real problem is that they apparently don't make it simple to remove the 
FORTIFY_SOURCE.

>
> Anyway, a dirty hack to turn off FORTIFY_SOURCE on Fedora 8, without
> touching the Bacula source, is to specify the CFLAGS in the RPM spec
> file. See below patch to Scott's bacula-2.2.8-1.src.rpm spec file. It is
> a really bad idea to specify CFLAGS in the spec file, but I don't know
> any other way to do it without changing Bacula's configure script. I
> have tested on a x86_64 FC8 pv domU under Xen 3.1 Centos 5.1 dom0 on a
> dual-core Opteron.
>
> It would be nice if there were a --disable-fortify arg to the configure
> script that removed the FORTIFY_SOURCE flag from the default CFLAGS or
> set it to zero. That would make life easier for the packagers, while
> still not requiring massive changes to the Bacula source. :)
> Considering that Suse is now using FORTIFY_SOURCE=2, they  too could
> very well break Bacula when they move to glibc 2.7+. Centos might at
> some point run into this "feature" as well.

The Bacula developers set the ./configure and Makefile up to build the source 
code the way we feel is the best. I am not too enthusiastic about adding a 
new option, because It is not really my way of doing things  to "forbid" 
certain options.  IMO it is the prerogative and responsibility of the person 
building Bacula to put whatever options he/she wants.  Since there *is* a 
workaround for this gcc bug, I prefer to leave it as is and spend time on 
adding new features.  However, were a patch submitted, I doubt it would be 
rejected :-)

By the way, I do know how to fix the problem (proper casting), but just have 
not found the time to get back and modify the code.  In the process, I should 
be able to remove what I consider an ugly kludge added to make the Win32 
compiler work.

Best regards,

Kern

>
> Please note that the patch also removes the cp of static-bacula-fd for
> fc8. This is because the static bacula-fd will not compile on Fedora 8
> with glibc 2.7-2, apparently due to a glibc bug. At least, I couldn't
> get it to compile.
>
> --- BEGIN_PATCH---------------
> --- bacula.spec 2008-01-27 09:39:18.000000000 -0500
> +++ new/bacula.spec     2008-02-08 15:43:54.000000000 -0500
> @@ -1203,6 +1203,14 @@
>  export LDFLAGS="${LDFLAGS} -L/usr/lib/termcap"
>  %endif
>
> +%if %{fc8} && %{x86_64}
> +export CFLAGS="-O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=0 -fexceptions
> -fstack-protector --param=ssp-buffer-size=4 -m64 -mtune=generic
> -fno-strict-aliasing -fno-exceptions -fno-rtti"
> +%endif
> +
> +%if %{fc8} && ! %{x86_64}
> +export CFLAGS="-O2 -g -pipe -Wall -Wp,-D_FORTIFY_SOURCE=0 -fexceptions
> -fstack-protector --param=ssp-buffer-size=4 -march=i386 -mtune=generic
> -fno-strict-aliasing -fno-exceptions -fno-rtti"
> +%endif
> +
>  cwd=${PWD}
>  cd %{depkgs}
>  %if %{sqlite}
> @@ -1584,7 +1592,9 @@
>  cp -pr %{_rescuesrc}/autoconf $RPM_BUILD_ROOT%{sysconf_dir}/rescue/
>  cp -pr %{_rescuesrc}/knoppix $RPM_BUILD_ROOT%{sysconf_dir}/rescue/
>  touch $RPM_BUILD_ROOT%{sysconf_dir}/rescue/linux/cdrom/rpm_release
> +%if ! %{fc8}
>  cp -p src/filed/static-bacula-fd
> $RPM_BUILD_ROOT%{sysconf_dir}/rescue/linux/cdrom/bacula/bin/bacula-fd
> +%endif
>  rm -f src/filed/static-bacula-fd
>
>  # install bat since make doesn't at the moment
> --- END_PATCH-----------------
>
> > Best regards,
> >
> > Kern
> >
> > On Tuesday 29 January 2008 00.02:45 Michael Lausch wrote:
> >> The error is due to the new (well ~ core 5) buffer overflow checking
> >> implemented by gcc and glibc. _FORTIFY_SOURCE=2  activates it. what
> >> happens can be read in detail at
> >> http://gcc.gnu.org/ml/gcc-patches/2004-09/msg02055.html. but basically
> >> the error is a buffer overflow check in parse.c in the bacula library.
> >>
> >> In this file the following definition can be found:
> >> extern  CURES res_all;
> >> CURES is a type defined in the library with a size of, let's say 120
> >> bytes. the actual value is not important.
> >>
> >> In the bat module for example, the res_all variable is redefined as
> >> URES res_all;
> >> in bat_conf.cpp. URES is a type with, let's say, 250 bytes. The actual
> >> value is not important as long as it's larger then the size of the URES
> >> type defined in the library. The variable res_all_size is initialized to
> >> the size of the res_all variable, in my example to 250.
> >>
> >> In the init_resource() function in parse_conf.c is a call to
> >> memset(&res_all, 0, res_all_size);
> >> This call is replaced to a boundary checking memset() call as outlined
> >> by the example 2 in
> >> http://gcc.gnu.org/ml/gcc-patches/2004-09/msg02055.html
> >> The 3rd parameter of the memset call, res_all_size, which is 250, is
> >> checked against the size of the CURES type (120) and the buffer overflow
> >> error is raised by the boundary check of the memset function.
> >>
> >>
> >> The solution is to allocate the res_all variable dynamically.
> >>
> >> My quick hack solution was to change the definition of CURES to
> >> union CURES {
> >>    MSGS  res_msgs;
> >>    RES hdr;
> >>    char _space_[1024];
> >> };
> >> This makes the size of the CURES union larger than all the other unions
> >> defined in the different bacula executables and the memset check
> >> succeeds. But as i said this is a hack and i used it only as a band aid
> >> to get a runnable system.
> >>
> >> The solution to disable boundary checking by using a D_FORTIFY_SOURCE=0
> >> definition in the compiler command line should not be done, because
> >> checking for errors in such a sensible application as a backup utility
> >> is always a good thing.
> >
> > -------------------------------------------------------------------------
> > This SF.net email is sponsored by: Microsoft
> > Defy all challenges. Microsoft(R) Visual Studio 2008.
> > http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
> > _______________________________________________
> > Bacula-devel mailing list
> > Bacula-devel@xxxxxxxxxxxxxxxxxxxxx
> > https://lists.sourceforge.net/lists/listinfo/bacula-devel
>
> -------------------------------------------------------------------------
> This SF.net email is sponsored by: Microsoft
> Defy all challenges. Microsoft(R) Visual Studio 2008.
> http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
> _______________________________________________
> Bacula-devel mailing list
> Bacula-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.sourceforge.net/lists/listinfo/bacula-devel



-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
Bacula-devel mailing list
Bacula-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.sourceforge.net/lists/listinfo/bacula-devel


This mailing list archive is a service of Copilotco.