cygwin + binutils 2.36 + ASLR/dynamicbase defaults

cygwin + binutils 2.36 + ASLR/dynamicbase defaults

[Christoph Reiter]

Hey,

binutils 2.36 now defaults to ASLR etc on Windows, so a cygwin compiled linker
will give you:

> peflags -v mydll.dll
mydll.dll: coff(0x2026[+executable_image,+line_nums_stripped,+bigaddr,+dll])
pe(0x0160[+high-entropy-va,+dynamicbase,+nxcompat])

Is this still problematic for cygwin?

The reason I'm asking is because we updated to 2.36 in MSYS2 and are
wondering if
we need to patch this out (or change the defaults) It seems to work as is right
now, but maybe we are just lucky(?).

Some context: https://github.com/msys2/MSYS2-packages/pull/2345

regards

Re: cygwin + binutils 2.36 + ASLR/dynamicbase defaults

[ASSI]

Christoph Reiter via Cygwin writes:
> binutils 2.36 now defaults to ASLR etc on Windows, so a cygwin compiled linker
> will give you:
>
>> peflags -v mydll.dll
> mydll.dll: coff(0x2026[+executable_image,+line_nums_stripped,+bigaddr,+dll])
> pe(0x0160[+high-entropy-va,+dynamicbase,+nxcompat])
>
> Is this still problematic for cygwin?

Yes it is and I'm currently figuring out how to best get rid of it in
order to be able to update binutils (why this was ever allowed in
without an accompanying configure option is a mystery to me).  I've
already nixed it for Cygwin, but I'm not yet sure what to do for the
cross compilation toolchain.  While it should in principle work there,
I'm pretty sure that there will be problems when it comes to the nitty
gritty details.  It's already transpired that some of the linker scripts
can't deal with the larger base addresses this change does generate
eventually.

> The reason I'm asking is because we updated to 2.36 in MSYS2 and are
> wondering if we need to patch this out (or change the defaults) It
> seems to work as is right now, but maybe we are just lucky(?).

You are just lucky and need to test more. :-)

Note that the change does not only affect DLL as the commit message
would want you to believe and you will eventually end up with a
situation where ASLR tries moves the stack of an executable, at which
point you can no longer fork.


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

Waldorf MIDI Implementation & additional documentation:
http://Synth.Stromeko.net/Downloads.html#WaldorfDocs

Re: cygwin + binutils 2.36 + ASLR/dynamicbase defaults

[Christoph Reiter]

On Sun, Feb 28, 2021 at 1:16 PM ASSI <Stromeko@nexgo.de> wrote:
>
> Christoph Reiter via Cygwin writes:
> > binutils 2.36 now defaults to ASLR etc on Windows, so a cygwin compiled linker
> > will give you:
> >
> >> peflags -v mydll.dll
> > mydll.dll: coff(0x2026[+executable_image,+line_nums_stripped,+bigaddr,+dll])
> > pe(0x0160[+high-entropy-va,+dynamicbase,+nxcompat])
> >
> > Is this still problematic for cygwin?
>
> Yes it is and I'm currently figuring out how to best get rid of it in
> order to be able to update binutils (why this was ever allowed in
> without an accompanying configure option is a mystery to me).

OK, thanks (nxcompat as well btw?). I've reverted these changes in MSYS2 now:
https://github.com/msys2/MSYS2-packages/commit/c5757a43b42fb20730792469facf9a65571a2e81

> I've
> already nixed it for Cygwin, but I'm not yet sure what to do for the
> cross compilation toolchain.  While it should in principle work there,
> I'm pretty sure that there will be problems when it comes to the nitty
> gritty details.  It's already transpired that some of the linker scripts
> can't deal with the larger base addresses this change does generate
> eventually.

MSYS2 builds all packages with ASLR since 6 months, so things look good.
We've added a patch that allows reverting the base address if needed:
https://github.com/msys2/MINGW-packages/blob/master/mingw-w64-binutils/2001-ld-option-to-move-default-bases-under-4GB.patch

> > The reason I'm asking is because we updated to 2.36 in MSYS2 and are
> > wondering if we need to patch this out (or change the defaults) It
> > seems to work as is right now, but maybe we are just lucky(?).
>
> You are just lucky and need to test more. :-)
>
> Note that the change does not only affect DLL as the commit message
> would want you to believe and you will eventually end up with a
> situation where ASLR tries moves the stack of an executable, at which
> point you can no longer fork.

OK, thanks.

Re: cygwin + binutils 2.36 + ASLR/dynamicbase defaults

[ASSI]

Christoph Reiter via Cygwin writes:
> MSYS2 builds all packages with ASLR since 6 months, so things look
> good.

That doesn't tell you all that much since you will have to wait for some
unfavorable address space layout constellation to appear for the problem
to announce itself and then you need someone to recognize the reason and
report it back to you.  I tend to see this only on 32bit on my
development machine where I have a large amount of dependencies
installed.  After a reboot the problem will move somewhere else, which
means that you will need to find another reproducer.

> We've added a patch that allows reverting the base address if needed:
> https://github.com/msys2/MINGW-packages/blob/master/mingw-w64-binutils/2001-ld-option-to-move-default-bases-under-4GB.patch

In other words, that should be the default then since you can't know if
it works otherwise.  Also, I really don't think we should need to change
all toolchains to use these options just in order to produce working
executables.  If "safer" means "it doesn't work", then there are clearly
easier ways to get there.


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

SD adaptation for Waldorf rackAttack V1.04R1:
http://Synth.Stromeko.net/Downloads.html#WaldorfSDada

Re: cygwin + binutils 2.36 + ASLR/dynamicbase defaults

[Christoph Reiter]

On Sun, Feb 28, 2021 at 3:22 PM ASSI <Stromeko@nexgo.de> wrote:
>
> Christoph Reiter via Cygwin writes:
> > MSYS2 builds all packages with ASLR since 6 months, so things look
> > good.
>
> That doesn't tell you all that much since you will have to wait for some
> unfavorable address space layout constellation to appear for the problem
> to announce itself and then you need someone to recognize the reason and
> report it back to you.  I tend to see this only on 32bit on my
> development machine where I have a large amount of dependencies
> installed.  After a reboot the problem will move somewhere else, which
> means that you will need to find another reproducer.

To clarify: I was referring to non-cygwin targets. I had assumed you
meant that by "cross compilation toolchain".

> > We've added a patch that allows reverting the base address if needed:
> > https://github.com/msys2/MINGW-packages/blob/master/mingw-w64-binutils/2001-ld-option-to-move-default-bases-under-4GB.patch
>
> In other words, that should be the default then since you can't know if
> it works otherwise.  Also, I really don't think we should need to change
> all toolchains to use these options just in order to produce working
> executables.  If "safer" means "it doesn't work", then there are clearly
> easier ways to get there.

We've only needed this to to work around linker errors, so it's pretty
clear when it is needed as the build will fail.

Anyway, maybe the linked patches can be helpful to you.

regards

Re: cygwin + binutils 2.36 + ASLR/dynamicbase defaults

[Achim Gratz]

Christoph Reiter via Cygwin writes:
> To clarify: I was referring to non-cygwin targets. I had assumed you
> meant that by "cross compilation toolchain".

Then maybe I don't understand what you meant when you said you've built
MSys2 packages with ASLR on.  The MingW-W64 toolchains indeed target
Windows, but the code you compile it with may not exactly be meant to be
used that way.

I'd like to caution again that the appearance of anything working is
unfortunately different from it actually working.  I've been seriously
trying to enable ASLR on Cygwin several years ago (some vestiges of
which are still in the autorebase scripts).  After the first rounds of
ironing out the (now obvious) kinks, it was always in a state of "almost
working" until the day it didn't or the next installation that just
never got off the ground (keep in mind these were all installations on
pretty much identical hardware from a single master image).

> We've only needed this to to work around linker errors, so it's pretty
> clear when it is needed as the build will fail.

A linker error should just show up when this hits, so yes that's easier
to figure out.  I'm more concerned about those problems that won't show
up until much later.

> Anyway, maybe the linked patches can be helpful to you.

Yes, thanks for the links.  I'm not sure how relevant those are for
Cygwin yet, but I'll have a look in more detail later.


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

Samples for the Waldorf Blofeld:
http://Synth.Stromeko.net/Downloads.html#BlofeldSamplesExtra

Re: cygwin + binutils 2.36 + ASLR/dynamicbase defaults

[Jeremy Drake]

[-- Attachment #1: Type: text/plain, Size: 2258 bytes --]

On Sun, 28 Feb 2021, ASSI wrote:

> > Is this still problematic for cygwin?
>
> Yes it is and I'm currently figuring out how to best get rid of it in
> order to be able to update binutils (why this was ever allowed in
> without an accompanying configure option is a mystery to me).

Well, Microsoft's LINK.EXE and LLVM's LLD have already been using these
new defaults for some time.  But I was surprised how quickly my patch was
accepted/merged.

> I've
> already nixed it for Cygwin, but I'm not yet sure what to do for the
> cross compilation toolchain.  While it should in principle work there,
> I'm pretty sure that there will be problems when it comes to the nitty
> gritty details.  It's already transpired that some of the linker scripts
> can't deal with the larger base addresses this change does generate
> eventually.

To clarify, default base addresses should not have changed for cygwin
targets, they were already above 4GB.

I have a prelimiary patch that I plan to send upstream once I get some
testing done on it, which reverts the default dll characteristics for
cygwin targets.  I don't know if what you've done to 'nix it' for Cygwin
was similar.

I have not seen anything one way or the other on the NXCOMPAT flag.  Does
that also needs to be reverted for Cygwin?

> > The reason I'm asking is because we updated to 2.36 in MSYS2 and are
> > wondering if we need to patch this out (or change the defaults) It
> > seems to work as is right now, but maybe we are just lucky(?).
>
> You are just lucky and need to test more. :-)

I have seen the issues you described on 32-bit, but my understanding of
how ASLR works suggested that it should be very rare on 64-bit.

> Note that the change does not only affect DLL as the commit message
> would want you to believe and you will eventually end up with a
> situation where ASLR tries moves the stack of an executable, at which
> point you can no longer fork.

... but now that you mention the stack moving, yes, I could see that
being an issue.

Yes, the specific field where these flags are stored is called "DLL
Characteristics" so that is how it was referred to, but they do not
exclusively apply to DLLs.

https://docs.microsoft.com/en-us/windows/win32/debug/pe-format#dll-characteristics

[-- Attachment #2: Type: text/plain, Size: 4564 bytes --]

From 39bde2f11a1eb97503bae9cf15f5fc05640e5251 Mon Sep 17 00:00:00 2001
From: Jeremy Drake <sourceware-bugzilla@jdrake.com>
Date: Sun, 28 Feb 2021 15:49:08 -0800
Subject: [PATCH] ld: revert default dll characteristics for Cygwin.

Mail thread from
https://cygwin.com/pipermail/cygwin/2021-February/247922.html suggests
these flags will NOT work for Cygwin, which relies on stable address
layouts for their fork() emulation.

In the process, renamed move_default_addr_high shell variable to
cygwin_beahior, as the old name wasn't quite accurate anymore and I
wanted to use it choose which dll characteristics flags to use by
default.

Also copied that switch to pe.em, as it was only in pep.em before but
32-bit also needed to switch defaults for Cygwin.
---
 ld/emultempl/pe.em  | 13 ++++++++++++-
 ld/emultempl/pep.em | 40 +++++++++++++++++++++-------------------
 2 files changed, 33 insertions(+), 20 deletions(-)

diff --git a/ld/emultempl/pe.em b/ld/emultempl/pe.em
index 748a6b49412..9f757cc31dc 100644
--- a/ld/emultempl/pe.em
+++ b/ld/emultempl/pe.em
@@ -5,6 +5,16 @@ if [ -z "$MACHINE" ]; then
 else
   OUTPUT_ARCH=${ARCH}:${MACHINE}
 fi
+
+case ${target} in
+  *-*-cygwin*)
+    cygwin_behavior=1
+    ;;
+  *)
+    cygwin_behavior=0;
+    ;;
+esac
+
 rm -f e${EMULATION_NAME}.c
 (echo;echo;echo;echo;echo)>e${EMULATION_NAME}.c # there, now line numbers match ;-)
 fragment <<EOF
@@ -104,7 +114,8 @@ fragment <<EOF
 #define DEFAULT_PSEUDO_RELOC_VERSION 1
 #endif
 
-#define DEFAULT_DLL_CHARACTERISTICS	(IMAGE_DLL_CHARACTERISTICS_DYNAMIC_BASE \
+#define DEFAULT_DLL_CHARACTERISTICS	(${cygwin_behavior} ? 0 : \
+					   IMAGE_DLL_CHARACTERISTICS_DYNAMIC_BASE \
 					 | IMAGE_DLL_CHARACTERISTICS_NX_COMPAT)
 
 #if defined(TARGET_IS_i386pe) || ! defined(DLL_SUPPORT)
diff --git a/ld/emultempl/pep.em b/ld/emultempl/pep.em
index ff49c15c002..69f5bd04655 100644
--- a/ld/emultempl/pep.em
+++ b/ld/emultempl/pep.em
@@ -8,10 +8,10 @@ fi
 
 case ${target} in
   *-*-cygwin*)
-    move_default_addr_high=1
+    cygwin_behavior=1
     ;;
   *)
-    move_default_addr_high=0;
+    cygwin_behavior=0;
     ;;
 esac
 
@@ -99,45 +99,47 @@ fragment <<EOF
 #define DLL_SUPPORT
 #endif
 
-#define DEFAULT_DLL_CHARACTERISTICS	(IMAGE_DLL_CHARACTERISTICS_DYNAMIC_BASE \
+#define DEFAULT_DLL_CHARACTERISTICS	(${cygwin_behavior} ? 0 : \
+					   IMAGE_DLL_CHARACTERISTICS_DYNAMIC_BASE \
 					 | IMAGE_DLL_CHARACTERISTICS_HIGH_ENTROPY_VA \
 					 | IMAGE_DLL_CHARACTERISTICS_NX_COMPAT)
 
+
 #if defined(TARGET_IS_i386pep) || ! defined(DLL_SUPPORT)
 #define	PE_DEF_SUBSYSTEM		3
 #undef NT_EXE_IMAGE_BASE
 #define NT_EXE_IMAGE_BASE \
-  ((bfd_vma) (${move_default_addr_high} ? 0x100400000LL \
-					: 0x140000000LL))
+  ((bfd_vma) (${cygwin_behavior} ? 0x100400000LL \
+				 : 0x140000000LL))
 #undef NT_DLL_IMAGE_BASE
 #define NT_DLL_IMAGE_BASE \
-  ((bfd_vma) (${move_default_addr_high} ? 0x400000000LL \
-					: 0x180000000LL))
+  ((bfd_vma) (${cygwin_behavior} ? 0x400000000LL \
+				 : 0x180000000LL))
 #undef NT_DLL_AUTO_IMAGE_BASE
 #define NT_DLL_AUTO_IMAGE_BASE \
-  ((bfd_vma) (${move_default_addr_high} ? 0x400000000LL \
-					: 0x1C0000000LL))
+  ((bfd_vma) (${cygwin_behavior} ? 0x400000000LL \
+				 : 0x1C0000000LL))
 #undef NT_DLL_AUTO_IMAGE_MASK
 #define NT_DLL_AUTO_IMAGE_MASK \
-  ((bfd_vma) (${move_default_addr_high} ? 0x1ffff0000LL \
-					: 0x1ffff0000LL))
+  ((bfd_vma) (${cygwin_behavior} ? 0x1ffff0000LL \
+				 : 0x1ffff0000LL))
 #else
 #undef  NT_EXE_IMAGE_BASE
 #define NT_EXE_IMAGE_BASE \
-  ((bfd_vma) (${move_default_addr_high} ? 0x100010000LL \
-					: 0x10000LL))
+  ((bfd_vma) (${cygwin_behavior} ? 0x100010000LL \
+				 : 0x10000LL))
 #undef NT_DLL_IMAGE_BASE
 #define NT_DLL_IMAGE_BASE \
-  ((bfd_vma) (${move_default_addr_high} ? 0x110000000LL \
-					: 0x10000000LL))
+  ((bfd_vma) (${cygwin_behavior} ? 0x110000000LL \
+				 : 0x10000000LL))
 #undef NT_DLL_AUTO_IMAGE_BASE
 #define NT_DLL_AUTO_IMAGE_BASE \
-  ((bfd_vma) (${move_default_addr_high} ? 0x120000000LL \
-					: 0x61300000LL))
+  ((bfd_vma) (${cygwin_behavior} ? 0x120000000LL \
+				 : 0x61300000LL))
 #undef NT_DLL_AUTO_IMAGE_MASK
 #define NT_DLL_AUTO_IMAGE_MASK \
-  ((bfd_vma) (${move_default_addr_high} ? 0x0ffff0000LL \
-					: 0x0ffc0000LL))
+  ((bfd_vma) (${cygwin_behavior} ? 0x0ffff0000LL \
+				 : 0x0ffc0000LL))
 #undef  PE_DEF_SECTION_ALIGNMENT
 #define	PE_DEF_SUBSYSTEM		2
 #undef  PE_DEF_FILE_ALIGNMENT
-- 
2.30.1.windows.1

Re: cygwin + binutils 2.36 + ASLR/dynamicbase defaults

[ASSI]

Jeremy Drake via Cygwin writes:
> Well, Microsoft's LINK.EXE and LLVM's LLD have already been using these
> new defaults for some time.  But I was surprised how quickly my patch was
> accepted/merged.

That was not under dispute in any of the discussions I've seen on this
topic.

> To clarify, default base addresses should not have changed for cygwin
> targets, they were already above 4GB.

For 64bit, yes -- so I wasn't overly worried about that.

> I have a prelimiary patch that I plan to send upstream once I get some
> testing done on it, which reverts the default dll characteristics for
> cygwin targets.  I don't know if what you've done to 'nix it' for Cygwin
> was similar.

I've just done what makes it close enough to 2.35.2 in my view:

https://cygwin.com/git-cygwin-packages/?p=git/cygwin-packages/binutils.git;a=blob_plain;f=binutils-2.36.1-cygwin-peflags.patch

I don't want to carry such a patch for any longer time of course.

> I have not seen anything one way or the other on the NXCOMPAT flag.  Does
> that also needs to be reverted for Cygwin?

I don't know.  I think maybe not since it should announce itself pretty
early in testing, but things are always a bit more complicated than they
seem at first.

> I have seen the issues you described on 32-bit, but my understanding of
> how ASLR works suggested that it should be very rare on 64-bit.

That's the crux of the matter.  It fails when it produces a collision
and the chances of it doing that are going down exponentially with the
size of the address space.  But the target is not to have it fail
rarely, it must not fail at all.

> ... but now that you mention the stack moving, yes, I could see that
> being an issue.

Note that I don't have an idea if the stack really got moved or
something else mapped into the space formerly occupied by the stack.
Either way would be surprising since the case where it happened isn't
using any libraries besides cygwin1.dll.

> Yes, the specific field where these flags are stored is called "DLL
> Characteristics" so that is how it was referred to, but they do not
> exclusively apply to DLLs.
>
> https://docs.microsoft.com/en-us/windows/win32/debug/pe-format#dll-characteristics

Again, the question really is "what does this flag actually do for
executables"?  ASLR is not very well documented beyond the fact that it
exists and the odd blog post here and there as far as I can tell.  Its
been changed quite a bit over the time too, but which change went into
which release of Windows (or patches) is not documented, again AFAIK.


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

SD adaptations for Waldorf Q V3.00R3 and Q+ V3.54R2:
http://Synth.Stromeko.net/Downloads.html#WaldorfSDada