Re: [PATCH 2/2] mingw: enable DEP and ASLR
- Date: Wed, 8 May 2019 13:27:08 +0200 (DST)
- From: Johannes Schindelin <Johannes.Schindelin@xxxxxx>
- Subject: Re: [PATCH 2/2] mingw: enable DEP and ASLR
Hi Jonathan & Peff,
On Wed, 1 May 2019, Jonathan Nieder wrote:
> Jeff King wrote:
> > I wonder if this points to this patch touching the wrong level. These
> > compiler flags are a thing that _some_ builds want (i.e., production
> > builds where people care most about security and not about debugging),
> > but not necessarily all.
> > I'd have expected this to be tweakable by a Makefile knob (either a
> > specific knob, or just the caller setting the right CFLAGS etc), and
> > then for the builds of Git for Windows to turn those knobs when making a
> > package to distribute.
> > Our internal package builds at GitHub all have this in their config.mak
> > (for Linux, of course):
> > CFLAGS += -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=1
> > CFLAGS += -fstack-protector-strong
> > CFLAGS += -fpie
> > LDFLAGS += -z relro -z now
> > LDFLAGS += -pie
> > and I wouldn't be surprised if other binary distributors (like the
> > Debian package) do something similar.
> Yes, the Debian package uses
> CFLAGS := -Wall \
> $(shell dpkg-buildflags --get CFLAGS) \
> $(shell dpkg-buildflags --get CPPFLAGS)
> and then passes CFLAGS='$(CFLAGS)' to "make".
> That means we're using
> -g -O2 -fstack-protector-strong -Wformat -Werror=format-security
> -Wdate-time -D_FORTIFY_SOURCE=2
> Dscho's suggestion for the Windows build sounds fine to me (if
> checking for -Og, too). Maybe it would make sense to factor out a
> makefile variable for this, that could be used for builds on other
> platforms, too. That way, the autodetection can be in one place, and
> there is a standard way to override it when the user wants something
Indeed, if I was to add a generic "are we building for production?"
function, this would be incorrect.
But this is not the case here, we are doing something very specific,
Windows-only here, and for the sole reason to keep debuggability (for
which the presence of the `-g` option indeed would not be a good
indicator: in Git for Windows, we build `.pdb` files so that stackdumps
can be more meaningful, but we do not want to have full debug information
in those executables).
In the long run, I think we need to become more explicit about this, by
adding a "FOR_PRODUCTION" flag. It's really no good if we use
implementation details such as CFLAGS to deduce intent.
That's for another patch series, though, as it is pretty clear-cut here:
If you build with optimization flags using Git for Windows' SDK, you
cannot use gdb for single-stepping, likewise if you use ASLR, so we can
totally piggyback the latter onto the former.