Re: CFLAGS overridden by distribution build system

[Stephen Smalley <stephen.smalley.work@gmail.com>]

On Fri, May 29, 2020 at 1:15 PM William Roberts
<bill.c.roberts@gmail.com> wrote:
>
> On Fri, May 29, 2020 at 11:40 AM Stephen Smalley
> <stephen.smalley.work@gmail.com> wrote:
> >
> > On Fri, May 29, 2020 at 12:05 PM William Roberts
> > <bill.c.roberts@gmail.com> wrote:
> > >
> > > On Fri, May 29, 2020 at 8:31 AM Stephen Smalley
> > > <stephen.smalley.work@gmail.com> wrote:
> > > >
> > > > On Sat, May 23, 2020 at 11:59 AM William Roberts
> > > > <bill.c.roberts@gmail.com> wrote:
> > > > >
> > > > > On Sat, May 23, 2020 at 5:57 AM Laurent Bigonville <bigon@debian.org> 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.