Message ID | 20180711143113.11927-4-matthew.weber@rockwellcollins.com |
---|---|
State | Changes Requested |
Headers | show |
Series | Hardening Flag Bugfix/Enhancement | expand |
On 11-07-18 16:31, Matt Weber wrote: > From: Stefan Sørensen <stefan.sorensen@spectralink.com> > > The PIE build flags are only intended for building executables and can not be > used in relocateable links (-r), static builds and shared library build - > including the flags here causes build errors. > > So instead of parsing the PIE flags directly on the command line to gcc, > include them in a gcc spec file where it is possible to only apply the flags > when other incompatible flags are not set. The commit message should also have the reasoning why this is considered better than doing the same in the wrapper. > > This method and the spec files are from the Fedora build system. > > Originally submitted as > http://patchwork.ozlabs.org/patch/907093/ > > Signed-off-by: Stefan Sørensen <stefan.sorensen@spectralink.com> > Signed-off-by: Matt Weber <matthew.weber@rockwellcollins.com> > --- > package/Makefile.in | 4 ++-- > toolchain/gcc-specs-pie-cc1 | 2 ++ > toolchain/gcc-specs-pie-ld | 2 ++ > 3 files changed, 6 insertions(+), 2 deletions(-) > create mode 100644 toolchain/gcc-specs-pie-cc1 > create mode 100644 toolchain/gcc-specs-pie-ld > > diff --git a/package/Makefile.in b/package/Makefile.in > index 14b3bbd243..00ddf48c10 100644 > --- a/package/Makefile.in > +++ b/package/Makefile.in > @@ -158,8 +158,8 @@ ifeq ($(BR2_RELRO_PARTIAL),y) > TARGET_HARDENED += $(TARGET_CFLAGS_RELRO) > TARGET_LDFLAGS += $(TARGET_CFLAGS_RELRO) > else ifeq ($(BR2_RELRO_FULL),y) > -TARGET_HARDENED += -fPIE $(TARGET_CFLAGS_RELRO_FULL) > -TARGET_LDFLAGS += -pie $(TARGET_CFLAGS_RELRO_FULL) > +TARGET_HARDENED += $(TARGET_CFLAGS_RELRO_FULL) -specs=$(TOPDIR)/toolchain/gcc-specs-pie-cc1 > +TARGET_LDFLAGS += $(TARGET_CFLAGS_RELRO_FULL) -specs=$(TOPDIR)/toolchain/gcc-specs-pie-ld As mentioned in my other mail, it is very likely that there are packages that link with TARGET_CFLAGS instead of TARGET_LDFLAGS. Would that be a problem? Actually, can't we just have a single spec file that contains both? Regards, Arnout > endif > > ifeq ($(BR2_FORTIFY_SOURCE_1),y) > diff --git a/toolchain/gcc-specs-pie-cc1 b/toolchain/gcc-specs-pie-cc1 > new file mode 100644 > index 0000000000..fc54bcb510 > --- /dev/null > +++ b/toolchain/gcc-specs-pie-cc1 > @@ -0,0 +1,2 @@ > +*cc1_options: > ++ %{!r:%{!fpie:%{!fPIE:%{!fpic:%{!fPIC:%{!fno-pic:-fPIE}}}}}} > diff --git a/toolchain/gcc-specs-pie-ld b/toolchain/gcc-specs-pie-ld > new file mode 100644 > index 0000000000..bd6b9071ff > --- /dev/null > +++ b/toolchain/gcc-specs-pie-ld > @@ -0,0 +1,2 @@ > +*self_spec: > ++ %{!static:%{!shared:%{!r:-pie}}}
Arnout, On Wed, Jul 11, 2018 at 4:44 PM, Arnout Vandecappelle <arnout@mind.be> wrote: > > > On 11-07-18 16:31, Matt Weber wrote: >> From: Stefan Sørensen <stefan.sorensen@spectralink.com> >> >> The PIE build flags are only intended for building executables and can not be >> used in relocateable links (-r), static builds and shared library build - >> including the flags here causes build errors. >> >> So instead of parsing the PIE flags directly on the command line to gcc, >> include them in a gcc spec file where it is possible to only apply the flags >> when other incompatible flags are not set. > > The commit message should also have the reasoning why this is considered better > than doing the same in the wrapper. > It probably isn't better then doing it in the wrapper. At this point, its just fixing something that's broken with the current approach. I was assuming a solid baseline option with this feature that's working first and then start looking at replacing the approach with the suggestion awhile back with doing it via the wrapper. >> -TARGET_HARDENED += -fPIE $(TARGET_CFLAGS_RELRO_FULL) >> -TARGET_LDFLAGS += -pie $(TARGET_CFLAGS_RELRO_FULL) >> +TARGET_HARDENED += $(TARGET_CFLAGS_RELRO_FULL) -specs=$(TOPDIR)/toolchain/gcc-specs-pie-cc1 >> +TARGET_LDFLAGS += $(TARGET_CFLAGS_RELRO_FULL) -specs=$(TOPDIR)/toolchain/gcc-specs-pie-ld > > As mentioned in my other mail, it is very likely that there are packages that > link with TARGET_CFLAGS instead of TARGET_LDFLAGS. Would that be a problem? > Actually, can't we just have a single spec file that contains both? Like you mentioned previously, flag usage isn't consistent. When we started doing validation of the elf files we found that inconsistency and we were individually trying to fix the packages. This didn't get to far as lots of the packages are in stable, no real maintaining occuring. I'm glad Stefan brought up the spec file idea as that solved all the rework on the linker ordering and combination of values. Related to using a single spec file. I'm not to familiar with using them but since we have a mix of good/bad flag options already occurring, guessing it wouldn't matter to consolidate to one file. I can test going to a single file. Stefan, do you know? Matt
On 12-07-18 01:17, Matthew Weber wrote: > Arnout, > > On Wed, Jul 11, 2018 at 4:44 PM, Arnout Vandecappelle <arnout@mind.be> wrote: >> >> >> On 11-07-18 16:31, Matt Weber wrote: >>> From: Stefan Sørensen <stefan.sorensen@spectralink.com> >>> >>> The PIE build flags are only intended for building executables and can not be >>> used in relocateable links (-r), static builds and shared library build - >>> including the flags here causes build errors. >>> >>> So instead of parsing the PIE flags directly on the command line to gcc, >>> include them in a gcc spec file where it is possible to only apply the flags >>> when other incompatible flags are not set. >> >> The commit message should also have the reasoning why this is considered better >> than doing the same in the wrapper. >> > > It probably isn't better then doing it in the wrapper. At this point, > its just fixing something that's broken with the current approach. I > was assuming a solid baseline option with this feature that's working > first and then start looking at replacing the approach with the > suggestion awhile back with doing it via the wrapper. I'm a bit confused about where you'd like to go in the long run: spec file or wrapper? In your reply to the cover letter you mention "I do like the concept of changing to the wrapper and evaluating if we can use GCC spec files"... I guess the spec files have the advantage that it is a lot easier to specify mutually exclusive options. We have that now in the wrapper for arch/cpu/tune, but it's a lot of code to specify something simple. That said, spec files are so arcane that you can't expect many people contributing to it. The C code, though more verbose, is a lot more understandable. Back to the patch: basically, the same could be done in the wrapper, but here the spec files are just taken from Fedora so that's a lot simpler. Right? >>> -TARGET_HARDENED += -fPIE $(TARGET_CFLAGS_RELRO_FULL) >>> -TARGET_LDFLAGS += -pie $(TARGET_CFLAGS_RELRO_FULL) >>> +TARGET_HARDENED += $(TARGET_CFLAGS_RELRO_FULL) -specs=$(TOPDIR)/toolchain/gcc-specs-pie-cc1 >>> +TARGET_LDFLAGS += $(TARGET_CFLAGS_RELRO_FULL) -specs=$(TOPDIR)/toolchain/gcc-specs-pie-ld >> >> As mentioned in my other mail, it is very likely that there are packages that >> link with TARGET_CFLAGS instead of TARGET_LDFLAGS. Would that be a problem? I don't think you answered this question? >> Actually, can't we just have a single spec file that contains both? > > Like you mentioned previously, flag usage isn't consistent. When we > started doing validation of the elf files we found that inconsistency > and we were individually trying to fix the packages. This didn't get > to far as lots of the packages are in stable, no real maintaining > occuring. I'm glad Stefan brought up the spec file idea as that > solved all the rework on the linker ordering and combination of > values. Yes, spec file (or wrapper) is definitely better than patching individual packages. Regards, Arnout > Related to using a single spec file. I'm not to familiar with using > them but since we have a mix of good/bad flag options already > occurring, guessing it wouldn't matter to consolidate to one file. I > can test going to a single file. Stefan, do you know? > > Matt >
Arnout, On Fri, Jul 13, 2018 at 4:39 AM, Arnout Vandecappelle <arnout@mind.be> wrote: > > > On 12-07-18 01:17, Matthew Weber wrote: >> Arnout, >> >> On Wed, Jul 11, 2018 at 4:44 PM, Arnout Vandecappelle <arnout@mind.be> wrote: >>> >>> >>> On 11-07-18 16:31, Matt Weber wrote: >>>> From: Stefan Sørensen <stefan.sorensen@spectralink.com> >>>> >>>> The PIE build flags are only intended for building executables and can not be >>>> used in relocateable links (-r), static builds and shared library build - >>>> including the flags here causes build errors. >>>> >>>> So instead of parsing the PIE flags directly on the command line to gcc, >>>> include them in a gcc spec file where it is possible to only apply the flags >>>> when other incompatible flags are not set. >>> >>> The commit message should also have the reasoning why this is considered better >>> than doing the same in the wrapper. >>> >> >> It probably isn't better then doing it in the wrapper. At this point, >> its just fixing something that's broken with the current approach. I >> was assuming a solid baseline option with this feature that's working >> first and then start looking at replacing the approach with the >> suggestion awhile back with doing it via the wrapper. > > I'm a bit confused about where you'd like to go in the long run: spec file or > wrapper? In your reply to the cover letter you mention "I do like the concept of > changing to the wrapper and evaluating if we can use GCC spec files"... Sorry, I was pulling a few thoughts together and didn't explain it clearly. I was thinking back to previous threads were it was discussed to take all the hardening options and use the wrapper to manage them vs directly setting the flag variables. Ideally, I agree that a wrapper approach for that is more ideal. Since that thread, there has been discussion on Stefan's patches about using spec files as an option to simplify what the wrapper currently does. However, I'd agree that the spec syntax is complex. For this patchset, the spec file is accompishing a specific task of the mutually exclusive options just related to hardening. So long term, I could see the wrapper handing the hardening flag management directly and the seperate possibility of spec files possibly being used for other unrelated items that might simplify wrapper logic. > > I guess the spec files have the advantage that it is a lot easier to specify > mutually exclusive options. We have that now in the wrapper for arch/cpu/tune, > but it's a lot of code to specify something simple. That said, spec files are so > arcane that you can't expect many people contributing to it. The C code, though > more verbose, is a lot more understandable. Agree. > > Back to the patch: basically, the same could be done in the wrapper, but here > the spec files are just taken from Fedora so that's a lot simpler. Right? Correct. > > >>>> -TARGET_HARDENED += -fPIE $(TARGET_CFLAGS_RELRO_FULL) >>>> -TARGET_LDFLAGS += -pie $(TARGET_CFLAGS_RELRO_FULL) >>>> +TARGET_HARDENED += $(TARGET_CFLAGS_RELRO_FULL) -specs=$(TOPDIR)/toolchain/gcc-specs-pie-cc1 >>>> +TARGET_LDFLAGS += $(TARGET_CFLAGS_RELRO_FULL) -specs=$(TOPDIR)/toolchain/gcc-specs-pie-ld >>> >>> As mentioned in my other mail, it is very likely that there are packages that >>> link with TARGET_CFLAGS instead of TARGET_LDFLAGS. Would that be a problem? > > I don't think you answered this question? Sorry lumped this in with the one below. We found that flag usage was inconsistent across packages and we were required to over-specify flags. So some packages that link correctly may get linker oriented flags through both cflags and ldflags. We verified that syntax wise that this will work and that the post build elf file testing reflects they take affect. > >>> Actually, can't we just have a single spec file that contains both? >> >> Like you mentioned previously, flag usage isn't consistent. When we >> started doing validation of the elf files we found that inconsistency >> and we were individually trying to fix the packages. This didn't get >> to far as lots of the packages are in stable, no real maintaining >> occuring. I'm glad Stefan brought up the spec file idea as that >> solved all the rework on the linker ordering and combination of >> values. > > Yes, spec file (or wrapper) is definitely better than patching individual packages. > > >> Related to using a single spec file. I'm not to familiar with using >> them but since we have a mix of good/bad flag options already >> occurring, guessing it wouldn't matter to consolidate to one file. I >> can test going to a single file. Stefan, do you know? I've verified you have to have individual files for each. Guessing there isn't shared syntax between cc/ld spec files based on the errors I got when the compiler was invoked with the additional self_spec entry in it's spec file. I couldn't fine enough documentation to understand what that entry is doing..... Matt
On Wed, 2018-07-11 at 18:17 -0500, Matthew Weber wrote: > Related to using a single spec file. I'm not to familiar with using > them but since we have a mix of good/bad flag options already > occurring, guessing it wouldn't matter to consolidate to one file. I > can test going to a single file. Stefan, do you know? It works fine with a single spec file - I used two different files since I was keeping CFLAGS and LDFLAGS separate. I will post an updated series with a single spec file and a few other changes. Stefan
Stefan, On Thu, Jul 19, 2018 at 4:49 AM, Sørensen, Stefan <Stefan.Sorensen@spectralink.com> wrote: > > On Wed, 2018-07-11 at 18:17 -0500, Matthew Weber wrote: > > Related to using a single spec file. I'm not to familiar with using > > them but since we have a mix of good/bad flag options already > > occurring, guessing it wouldn't matter to consolidate to one file. I > > can test going to a single file. Stefan, do you know? > > It works fine with a single spec file - I used two different files > since I was keeping CFLAGS and LDFLAGS separate. > Something in the syntax then would have to change, right? I combined the files and was getting CC errors.
On Thu, 2018-07-19 at 07:58 -0500, Matthew Weber wrote: > > It works fine with a single spec file - I used two different files > > since I was keeping CFLAGS and LDFLAGS separate. > > > > Something in the syntax then would have to change, right? I combined > the files and was getting CC errors. That was what I did - I have not run into any problems. Any particular packages that fails? Stefan
On Thu, Jul 19, 2018 at 8:10 AM Sørensen, Stefan <Stefan.Sorensen@spectralink.com> wrote: > > On Thu, 2018-07-19 at 07:58 -0500, Matthew Weber wrote: > > > It works fine with a single spec file - I used two different files > > > since I was keeping CFLAGS and LDFLAGS separate. > > > > > > > Something in the syntax then would have to change, right? I combined > > the files and was getting CC errors. > > That was what I did - I have not run into any problems. Any particular > packages that fails? attr pkg during the configuring stage https://paste.ubuntu.com/p/wdn3Pjsd39/ I can work up a simple defconfig but I didn't get to far before I ran into this. The combined specs file looked like. *cc1_options: + %{!r:%{!fpie:%{!fPIE:%{!fpic:%{!fPIC:%{!fno-pic:-fPIE}}}}}} *self_spec: + %{!static:%{!shared:%{!r:-pie}}} Matt
Stefan, On Tue, Aug 7, 2018 at 12:02 PM Matthew Weber <matthew.weber@rockwellcollins.com> wrote: > > On Thu, Jul 19, 2018 at 8:10 AM Sørensen, Stefan > <Stefan.Sorensen@spectralink.com> wrote: > > > > On Thu, 2018-07-19 at 07:58 -0500, Matthew Weber wrote: > > > > It works fine with a single spec file - I used two different files > > > > since I was keeping CFLAGS and LDFLAGS separate. > > > > > > > > > > Something in the syntax then would have to change, right? I combined > > > the files and was getting CC errors. > > > > That was what I did - I have not run into any problems. Any particular > > packages that fails? > > attr pkg during the configuring stage > https://paste.ubuntu.com/p/wdn3Pjsd39/ > > I can work up a simple defconfig but I didn't get to far before I ran into this. > > The combined specs file looked like. > *cc1_options: > + %{!r:%{!fpie:%{!fPIE:%{!fpic:%{!fPIC:%{!fno-pic:-fPIE}}}}}} > *self_spec: > + %{!static:%{!shared:%{!r:-pie}}} > Config to reproduce. BR2_aarch64=y BR2_SSP_STRONG=y BR2_RELRO_FULL=y BR2_FORTIFY_SOURCE_2=y BR2_TOOLCHAIN_EXTERNAL=y BR2_TARGET_GENERIC_GETTY_PORT="ttyAMA0" BR2_SYSTEM_DHCP="eth0" BR2_LINUX_KERNEL=y BR2_LINUX_KERNEL_CUSTOM_VERSION=y BR2_LINUX_KERNEL_CUSTOM_VERSION_VALUE="4.13.6" BR2_LINUX_KERNEL_USE_CUSTOM_CONFIG=y BR2_LINUX_KERNEL_CUSTOM_CONFIG_FILE="board/qemu/aarch64-virt/linux-4.13.config" BR2_PACKAGE_ATTR=y BR2_TARGET_ROOTFS_EXT2=y BR2_TARGET_ROOTFS_EXT2_4=y # BR2_TARGET_ROOTFS_TAR is not set
Hi Matt, thanks for working on this. When I apply this patch, I'm getting a build failure from an out-of-tree [1] package, see the attached log. The package lives at [1], the Buildroot recipe is pretty straightforward [2]. Here's the relevant snippet from the failure: /home/jkt/work/prog/_build/br-cfb/host/bin/arm-linux-gnueabihf-gcc -Wall -Wl,-z -Wl,now -Wl,-z -Wl,relro -specs=/home/jkt/work/cesnet/gerrit/github/buildroot/buildroot/toolchain/gcc-specs-pie-ld -o .libs/example example.o ./.libs/libredblack.so /home/jkt/work/prog/_build/br-cfb/host/opt/ext-toolchain/bin/../lib/gcc/arm-linux-gnueabihf/7.3.1/../../../../arm-linux-gnueabihf/bin/ld: example.o: relocation R_ARM_MOVW_ABS_NC against `stderr' can not be used when making a shared object; recompile with -fPIC Is that a failure of that particular package, or a problem in the compiler specs? Also, Gentoo Linux has been doing compiler hardening for years and their specs appear to be more complex according to the docs [4]. I have no clue if that extra complexity is needed in Buildroot. With kind regards, Jan [1] I'll submit them once my patches reach a release upstream. [2] https://github.com/sysrepo/libredblack [3] https://gerrit.cesnet.cz/plugins/gitiles/github/buildroot/buildroot/+/a0cff08df5918e1721723400da42c8c68b8c0928%5E%21/ [4] https://wiki.gentoo.org/wiki/Hardened/Toolchain $ make -j1 libredblack umask 0022 && make -C /home/jkt/work/cesnet/gerrit/github/buildroot/buildroot O=/home/jkt/work/prog/_build/br-cfb/. libredblack PATH="/home/jkt/work/prog/_build/br-cfb/host/bin:/home/jkt/work/prog/_build/br-cfb/host/sbin:/home/jkt/.local/bin:/home/jkt/bin/:/opt/qtc/bin:/opt/darktable/bin:/home/jkt/.local/bin:/home/jkt/bin/:/opt/qtc/bin:/opt/darktable/bin:/usr/x86_64-pc-linux-gnu/gcc-bin/8.1.0:/usr/lib/llvm/6/bin:/usr/lib/llvm/4/bin:/usr/local/bin:/usr/bin:/bin:/opt/bin:/usr/games/bin" BR2_DL_DIR="/home/jkt/work/cesnet/gerrit/github/buildroot/buildroot/dl" BUILD_DIR=/home/jkt/work/prog/_build/br-cfb/build O=/home/jkt/work/prog/_build/br-cfb flock /home/jkt/work/cesnet/gerrit/github/buildroot/buildroot/dl/libredblack/ support/download/dl-wrapper -c '1.3' -d '/home/jkt/work/cesnet/gerrit/github/buildroot/buildroot/dl/libredblack' -D '/home/jkt/work/cesnet/gerrit/github/buildroot/buildroot/dl' -f 'libredblack-1.3.tar.gz' -H 'package/libredblack//libredblack.hash' -n 'libredblack-1.3' -N 'libredblack' -o '/home/jkt/work/cesnet/gerrit/github/buildroot/buildroot/dl/libredblack/libredblack-1.3.tar.gz' -u http+http://downloads.sourceforge.net/project/libredblack/libredblack/1.3 -u http\|urlencode+http://sources.buildroot.net/libredblack -u http\|urlencode+http://sources.buildroot.net -- libredblack-1.3.tar.gz: OK (sha256: a0ecc59b0aae2df01558a6950532c711a782a099277b439a51d270003092f44f) >>> libredblack 1.3 Extracting gzip -d -c /home/jkt/work/cesnet/gerrit/github/buildroot/buildroot/dl/libredblack/libredblack-1.3.tar.gz | tar --strip-components=1 -C /home/jkt/work/prog/_build/br-cfb/build/libredblack-1.3 -xf - >>> libredblack 1.3 Patching >>> libredblack 1.3 Updating config.sub and config.guess for file in config.guess config.sub; do for i in $(find /home/jkt/work/prog/_build/br-cfb/build/libredblack-1.3 -name $file); do cp support/gnuconfig/$file $i; done; done >>> libredblack 1.3 Patching libtool >>> libredblack 1.3 Configuring (cd /home/jkt/work/prog/_build/br-cfb/build/libredblack-1.3/ && rm -rf config.cache && PATH="/home/jkt/work/prog/_build/br-cfb/host/bin:/home/jkt/work/prog/_build/br-cfb/host/sbin:/home/jkt/.local/bin:/home/jkt/bin/:/opt/qtc/bin:/opt/darktable/bin:/home/jkt/.local/bin:/home/jkt/bin/:/opt/qtc/bin:/opt/darktable/bin:/usr/x86_64-pc-linux-gnu/gcc-bin/8.1.0:/usr/lib/llvm/6/bin:/usr/lib/llvm/4/bin:/usr/local/bin:/usr/bin:/bin:/opt/bin:/usr/games/bin" AR="/home/jkt/work/prog/_build/br-cfb/host/bin/arm-linux-gnueabihf-ar" AS="/home/jkt/work/prog/_build/br-cfb/host/bin/arm-linux-gnueabihf-as" LD="/home/jkt/work/prog/_build/br-cfb/host/bin/arm-linux-gnueabihf-ld" NM="/home/jkt/work/prog/_build/br-cfb/host/bin/arm-linux-gnueabihf-nm" CC="/home/jkt/work/prog/_build/br-cfb/host/bin/arm-linux-gnueabihf-gcc" GCC="/home/jkt/work/prog/_build/br-cfb/host/bin/arm-linux-gnueabihf-gcc" CPP="/home/jkt/work/prog/_build/br-cfb/host/bin/arm-linux-gnueabihf-cpp" CXX="/home/jkt/work/prog/_build/br-cfb/host/bin/arm-linux-gnueabihf-g++" FC="/home/jkt/work/prog/_build/br-cfb/host/bin/arm-linux-gnueabihf-gfortran" F77="/home/jkt/work/prog/_build/br-cfb/host/bin/arm-linux-gnueabihf-gfortran" RANLIB="/home/jkt/work/prog/_build/br-cfb/host/bin/arm-linux-gnueabihf-ranlib" READELF="/home/jkt/work/prog/_build/br-cfb/host/bin/arm-linux-gnueabihf-readelf" STRIP="/home/jkt/work/prog/_build/br-cfb/host/bin/arm-linux-gnueabihf-strip" OBJCOPY="/home/jkt/work/prog/_build/br-cfb/host/bin/arm-linux-gnueabihf-objcopy" OBJDUMP="/home/jkt/work/prog/_build/br-cfb/host/bin/arm-linux-gnueabihf-objdump" AR_FOR_BUILD="/usr/bin/ar" AS_FOR_BUILD="/usr/bin/as" CC_FOR_BUILD="/usr/x86_64-pc-linux-gnu/gcc-bin/8.1.0/gcc" GCC_FOR_BUILD="/usr/x86_64-pc-linux-gnu/gcc-bin/8.1.0/gcc" CXX_FOR_BUILD="/usr/x86_64-pc-linux-gnu/gcc-bin/8.1.0/g++" LD_FOR_BUILD="/usr/bin/ld" CPPFLAGS_FOR_BUILD="-I/home/jkt/work/prog/_build/br-cfb/host/include" CFLAGS_FOR_BUILD="-O2 -I/home/jkt/work/prog/_build/br-cfb/host/include" CXXFLAGS_FOR_BUILD="-O2 -I/home/jkt/work/prog/_build/br-cfb/host/include" LDFLAGS_FOR_BUILD="-L/home/jkt/work/prog/_build/br-cfb/host/lib -Wl,-rpath,/home/jkt/work/prog/_build/br-cfb/host/lib" FCFLAGS_FOR_BUILD="" DEFAULT_ASSEMBLER="/home/jkt/work/prog/_build/br-cfb/host/bin/arm-linux-gnueabihf-as" DEFAULT_LINKER="/home/jkt/work/prog/_build/br-cfb/host/bin/arm-linux-gnueabihf-ld" CPPFLAGS="-D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64" CFLAGS="-D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -Os -g2 -fstack-protector-strong -Wl,-z,now -Wl,-z,relro -specs=/home/jkt/work/cesnet/gerrit/github/buildroot/buildroot/toolchain/gcc-specs-pie-cc1 -D_FORTIFY_SOURCE=2" CXXFLAGS="-D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -Os -g2 -fstack-protector-strong -Wl,-z,now -Wl,-z,relro -specs=/home/jkt/work/cesnet/gerrit/github/buildroot/buildroot/toolchain/gcc-specs-pie-cc1 -D_FORTIFY_SOURCE=2" LDFLAGS=" -Wl,-z,now -Wl,-z,relro -specs=/home/jkt/work/cesnet/gerrit/github/buildroot/buildroot/toolchain/gcc-specs-pie-ld" FCFLAGS=" -Os -g2" FFLAGS=" -Os -g2" PKG_CONFIG="/home/jkt/work/prog/_build/br-cfb/host/bin/pkg-config" STAGING_DIR="/home/jkt/work/prog/_build/br-cfb/host/arm-buildroot-linux-gnueabihf/sysroot" INTLTOOL_PERL=/usr/bin/perl ac_cv_lbl_unaligned_fail=yes ac_cv_func_mmap_fixed_mapped=yes ac_cv_func_memcmp_working=yes ac_cv_have_decl_malloc=yes gl_cv_func_malloc_0_nonnull=yes ac_cv_func_malloc_0_nonnull=yes ac_cv_func_calloc_0_nonnull=yes ac_cv_func_realloc_0_nonnull=yes lt_cv_sys_lib_search_path_spec="" ac_cv_c_bigendian=no CONFIG_SITE=/dev/null ./configure --target=arm-buildroot-linux-gnueabihf --host=arm-buildroot-linux-gnueabihf --build=x86_64-pc-linux-gnu --prefix=/usr --exec-prefix=/usr --sysconfdir=/etc --localstatedir=/var --program-prefix="" --disable-gtk-doc --disable-gtk-doc-html --disable-doc --disable-docs --disable-documentation --with-xmlto=no --with-fop=no --disable-dependency-tracking --enable-ipv6 --disable-nls --disable-static --enable-shared --with-rbgen=no ) configure: loading site script /dev/null checking for a BSD-compatible install... /usr/bin/install -c checking whether build environment is sane... yes checking for gawk... gawk checking whether make sets $(MAKE)... yes checking for arm-buildroot-linux-gnueabihf-strip... /home/jkt/work/prog/_build/br-cfb/host/bin/arm-linux-gnueabihf-strip checking for arm-buildroot-linux-gnueabihf-gcc... /home/jkt/work/prog/_build/br-cfb/host/bin/arm-linux-gnueabihf-gcc checking for C compiler default output... a.out checking whether the C compiler works... yes checking whether we are cross compiling... yes checking for suffix of executables... checking for suffix of object files... o checking whether we are using the GNU C compiler... yes checking whether /home/jkt/work/prog/_build/br-cfb/host/bin/arm-linux-gnueabihf-gcc accepts -g... yes checking for /home/jkt/work/prog/_build/br-cfb/host/bin/arm-linux-gnueabihf-gcc option to accept ANSI C... none needed checking for style of include used by make... GNU checking dependency style of /home/jkt/work/prog/_build/br-cfb/host/bin/arm-linux-gnueabihf-gcc... none checking for a BSD-compatible install... /usr/bin/install -c checking build system type... x86_64-pc-linux-gnu checking host system type... arm-buildroot-linux-gnueabihf checking for ld used by GCC... /home/jkt/work/prog/_build/br-cfb/host/bin/arm-linux-gnueabihf-ld checking if the linker (/home/jkt/work/prog/_build/br-cfb/host/bin/arm-linux-gnueabihf-ld) is GNU ld... yes checking for /home/jkt/work/prog/_build/br-cfb/host/bin/arm-linux-gnueabihf-ld option to reload object files... -r checking for BSD-compatible nm... /home/jkt/work/prog/_build/br-cfb/host/bin/arm-linux-gnueabihf-nm checking for a sed that does not truncate output... /bin/sed checking whether ln -s works... yes checking how to recognise dependent libraries... file_magic ELF [0-9][0-9]*-bit [LM]SB (shared object|dynamic lib ) checking command to parse /home/jkt/work/prog/_build/br-cfb/host/bin/arm-linux-gnueabihf-nm output... ok checking how to run the C preprocessor... /home/jkt/work/prog/_build/br-cfb/host/bin/arm-linux-gnueabihf-cpp checking for egrep... grep -E checking for ANSI C header files... yes checking for sys/types.h... yes checking for sys/stat.h... yes checking for stdlib.h... yes checking for string.h... yes checking for memory.h... yes checking for strings.h... yes checking for inttypes.h... yes checking for stdint.h... yes checking for unistd.h... yes checking dlfcn.h usability... yes checking dlfcn.h presence... yes checking for dlfcn.h... yes checking for arm-buildroot-linux-gnueabihf-file... no checking for file... /usr/bin/file checking for arm-buildroot-linux-gnueabihf-ranlib... /home/jkt/work/prog/_build/br-cfb/host/bin/arm-linux-gnueabihf-ranlib checking for arm-buildroot-linux-gnueabihf-strip... (cached) /home/jkt/work/prog/_build/br-cfb/host/bin/arm-linux-gnueabihf-strip checking for objdir... .libs checking for /home/jkt/work/prog/_build/br-cfb/host/bin/arm-linux-gnueabihf-gcc option to produce PIC... -fPIC checking if /home/jkt/work/prog/_build/br-cfb/host/bin/arm-linux-gnueabihf-gcc PIC flag -fPIC works... yes checking if /home/jkt/work/prog/_build/br-cfb/host/bin/arm-linux-gnueabihf-gcc static flag -static works... yes checking if /home/jkt/work/prog/_build/br-cfb/host/bin/arm-linux-gnueabihf-gcc supports -c -o file.o... yes checking if /home/jkt/work/prog/_build/br-cfb/host/bin/arm-linux-gnueabihf-gcc supports -c -o file.lo... yes checking if /home/jkt/work/prog/_build/br-cfb/host/bin/arm-linux-gnueabihf-gcc supports -fno-rtti -fno-exceptions... yes checking whether the linker (/home/jkt/work/prog/_build/br-cfb/host/bin/arm-linux-gnueabihf-ld) supports shared libraries... yes checking how to hardcode library paths into programs... immediate checking whether stripping libraries is possible... yes checking dynamic linker characteristics... GNU/Linux ld.so checking if libtool supports shared libraries... yes checking whether to build shared libraries... yes checking whether to build static libraries... no checking whether -lc should be explicitly linked in... no creating libtool checking for ANSI C header files... (cached) yes checking for an ANSI C-conforming const... yes checking for strdup... yes configure: creating ./config.status config.status: creating Makefile config.status: creating libredblack.spec config.status: creating rbgen config.status: creating config.h config.status: executing depfiles commands config.status: executing default commands >>> libredblack 1.3 Building PATH="/home/jkt/work/prog/_build/br-cfb/host/bin:/home/jkt/work/prog/_build/br-cfb/host/sbin:/home/jkt/.local/bin:/home/jkt/bin/:/opt/qtc/bin:/opt/darktable/bin:/home/jkt/.local/bin:/home/jkt/bin/:/opt/qtc/bin:/opt/darktable/bin:/usr/x86_64-pc-linux-gnu/gcc-bin/8.1.0:/usr/lib/llvm/6/bin:/usr/lib/llvm/4/bin:/usr/local/bin:/usr/bin:/bin:/opt/bin:/usr/games/bin" /usr/bin/make -C /home/jkt/work/prog/_build/br-cfb/build/libredblack-1.3/ /usr/bin/make all-am /bin/sh ./libtool --mode=compile /home/jkt/work/prog/_build/br-cfb/host/bin/arm-linux-gnueabihf-gcc -DHAVE_CONFIG_H -I. -I. -I. -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -Wall -c -o redblack.lo `test -f 'redblack.c' || echo './'`redblack.c /home/jkt/work/prog/_build/br-cfb/host/bin/arm-linux-gnueabihf-gcc -DHAVE_CONFIG_H -I. -I. -I. -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -Wall -c redblack.c -fPIC -DPIC -o redblack.lo redblack.c: In function 'rbinit': redblack.c:143:7: warning: variable 'c' set but not used [-Wunused-but-set-variable] char c; ^ /bin/sh ./libtool --mode=link /home/jkt/work/prog/_build/br-cfb/host/bin/arm-linux-gnueabihf-gcc -Wall -Wl,-z,now -Wl,-z,relro -specs=/home/jkt/work/cesnet/gerrit/github/buildroot/buildroot/toolchain/gcc-specs-pie-ld -o libredblack.la -rpath /usr/lib -version-info 2:3:2 redblack.lo mkdir .libs rm -fr .libs/libredblack.la .libs/libredblack.* .libs/libredblack.* (cd . && ln -s redblack.lo redblack.o) /home/jkt/work/prog/_build/br-cfb/host/bin/arm-linux-gnueabihf-gcc -shared redblack.lo -Wl,-z -Wl,now -Wl,-z -Wl,relro -Wl,-soname -Wl,libredblack.so.0 -o .libs/libredblack.so.0.2.3 (cd .libs && rm -f libredblack.so.0 && ln -s libredblack.so.0.2.3 libredblack.so.0) (cd .libs && rm -f libredblack.so && ln -s libredblack.so.0.2.3 libredblack.so) creating libredblack.la (cd .libs && rm -f libredblack.la && ln -s ../libredblack.la libredblack.la) /home/jkt/work/prog/_build/br-cfb/host/bin/arm-linux-gnueabihf-gcc -DHAVE_CONFIG_H -I. -I. -I. -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -Wall -c `test -f 'example.c' || echo './'`example.c /bin/sh ./libtool --mode=link /home/jkt/work/prog/_build/br-cfb/host/bin/arm-linux-gnueabihf-gcc -Wall -Wl,-z,now -Wl,-z,relro -specs=/home/jkt/work/cesnet/gerrit/github/buildroot/buildroot/toolchain/gcc-specs-pie-ld -o example example.o libredblack.la /home/jkt/work/prog/_build/br-cfb/host/bin/arm-linux-gnueabihf-gcc -Wall -Wl,-z -Wl,now -Wl,-z -Wl,relro -specs=/home/jkt/work/cesnet/gerrit/github/buildroot/buildroot/toolchain/gcc-specs-pie-ld -o .libs/example example.o ./.libs/libredblack.so /home/jkt/work/prog/_build/br-cfb/host/opt/ext-toolchain/bin/../lib/gcc/arm-linux-gnueabihf/7.3.1/../../../../arm-linux-gnueabihf/bin/ld: example.o: relocation R_ARM_MOVW_ABS_NC against `stderr' can not be used when making a shared object; recompile with -fPIC example.o: error adding symbols: Bad value collect2: error: ld returned 1 exit status make[3]: *** [Makefile:267: example] Error 1 make[2]: *** [Makefile:187: all] Error 2 make[1]: *** [package/pkg-generic.mk:232: /home/jkt/work/prog/_build/br-cfb/build/libredblack-1.3/.stamp_built] Error 2 make: *** [Makefile:16: _all] Error 2
I just got a similar failure from an in-tree package, the spi-tools:
>>> spi-tools 0.8.1 Building
PATH="/home/jkt/work/prog/_build/br-cfb/host/bin:/home/jkt/work/prog/_build/br-cfb/host/sbin:/home/jkt/.local/bin:/home/jkt/bin/:/opt/qtc/bin:/opt/darktable/bin:/home/jkt/.local/bin:/home/jkt/bin/:/opt/qtc/bin:/opt/darktable/bin:/usr/x86_64-pc-linux-gnu/gcc-bin/8.1.0:/usr/lib/llvm/6/bin:/usr/lib/llvm/4/bin:/usr/local/bin:/usr/bin:/bin:/opt/bin:/usr/games/bin"
/usr/bin/make -C
/home/jkt/work/prog/_build/br-cfb/build/spi-tools-0.8.1/
Making all in src
/usr/bin/make all-am
/home/jkt/work/prog/_build/br-cfb/host/bin/arm-linux-gnueabihf-gcc -O0
-Wl,-z,now -Wl,-z,relro
-specs=/home/jkt/work/cesnet/gerrit/github/buildroot/buildroot/toolchain/gcc-specs-pie-ld
-o spi-config spi-config.o
/home/jkt/work/prog/_build/br-cfb/host/opt/ext-toolchain/bin/../lib/gcc/arm-linux-gnueabihf/7.3.1/../../../../arm-linux-gnueabihf/bin/ld:
spi-config.o: relocation R_ARM_MOVW_ABS_NC against `stderr' can not be used
when making a shared object; recompile with -fPIC
spi-config.o: error adding symbols: Bad value
collect2: error: ld returned 1 exit status
Jan, On Wed, Aug 8, 2018 at 3:36 AM Jan Kundrát <jan.kundrat@cesnet.cz> wrote: > > I just got a similar failure from an in-tree package, the spi-tools: Cool, would you mind sharing your buildroot master hash + which of the patchwork patches you've applied to it? I'll recreate and take a look. Matt
Jan, On Wed, Aug 8, 2018 at 3:36 AM Jan Kundrát <jan.kundrat@cesnet.cz> wrote: > > I just got a similar failure from an in-tree package, the spi-tools: > > >>> spi-tools 0.8.1 Building > PATH="/home/jkt/work/prog/_build/br-cfb/host/bin:/home/jkt/work/prog/_build/br-cfb/host/sbin:/home/jkt/.local/bin:/home/jkt/bin/:/opt/qtc/bin:/opt/darktable/bin:/home/jkt/.local/bin:/home/jkt/bin/:/opt/qtc/bin:/opt/darktable/bin:/usr/x86_64-pc-linux-gnu/gcc-bin/8.1.0:/usr/lib/llvm/6/bin:/usr/lib/llvm/4/bin:/usr/local/bin:/usr/bin:/bin:/opt/bin:/usr/games/bin" > /usr/bin/make -C > /home/jkt/work/prog/_build/br-cfb/build/spi-tools-0.8.1/ > Making all in src > /usr/bin/make all-am > /home/jkt/work/prog/_build/br-cfb/host/bin/arm-linux-gnueabihf-gcc -O0 > -Wl,-z,now -Wl,-z,relro > -specs=/home/jkt/work/cesnet/gerrit/github/buildroot/buildroot/toolchain/gcc-specs-pie-ld > -o spi-config spi-config.o > /home/jkt/work/prog/_build/br-cfb/host/opt/ext-toolchain/bin/../lib/gcc/arm-linux-gnueabihf/7.3.1/../../../../arm-linux-gnueabihf/bin/ld: > spi-config.o: relocation R_ARM_MOVW_ABS_NC against `stderr' can not be used > when making a shared object; recompile with -fPIC > spi-config.o: error adding symbols: Bad value > collect2: error: ld returned 1 exit status This one has to do with the configure.ac of that package not observing cflags/cxxflags being passed to configure. So the *.o that are trying to be linked weren't build as position independent. Probably a similar case for your libredblack package. I've posted an upstream issue at this point and a basic fix. As buildroot could probably use a bump, rather then carrying the patch, I'll ask upstream if they could do a tag after this fix is merged. https://github.com/cpb-/spi-tools/issues/12 Matt
Matt, Stefan, On Wed, 11 Jul 2018 09:31:10 -0500, Matt Weber wrote: > From: Stefan Sørensen <stefan.sorensen@spectralink.com> > > The PIE build flags are only intended for building executables and can not be > used in relocateable links (-r), static builds and shared library build - > including the flags here causes build errors. > > So instead of parsing the PIE flags directly on the command line to gcc, > include them in a gcc spec file where it is possible to only apply the flags > when other incompatible flags are not set. > > This method and the spec files are from the Fedora build system. > > Originally submitted as > http://patchwork.ozlabs.org/patch/907093/ > > Signed-off-by: Stefan Sørensen <stefan.sorensen@spectralink.com> > Signed-off-by: Matt Weber <matthew.weber@rockwellcollins.com> I've read the whole discussion on this patch, but still I don't understand which direction we want to take moving forward: move away from handling options in the wrapper, and use a spec file for everything ? Or keep the entire logic in the wrapper ? I'm not happy at all with the approach of having some flags handled in the wrapper, some flags handled through spec files. I believe choosing the spec file direction makes this patch series more difficult to merge, because we have to go through this whole discussion of spec file vs. wrapper. I have nothing against using spec files, but right now, our logic is based on a wrapper program. Therefore, I would be more comfortable with an approach that relies on the wrapper program, so that it is in line with what we are doing today. Then, separately, we can discuss how our wrapper can be replaced, completely or partially, by a spec file. Best regards, Thomas
Thomas, On Fri, Aug 10, 2018 at 3:50 PM Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote: > > Matt, Stefan, > > On Wed, 11 Jul 2018 09:31:10 -0500, Matt Weber wrote: > > From: Stefan Sørensen <stefan.sorensen@spectralink.com> > > > > The PIE build flags are only intended for building executables and can not be > > used in relocateable links (-r), static builds and shared library build - > > including the flags here causes build errors. > > > > So instead of parsing the PIE flags directly on the command line to gcc, > > include them in a gcc spec file where it is possible to only apply the flags > > when other incompatible flags are not set. > > > > This method and the spec files are from the Fedora build system. > > > > Originally submitted as > > http://patchwork.ozlabs.org/patch/907093/ > > > > Signed-off-by: Stefan Sørensen <stefan.sorensen@spectralink.com> > > Signed-off-by: Matt Weber <matthew.weber@rockwellcollins.com> > > I've read the whole discussion on this patch, but still I don't > understand which direction we want to take moving forward: move away > from handling options in the wrapper, and use a spec file for > everything ? Or keep the entire logic in the wrapper ? So far, the spec files are just fixing a conditional case where a option isn't compatible. ie PIE and shared can't be used together. This prevents a lot of patchwork on all the packages. I could move this string search and condition process to the wrapper but it seems cleaner to do that in the spec file. I do want to note that we did not go to the extent that gentoo did where they use spec files to enable and do the conditional compatibility fixups at link time(PIE vs shared) for all the hardening options. > > I'm not happy at all with the approach of having some flags handled in > the wrapper, some flags handled through spec files. I believe choosing > the spec file direction makes this patch series more difficult to > merge, because we have to go through this whole discussion of spec file > vs. wrapper. Agree, the specfile even just doing the conditional fixup is a 3rd leg on this whole thing. We can add it to the wrapper code but we will loose visibility to what the wrapper is doing vs with the spec we can dump and analyze the output. > > I have nothing against using spec files, but right now, our logic is > based on a wrapper program. Therefore, I would be more comfortable with > an approach that relies on the wrapper program, so that it is in line > with what we are doing today. Then, separately, we can discuss how our > wrapper can be replaced, completely or partially, by a spec file. Ok. If the above feedback in this email doesn't change the view related to spec files, I can look at switching to a pure wrapper for fixups and static setting of variables with flags. Matt
Hello, On Fri, 10 Aug 2018 19:42:32 -0500, Matthew Weber wrote: > > I've read the whole discussion on this patch, but still I don't > > understand which direction we want to take moving forward: move away > > from handling options in the wrapper, and use a spec file for > > everything ? Or keep the entire logic in the wrapper ? > > So far, the spec files are just fixing a conditional case where a > option isn't compatible. ie PIE and shared can't be used together. > This prevents a lot of patchwork on all the packages. I could move > this string search and condition process to the wrapper but it seems > cleaner to do that in the spec file. I do want to note that we did > not go to the extent that gentoo did where they use spec files to > enable and do the conditional compatibility fixups at link time(PIE vs > shared) for all the hardening options. I understand the idea of using the spec file. However, I'm worried that it further complicates the overall toolchain setup. In addition, the -spec option passed in TARGET_CFLAGS/TARGET_LDFLAGS will only work with packages that do obey to CFLAGS/LDFLAGS, and some broken packages don't. Being in the wrapper allows to ensure the flags are used. And additionally, what's done in the wrapper is also used when the Buildroot toolchain is used "manually" (outside of the Buildroot infrastructure) to build stuff. > > I'm not happy at all with the approach of having some flags handled in > > the wrapper, some flags handled through spec files. I believe choosing > > the spec file direction makes this patch series more difficult to > > merge, because we have to go through this whole discussion of spec file > > vs. wrapper. > > Agree, the specfile even just doing the conditional fixup is a 3rd leg > on this whole thing. We can add it to the wrapper code but we will > loose visibility to what the wrapper is doing vs with the spec we can > dump and analyze the output. That's not true: the wrapper looks at BR2_DEBUG_WRAPPER={1,2} to dump what it is doing: /* Debug the wrapper to see actual arguments passed to * the compiler: * unset, empty, or 0: do not trace * set to 1 : trace all arguments on a single line * set to 2 : trace one argument per line */ if ((env_debug = getenv("BR2_DEBUG_WRAPPER"))) { > > I have nothing against using spec files, but right now, our logic is > > based on a wrapper program. Therefore, I would be more comfortable with > > an approach that relies on the wrapper program, so that it is in line > > with what we are doing today. Then, separately, we can discuss how our > > wrapper can be replaced, completely or partially, by a spec file. > > Ok. If the above feedback in this email doesn't change the view > related to spec files, I can look at switching to a pure wrapper for > fixups and static setting of variables with flags. I think I'd be happier with a first step where the wrapper is being used. Then in a second step, we can decide to rethink what the wrapper is doing, and move more of its logic to a spec file. I think a wrapper will still be needed anyway, at least to pass --sysroot and --spec. Best regards, Thomas
Thomas, On Sat, Aug 11, 2018 at 5:30 AM Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote: > > Hello, > > On Fri, 10 Aug 2018 19:42:32 -0500, Matthew Weber wrote: > > > > I've read the whole discussion on this patch, but still I don't > > > understand which direction we want to take moving forward: move away > > > from handling options in the wrapper, and use a spec file for > > > everything ? Or keep the entire logic in the wrapper ? > > > > So far, the spec files are just fixing a conditional case where a > > option isn't compatible. ie PIE and shared can't be used together. > > This prevents a lot of patchwork on all the packages. I could move > > this string search and condition process to the wrapper but it seems > > cleaner to do that in the spec file. I do want to note that we did > > not go to the extent that gentoo did where they use spec files to > > enable and do the conditional compatibility fixups at link time(PIE vs > > shared) for all the hardening options. > > I understand the idea of using the spec file. However, I'm worried that > it further complicates the overall toolchain setup. In addition, the > -spec option passed in TARGET_CFLAGS/TARGET_LDFLAGS will only work with > packages that do obey to CFLAGS/LDFLAGS, and some broken packages > don't. Being in the wrapper allows to ensure the flags are used. And > additionally, what's done in the wrapper is also used when the > Buildroot toolchain is used "manually" (outside of the Buildroot > infrastructure) to build stuff. > > > > I'm not happy at all with the approach of having some flags handled in > > > the wrapper, some flags handled through spec files. I believe choosing > > > the spec file direction makes this patch series more difficult to > > > merge, because we have to go through this whole discussion of spec file > > > vs. wrapper. > > > > Agree, the specfile even just doing the conditional fixup is a 3rd leg > > on this whole thing. We can add it to the wrapper code but we will > > loose visibility to what the wrapper is doing vs with the spec we can > > dump and analyze the output. > > That's not true: the wrapper looks at BR2_DEBUG_WRAPPER={1,2} to dump > what it is doing: > > /* Debug the wrapper to see actual arguments passed to > * the compiler: > * unset, empty, or 0: do not trace > * set to 1 : trace all arguments on a single line > * set to 2 : trace one argument per line > */ > if ((env_debug = getenv("BR2_DEBUG_WRAPPER"))) { > > > > I have nothing against using spec files, but right now, our logic is > > > based on a wrapper program. Therefore, I would be more comfortable with > > > an approach that relies on the wrapper program, so that it is in line > > > with what we are doing today. Then, separately, we can discuss how our > > > wrapper can be replaced, completely or partially, by a spec file. > > > > Ok. If the above feedback in this email doesn't change the view > > related to spec files, I can look at switching to a pure wrapper for > > fixups and static setting of variables with flags. > > I think I'd be happier with a first step where the wrapper is being > used. Then in a second step, we can decide to rethink what the wrapper > is doing, and move more of its logic to a spec file. I think a wrapper > will still be needed anyway, at least to pass --sysroot and --spec. > I started to look at this tonight and if I understand the current wrapper, it's solely working with cc/gcc. If someone explicity uses ld, the wrapper doesn't take affect? (I'll do a build tomorrow and look at behavior, didn't have a linux machine handy tonight to check. If that's the case, any pointers to detecting the type of invocation of the wrapper if I update the symlinking so ld also goes through? Matt
Hello, On Sat, 11 Aug 2018 22:55:47 -0500, Matthew Weber wrote: > I started to look at this tonight and if I understand the current > wrapper, it's solely working with cc/gcc. If someone explicity uses > ld, the wrapper doesn't take affect? (I'll do a build tomorrow and > look at behavior, didn't have a linux machine handy tonight to check. We only wrap gcc/cc indeed. Packages should not call "ld" directly, as it breaks a bunch of other situations. For example, on mips64, using "ld" directly doesn't work as it needs to be passed the "machine emulation", which gcc does. So I don't think we need to wrap "ld", as ld shouldn't be used directly. The only packages that should use "ld" directly are things like the Linux kernel or bootloaders. Best regards, Thomas
Thomas, On Sun, Aug 12, 2018 at 2:41 AM Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote: > > Hello, > > On Sat, 11 Aug 2018 22:55:47 -0500, Matthew Weber wrote: > > > I started to look at this tonight and if I understand the current > > wrapper, it's solely working with cc/gcc. If someone explicity uses > > ld, the wrapper doesn't take affect? (I'll do a build tomorrow and > > look at behavior, didn't have a linux machine handy tonight to check. > > We only wrap gcc/cc indeed. Packages should not call "ld" directly, as > it breaks a bunch of other situations. For example, on mips64, using > "ld" directly doesn't work as it needs to be passed the "machine > emulation", which gcc does. > > So I don't think we need to wrap "ld", as ld shouldn't be used > directly. The only packages that should use "ld" directly are things > like the Linux kernel or bootloaders. > The current hardening approach is trying to cover the cases where packages are still using ld directly and have other incompatible flags set (static/shared/r). I don't have the exact list but I believe busybox is even one of those and others like valgrind, boost, etc who use the "shared" flag and adding "pie" causes a compile failure. So we do still need to cover the ld case or go patch packages. What I could do is move the cc1 spec file conditional add of PIE into the wrapper. Then leave the LDFLAGS as we have them and the associated spec file that does a conditional add of "pie". This would prevent us from wrapping the ld tool and keep the existing wrapper approach consistent. Matt
Hello, On Sun, 12 Aug 2018 07:49:19 -0500, Matthew Weber wrote: > > So I don't think we need to wrap "ld", as ld shouldn't be used > > directly. The only packages that should use "ld" directly are things > > like the Linux kernel or bootloaders. > > The current hardening approach is trying to cover the cases where > packages are still using ld directly and have other incompatible flags > set (static/shared/r). I don't have the exact list but I believe > busybox is even one of those and others like valgrind, boost, etc who > use the "shared" flag and adding "pie" causes a compile failure. So > we do still need to cover the ld case or go patch packages. I find it weird that those packages are using "ld" directly, because if that's the case, we would have build failures on some mips64 configurations. > What I could do is move the cc1 spec file conditional add of PIE into > the wrapper. Then leave the LDFLAGS as we have them and the > associated spec file that does a conditional add of "pie". This would > prevent us from wrapping the ld tool and keep the existing wrapper > approach consistent. If we really need to do some custom logic around ld, then I believe I'd prefer to have a wrapper for it as well, to keep things consistent. But of course, Arnout's opinion on the matter would be welcome. Thomas
On 12-08-18 17:07, Thomas Petazzoni wrote: > Hello, > > On Sun, 12 Aug 2018 07:49:19 -0500, Matthew Weber wrote: > >>> So I don't think we need to wrap "ld", as ld shouldn't be used >>> directly. The only packages that should use "ld" directly are things >>> like the Linux kernel or bootloaders. >> >> The current hardening approach is trying to cover the cases where >> packages are still using ld directly and have other incompatible flags >> set (static/shared/r). I don't have the exact list but I believe >> busybox is even one of those and others like valgrind, boost, etc who >> use the "shared" flag and adding "pie" causes a compile failure. So >> we do still need to cover the ld case or go patch packages. > > I find it weird that those packages are using "ld" directly, because if > that's the case, we would have build failures on some mips64 > configurations. In addition, if you call ld directly, the spec file is not used at all... The spec file is only used by the gcc wrapper. >> What I could do is move the cc1 spec file conditional add of PIE into >> the wrapper. Then leave the LDFLAGS as we have them and the >> associated spec file that does a conditional add of "pie". This would >> prevent us from wrapping the ld tool and keep the existing wrapper >> approach consistent. This indicates the crux of the problem with the wrapper: it is not easy to detect in the wrapper that we're linking. This is "solved" by specifying the linker spec file in LDFLAGS, but since many packages (probably) don't use LDFLAGS, it is not really solved at all... Well, except in the sense that if a package doesn't look at LDFLAGS, it's not going to get the -pie hardening flag either so the problem that this patch is fixing will not occur. We *can* keep passing -pie in LDFLAGS, and then remove it again in the wrapper when -shared or -r or -static is present, but it feels weird... Hm, but apparently we can just always pass -pie in the wrapper, even when we're not linking. That would solve the issue relatively elegantly within the wrapper. > If we really need to do some custom logic around ld, then I believe I'd > prefer to have a wrapper for it as well, to keep things consistent. But > of course, Arnout's opinion on the matter would be welcome. I don't think an ld wrapper is needed. We've discussed this many times already, and I see less and less reason for it. Regards, Arnout > > Thomas >
Jan, On Wed, Aug 8, 2018 at 3:36 AM Jan Kundrát <jan.kundrat@cesnet.cz> wrote: > > I just got a similar failure from an in-tree package, the spi-tools: > > >>> spi-tools 0.8.1 Building > PATH="/home/jkt/work/prog/_build/br-cfb/host/bin:/home/jkt/work/prog/_build/br-cfb/host/sbin:/home/jkt/.local/bin:/home/jkt/bin/:/opt/qtc/bin:/opt/darktable/bin:/home/jkt/.local/bin:/home/jkt/bin/:/opt/qtc/bin:/opt/darktable/bin:/usr/x86_64-pc-linux-gnu/gcc-bin/8.1.0:/usr/lib/llvm/6/bin:/usr/lib/llvm/4/bin:/usr/local/bin:/usr/bin:/bin:/opt/bin:/usr/games/bin" > /usr/bin/make -C > /home/jkt/work/prog/_build/br-cfb/build/spi-tools-0.8.1/ > Making all in src > /usr/bin/make all-am > /home/jkt/work/prog/_build/br-cfb/host/bin/arm-linux-gnueabihf-gcc -O0 > -Wl,-z,now -Wl,-z,relro > -specs=/home/jkt/work/cesnet/gerrit/github/buildroot/buildroot/toolchain/gcc-specs-pie-ld > -o spi-config spi-config.o > /home/jkt/work/prog/_build/br-cfb/host/opt/ext-toolchain/bin/../lib/gcc/arm-linux-gnueabihf/7.3.1/../../../../arm-linux-gnueabihf/bin/ld: > spi-config.o: relocation R_ARM_MOVW_ABS_NC against `stderr' can not be used > when making a shared object; recompile with -fPIC > spi-config.o: error adding symbols: Bad value > collect2: error: ld returned 1 exit status I was able to get the spi-tools cflags not passing through from buildroot fix upstreamed and new release from spi-tools. Just sent a bump to buildroot. http://patchwork.ozlabs.org/patch/963100/
diff --git a/package/Makefile.in b/package/Makefile.in index 14b3bbd243..00ddf48c10 100644 --- a/package/Makefile.in +++ b/package/Makefile.in @@ -158,8 +158,8 @@ ifeq ($(BR2_RELRO_PARTIAL),y) TARGET_HARDENED += $(TARGET_CFLAGS_RELRO) TARGET_LDFLAGS += $(TARGET_CFLAGS_RELRO) else ifeq ($(BR2_RELRO_FULL),y) -TARGET_HARDENED += -fPIE $(TARGET_CFLAGS_RELRO_FULL) -TARGET_LDFLAGS += -pie $(TARGET_CFLAGS_RELRO_FULL) +TARGET_HARDENED += $(TARGET_CFLAGS_RELRO_FULL) -specs=$(TOPDIR)/toolchain/gcc-specs-pie-cc1 +TARGET_LDFLAGS += $(TARGET_CFLAGS_RELRO_FULL) -specs=$(TOPDIR)/toolchain/gcc-specs-pie-ld endif ifeq ($(BR2_FORTIFY_SOURCE_1),y) diff --git a/toolchain/gcc-specs-pie-cc1 b/toolchain/gcc-specs-pie-cc1 new file mode 100644 index 0000000000..fc54bcb510 --- /dev/null +++ b/toolchain/gcc-specs-pie-cc1 @@ -0,0 +1,2 @@ +*cc1_options: ++ %{!r:%{!fpie:%{!fPIE:%{!fpic:%{!fPIC:%{!fno-pic:-fPIE}}}}}} diff --git a/toolchain/gcc-specs-pie-ld b/toolchain/gcc-specs-pie-ld new file mode 100644 index 0000000000..bd6b9071ff --- /dev/null +++ b/toolchain/gcc-specs-pie-ld @@ -0,0 +1,2 @@ +*self_spec: ++ %{!static:%{!shared:%{!r:-pie}}}