Re: [PATCH] binfmt_elf: Fix bug in loading of PIE binaries

[Sebastian Parschauer <s.parschauer@gmx.de>]

On 16.07.2015 22:34, Kees Cook wrote:
> On Thu, Jul 16, 2015 at 12:57 PM, Sebastian Parschauer
> <s.parschauer@gmx.de> wrote:
>> I'm a professional Linux game cheater and the co-maintainer of scanmem.
>> With scanmem we determine the load addresses for PIC and PIE binaries to
>> be able to support static memory cheating with ASLR. At the moment
>> ugtrain is the only universal game trainer able to determine the PIE
>> load address as well and to re-add it to the found match offset from
>> scanmem.
> 
> scanmem is such a fun tool. :)

Thank you very much for your kind words!

Scanmem and GameConqueror were especially very inactive tools before
I've pushed further development to lead game cheating on Linux. PIE made
all noob tools unusable and discontinued. :-)

>> I'd like to complain a bit about this patch as it makes the address
>> space layout for the executable really ugly by loading unrelated stuff
>> between .text and .rodata.
>>
>> Is it really required on top of 3.13 or 3.16 where Ubuntu has put it?
>>
>> I've also checked v4.2-rc1. There everything is beautiful again.
>> Thank you very much for that!
>>
>> References:
>> https://github.com/scanmem/scanmem/issues/122
>> https://github.com/ugtrain/ugtrain
> 
> To summarize the two commits:
> 
> This commit fixed a problem where PIE binaries could collide with
> other memory regions, but breaks scanmem:
> https://git.kernel.org/linus/a87938b2e246b81b4fb713edb371a9fa3c5c3c86
> It was sent to stable (and various distros like Ubuntu picked it up),
> since it was (correctly) seen as a bug fix (without realizing it broke
> other programs).
> 
> This commit reorganized PIE ASLR to split it from mmap ASLR:
> https://git.kernel.org/linus/a87938b2e246b81b4fb713edb371a9fa3c5c3c86
> This was part of a larger series of patches that did refactoring
> across multiple architectures, and was not sent to stable (since it is
> considered more of a feature than a bug fix).
> 
> I believe it should be safe to backport the series, but not every
> kernel team will be willing to do that on their own. As the series has
> been living fine since 4.1, I could be convinced that it should be
> sent to stable. It does fix an ASLR weakness, and it does fix a
> situation where the other commit that got sent to stable breaks
> existing userspace tools. I think both of those factors together
> warrants sending the series to stable.
> 
> Greg, would you be willing to take these into stable?
> 
> fbbc400f3924ce095b466c776dc294727ec0a202
> 82168140bc4cec7ec9bad39705518541149ff8b7
> dd04cff1dceab18226853b555cf07914648a235f
> 1f0569df0b0285e7ec2432d804a4921b06a61618
> ed6322746afb74c2509e2f3a6464182793b16eb9
> 8e89a356feb6f196824a72101861d931a97ac2d2
> 2b68f6caeac271620cd2f9362aeaed360e317df0
> c6f5b001e65cdac592b65a08c5d2dd179cfba568
> d1fd836dcf00d2028c700c7e44d2c23404062c90
> 204db6ed17743000691d930368a5abd6ea541c58

Thank you very much for your efforts!

Backporting the patch set to 4.0.y was no problem but I ran into major
issues backporting it to 3.18.y. It plain touches too many archs and too
many other patches would be required additionally.
The easiest and most stable way would be to just create a completely new
patch to "beautify the ugly pie" but the linux-stable rules don't allow
this.

"It or an equivalent fix must already exist in Linus' tree (upstream)."

So I guess we have to "eat the ugly pie" and fix it in scanmem where the
complexity literally explodes. PIC/PIE regions detection is already
complex and has already a long explanation text in the source. Let's see
if I can explain the support for ugly PIE I've developed for scanmem to
the original maintainer Lu Wang to get it merged.

For the future please always keep the four ELF segments .text, .rodata,
.data and .bss together in memory so that we don't run into this
situation again. Thanks!

Cheers,
Sebastian