Thread (16 messages) 16 messages, 7 authors, 2019-05-08

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

From: Johannes Schindelin <hidden>
Date: 2019-05-08 11:33:19

Hi Alban,

On Wed, 1 May 2019, Alban Gruin wrote:
Le 01/05/2019 à 00:41, Johannes Schindelin a écrit :
quoted
On Tue, 30 Apr 2019, Johannes Sixt wrote:
quoted
[had to add Dscho as recipient manually, mind you]
I usually pick up responses to GitGitGadget patch series even if I am not
on explicit Cc: (but it might take a couple of days when I am too busy
elsewhere to read the Git mailing list).
quoted
Am 29.04.19 um 23:56 schrieb İsmail Dönmez via GitGitGadget:
quoted
From: =?UTF-8?q?=C4=B0smail=20D=C3=B6nmez?= <redacted>

Enable DEP (Data Execution Prevention) and ASLR (Address Space Layout
Randomization) support. This applies to both 32bit and 64bit builds
and makes it substantially harder to exploit security holes in Git by
offering a much more unpredictable attack surface.

ASLR interferes with GDB's ability to set breakpoints. A similar issue
holds true when compiling with -O2 (in which case single-stepping is
messed up because GDB cannot map the code back to the original source
code properly). Therefore we simply enable ASLR only when an
I don’t know if it stands true when combined with something like -ggdb3,
but I may be very wrong.  Feel free to correct me.
Possibly, but that makes my job here harder, so I won't even try right now
;-)
quoted
quoted
quoted
optimization flag is present in the CFLAGS, using it as an indicator
that the developer does not want to debug in GDB anyway.

Signed-off-by: İsmail Dönmez <redacted>
Signed-off-by: Johannes Schindelin <redacted>
---
 config.mak.uname | 6 ++++++
 1 file changed, 6 insertions(+)
diff --git a/config.mak.uname b/config.mak.uname
index e7c7d14e5f..a9edcc5f0b 100644
--- a/config.mak.uname
+++ b/config.mak.uname
@@ -570,6 +570,12 @@ else
 	ifeq ($(shell expr "$(uname_R)" : '2\.'),2)
 		# MSys2
 		prefix = /usr/
+		# Enable DEP
+		BASIC_LDFLAGS += -Wl,--nxcompat
+		# Enable ASLR (unless debugging)
+		ifneq (,$(findstring -O,$(CFLAGS)))
+			BASIC_LDFLAGS += -Wl,--dynamicbase
+		endif
 		ifeq (MINGW32,$(MSYSTEM))
 			prefix = /mingw32
 			HOST_CPU = i686
I'm a bit concerned that this breaks my debug sessions where I use -O0.
But I'll test without -O0 before I really complain.
Weird. Jameson Miller also mentioned this very concern in an internal
review.

I guess I'll do something like

	ifneq (,$(findstring -O,$(filter-out -O0,$(CFLAGS))))
-Og also exists to debug[0], even if it’s far less known.
Good point.
Perhaps it’s better to check for -g (and its variants[1]) as the user
clearly states their intent to debug the resulting binary, rather than
checking for special cases.
I don't think we can use that, as we specifically build Git for Windows
with optimization *and* with debug symbols (and then use cv2pdb to extract
those debug symbols into external .pdb files for use with advanced
post-mortem tools, i.e. we do *not* need to single-step).

Thanks,
Dscho
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help