On Sat, 2008-02-09 at 08:54 +0100, Kern Sibbald wrote:
> 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.
Wouldn't the following work, without modifying any other settings?
export CFLAGS="$CFLAGS -D_FORTIFY_SOURCE=0"
The string would then first set it to 2 and last reset to zero. And it
works for all platforms.
>
> >
> > 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@lists.sourceforge.net
> > > 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@lists.sourceforge.net
> > 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@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bacula-devel
