Re: [PATCH 2/2] mingw: enable DEP and ASLR

From: Jonathan Nieder <jrnieder@gmail.com>
Date: May 1, 2019, 22:02 UTC
Message-ID: <20190501220219.GA42435@google.com>
In-Reply-To: <20190501204631.GB13372@sigill.intra.peff.net>

Hi,

Jeff King wrote:

Show 22 quoted lines

> 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 else.

Thanks, Jonathan