On Fri, May 29, 2020 at 03:59:13PM -0500, William Roberts wrote: > On Fri, May 29, 2020 at 1:18 PM Stephen Smalley > <stephen.smalley.work@xxxxxxxxx> wrote: > > > > On Fri, May 29, 2020 at 1:15 PM William Roberts > > <bill.c.roberts@xxxxxxxxx> wrote: > > > > > > On Fri, May 29, 2020 at 11:40 AM Stephen Smalley > > > <stephen.smalley.work@xxxxxxxxx> wrote: > > > > > > > > On Fri, May 29, 2020 at 12:05 PM William Roberts > > > > <bill.c.roberts@xxxxxxxxx> wrote: > > > > > > > > > > On Fri, May 29, 2020 at 8:31 AM Stephen Smalley > > > > > <stephen.smalley.work@xxxxxxxxx> wrote: > > > > > > > > > > > > On Sat, May 23, 2020 at 11:59 AM William Roberts > > > > > > <bill.c.roberts@xxxxxxxxx> wrote: > > > > > > > > > > > > > > On Sat, May 23, 2020 at 5:57 AM Laurent Bigonville <bigon@xxxxxxxxxx> wrote: > > > > > > > > > > > > > > > > Hello, > > > > > > > > > > > > > > > > The current build system of the userspace is setting a lot of CFLAGS, > > > > > > > > but most of these are overridden by the distributions when building. > > > > > > > > > > > > > > > > Today I received a bug report[0] from Christian Göttsche asking me to > > > > > > > > set -fno-semantic-interposition again in libsepol. I see also the same > > > > > > > > flag and also a lot of others set in libselinux and libsemanage build > > > > > > > > system. > > > > > > > > > > > > > > Why would it be again? The old DSO design made that option impotent. > > > > > > > Clang does it by default. > > > > > > > > > > > > > > > > > > > > > > > For what I understand some of these are just needed for code quality > > > > > > > > (-W) and could be controlled by distributions but others might actually > > > > > > > > need to be always set (-f?). > > > > > > > > > > > > > > If you look at the Makefiles overall in all the projects, you'll see hardening > > > > > > > flags, etc. Libsepol has a pretty minimal set compared to say libselinux, but > > > > > > > they all get overridden by build time CFLAGS if you want. > > > > > > > > > > > > > > > > > > > > > > > Shouldn't the flags that always need to be set (which ones?) be moved to > > > > > > > > a "override CFLAGS" directive to avoid these to be unset by distributions? > > > > > > > > > > > > > > If you we're cleaver with CFLAGS before, you could acually circumvent > > > > > > > the buildtime > > > > > > > DSO stuff. While i'm not opposed to adding it to immutable flag, if > > > > > > > you're setting > > > > > > > CFLAGS, you're on your own. At least with the current design. > > > > > > > > > > > > > > I have no issues with adding it to override, but we would have to > > > > > > > overhaul a bunch > > > > > > > of them and make them consistent. > > > > > > > > > > > > I'm inclined to agree with Laurent here - we should always set this > > > > > > flag in order to preserve the same behavior prior to the patches that > > > > > > removed hidden_def/hidden_proto and switched to relying on this > > > > > > instead. > > > > > > > > > > Theirs a lot of features that rely on particular cflags, consider hardening > > > > > options. A lot of the warnings/error stuff is not just a code hygiene, but also > > > > > spotting potential issues that could arise as security issues. If one starts > > > > > tinkering with cflags, you can really change the code quite a bit. This is why > > > > > some places only approve sanctioned builds of crypto libraries. > > > > > > > > I think the difference here is that we made a change in the source > > > > code (hidden_def/hidden_proto removal) that requires use of > > > > -fno-semantic-interposition to preserve existing behavior. > > > > > > > > > But one of the things to consider is that for clang builds, clang > > > > > doesn't support > > > > > this option, so we would need to move the compiler checking code into each > > > > > projects root makefile and ensure any consuming subordinate leafs add a > > > > > default, or move it to the global makefile and make sure each leaf that needs it > > > > > has a default. > > > > > > > > I think clang does support it now? https://reviews.llvm.org/D72724 > > > > > > Yeah but that bug is all 2020 stuff. It is in the clang-10 release. I > > > verified that > > > with a local build from here: > > > - https://apt.llvm.org/ > > > So anything sub clang-10 won't support it, do you want to tie us to that > > > new of a clang? > > > > No, I guess not. So I guess our options are to leave it alone and > > just make sure it is well documented or complicate the Makefiles. > > Pretty much, which is why I go back to, "If your setting CFLAGS, you better > make sure you get em right". There are so many options and so many things > that can affect the build output, you really need to get em right. > This is the one area autoconf/automake is nice (I have a hate/hate > relationship with it), > but it does give one easy-peasy feature test macros, so this problem > is simple to fix. > > The problem with documentation is no one reads it :-p. We could do something > like issue a warning, compiler agnostic, from make if we don't detect the > -fsemantic-interposition option (easy). That would be simple, or I can > work up the > gcc/clang split patches (slightly harder). > > Or we can just say, if your messing with CFLAGS do it right and you should > probably add -fsemantic-interposition if your using gcc documentation (easiest). > > If they didn't change the CFLAGS for their build, they wouldn't have this issue. > Which brings up the question, "can they add the flags they need to the > Makefile?" > or "What do they need added/removed/changed in CFLAGS overall?" > > I'd like to start with why they need to change CFLAGS, and can we just > make that the default? > Fedora packager point of view: >From time to time, Fedora project decides to use new/different flags for different reasons. In order to make it easier for packagers there are Fedora distribution macros available which can be used in .spec files and which set all needed flags, e.g: $ grep CFLAGS libsepol.spec make %{?_smp_mflags} CFLAGS="%{optflags}" LDFLAGS="%{?__global_ldflags}" $ rpm --eval 'make %{?_smp_mflags} CFLAGS="%{optflags}" LDFLAGS="%{?__global_ldflags}"' make -j4 CFLAGS="-O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fexceptions -fstack-protector-strong -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -m64 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection" LDFLAGS="-Wl,-z,relro -Wl,--as-needed -Wl,-z,now -specs=/usr/lib/rpm/redhat/redhat-hardened-ld" There's a simpler way using %set_build_flags in .spec file: $ rpm --eval '%set_build_flags' CFLAGS="${CFLAGS:--O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fexceptions -fstack-protector-strong -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -m64 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection}" ; export CFLAGS ; CXXFLAGS="${CXXFLAGS:--O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fexceptions -fstack-protector-strong -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -m64 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection}" ; export CXXFLAGS ; FFLAGS="${FFLAGS:--O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fexceptions -fstack-protector-strong -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -m64 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection -I/usr/lib64/gfortran/modules}" ; export FFLAGS ; FCFLAGS="${FCFLAGS:--O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fexceptions -fstack-protector-strong -grecord-gcc-switches -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1 -m64 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection -I/usr/lib64/gfortran/modules}" ; export FCFLAGS ; LDFLAGS="${LDFLAGS:--Wl,-z,relro -Wl,--as-needed -Wl,-z,now -specs=/usr/lib/rpm/redhat/redhat-hardened-ld}" ; export LDFLAGS ; LT_SYS_LIBRARY_PATH="${LT_SYS_LIBRARY_PATH:-/usr/lib64:}" ; export LT_SYS_LIBRARY_PATH >From this POV it would be great to have -fno-semantic-interposition in CFLAGS override in libsepol Makefile. But given the compatibility problem described in this thread, I, as Fedora packager, would consider as sufficient to have this requirement in the build instructions in documentation, and also as part of new release notes as it's changed in the latest release. Petr
Attachment:
signature.asc
Description: PGP signature
