Re: NACK Re: [PATCH 2/4] debian: Enable CET on amd64

[Dave Chinner <david@fromorbit.com>]

On Sun, Feb 21, 2021 at 04:32:46AM +0000, Dimitri John Ledkov wrote:
> On Sun, Feb 21, 2021 at 4:28 AM Dave Chinner <david@fromorbit.com> wrote:
> >
> > On Sun, Feb 21, 2021 at 04:02:55AM +0000, Dimitri John Ledkov wrote:
> > > The patch in question is specific to Ubuntu and was not submitted by
> > > me to neither Debian or Upstream.
> > >
> > > Indeed, this is very distro specific, because of all the other things
> > > that we turn on by default in our toolchain, dpkg build flags, and all
> > > other packages.
> > >
> > > This patch if taken at face value, will not enable CET. And will make
> > > the package start failing to build from source, when using older
> > > toolchains that don't support said flag.
> >
> > Yes, that is exactly what I said when pointing out how to *support
> > this properly* so it doesn't break builds in environments that do
> > not support such functionality.
> >
> > Having it as a configure option allows the configure script to -test
> > whether the toolchain supports it- and then either fail (enable=yes)
> > or not use it (enable=probe) and continue the build without it.
> >
> > > It should not go upstream nor into debian.
> >
> > There is no reason it cannot be implemented as a build option in the
> > upstream package. Then you can get rid of all your nasty hacks and
> > simply add --enable-cf-protections to your distro's configure
> > options.
> >
> > And other distros that also support all this functionality can use
> > it to. Please play nice with others and do things the right way
> > instead of making silly claims about how "nobody else can use this"
> > when it's clear that they can if they also tick all the necessary
> > boxes.
> 
> debian will probably will not want --enable-cf-protections as a
> configure option, and will enable CET via dpkg-buildflags as a
> hardening option, which will then be turned on by default for relevant
> architectures and series as of when debian kernel starts to support
> it.
> 
> as that way, debian will be able to affect that change across the whole distro.

Except now you've got the problem that dpkg-buildflags is not used
by various packages such as xfsprogs, so the flags it sets up,
even with overrides like DEB_BUILD_MAINT_OPTIONS are completely
ignored by the debian package build because debian/rules is not set
up to source the buildflags at all.

e.g. if you run 'make Q= deb' you won't see any of the compiler or
linker options emitted by dpkg-buildflags being used during the
debian package build. You will, OTOH, see it emit stuff like LTO
options because those get turned on via configure.

> once CET is actually merged into kernel, I do not expect configure
> options or debian/rules changes to enable CET. At most `export
> DEB_BUILD_MAINT_OPTIONS=hardening` should be enough in debian/rules.

The default dpkg-buildflags output already includes "hardening"
options, and as I said above, they aren't actually included in the
build rules. Hence what you suggest just won't work like you think
it should.

Again, if you want build option stuff to work correctly, don't rely
on distro specific magic to turn stuff on because it just doesn't
work for everyone. Use configure options to enable, probe and detect
support and enable the feature in the build, then upstream and other
distros can actually build with it and test it outside the magical
unicorn distro build environment you are running in...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com