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

Dimitri John Ledkov
2021-02-21T04:04:40Z

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.

It should not go upstream nor into debian.

NACK

On Sun, Feb 21, 2021 at 3:59 AM Dave Chinner <david@fromorbit.com> wrote:
>
> On Sat, Feb 20, 2021 at 01:16:07PM +0100, Bastian Germann wrote:
> > This is a change introduced in 5.6.0-1ubuntu3.
> >
> > Reported-by: Dimitri John Ledkov <xnox@ubuntu.com>
> > Signed-off-by: Bastian Germann <bastiangermann@fishpost.de>
> > ---
> > debian/changelog | 1 +
> > debian/rules | 8 +++++++-
> > 2 files changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/debian/changelog b/debian/changelog
> > index 8320a2e8..c77f04ab 100644
> > --- a/debian/changelog
> > +++ b/debian/changelog
> > @@ -2,6 +2,7 @@ xfsprogs (5.11.0-rc0-1) experimental; urgency=medium
> >
> > [ Dimitri John Ledkov ]
> > * Drop trying to create upstream distribution
> > + * Enable CET on amd64
> >
> > -- Bastian Germann <bastiangermann@fishpost.de> Sat, 20 Feb 2021 11:57:31 +0100
> >
> > diff --git a/debian/rules b/debian/rules
> > index 8a3345b6..dd093f2c 100755
> > --- a/debian/rules
> > +++ b/debian/rules
> > @@ -23,8 +23,14 @@ pkgdev = DIST_ROOT=`pwd`/$(dirdev); export DIST_ROOT;
> > pkgdi = DIST_ROOT=`pwd`/$(dirdi); export DIST_ROOT;
> > stdenv = @GZIP=-q; export GZIP;
> >
> > +ifeq ($(target),amd64)
> > +export DEB_CFLAGS_MAINT_APPEND=-fcf-protection
> > +export DEB_LDFLAGS_MAINT_APPEND=-fcf-protection
> > +endif
> > +include /usr/share/dpkg/default.mk
> > +
> > options = export DEBUG=-DNDEBUG DISTRIBUTION=debian \
> > - INSTALL_USER=root INSTALL_GROUP=root \
> > + INSTALL_USER=root INSTALL_GROUP=root LDFLAGS='$(LDFLAGS)' \
> > LOCAL_CONFIGURE_OPTIONS="--enable-editline=yes --enable-blkid=yes --disable-ubsan --disable-addrsan --disable-threadsan --enable-lto" ;
> > diopts = $(options) \
> > export OPTIMIZER=-Os LOCAL_CONFIGURE_OPTIONS="--enable-gettext=no --disable-ubsan --disable-addrsan --disable-threadsan --enable-lto" ;
>
> No. This is not the way to turn on build wide compiler/linker
> options/protections.
>
> IOWs, if you want to turn on control flow protections to make ROP
> exploits harder (why that actually matters for xfsprogs is beyond
> me), then it you need to add a configure option similar to
> --enable-lto. Then it can actually be enabled and used by other
> distros, not just Ubuntu, and it will also ensure that builds will
> fail at configure time if the compiler/linker does not support this
> functionality.
>
> Cheers,
>
> Dave.
> --
> Dave Chinner
> david@fromorbit.com

--
Regards,

Dimitri.