Skip to contentSkip to conversation

binfmt_elf: Fix bug in loading of PIE binaries.

PATCH In review Michael Davidson 5 participants 6 messages Apr 13, 2015 – Aug 8, 2015

Subsystems (2)
VFS exec and binfmt
lore
Share & tools
lore

Michael Davidson

author
#0 lore raw

PATCH 1/1 binfmt_elf: Fix bug in loading of PIE binaries.

With CONFIG_ARCH_BINFMT_ELF_RANDOMIZE_PIE enabled, and a normal top-down address allocation strategy, load_elf_binary() will attempt to map a PIE binary into an address range immediately below mm->mmap_base.

Unfortunately, load_elf_ binary() does not take account of the need to allocate sufficient space for the entire binary which means that, while the first PT_LOAD segment is mapped below mm->mmap_base, the subsequent PT_LOAD segment(s) end up being mapped above mm->mmap_base into the are that is supposed to be the "gap" between the stack and the binary.

Since the size of the "gap" on x86_64 is only guaranteed to be 128MB this means that binaries with large data segments > 128MB can end up mapping part of their data segment over their stack resulting in corruption of the stack (and the data segment once the binary starts to run).

Any PIE binary with a data segment > 128MB is vulnerable to this although address randomization means that the actual gap between the stack and the end of the binary is normally greater than 128MB. The larger the data segment of the binary the higher the probability of failure.

Fix this by calculating the total size of the binary in the same way as load_elf_interp().

Signed-off-by Michael Davidson <[email protected]>
---
 fs/binfmt_elf.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)
fs/binfmt_elf.c +8 -1
index 995986b..d925f55 100644
@@ -862,6 +862,7 @@ static int load_elf_binary(struct linux_binprm *bprm)
 	    i < loc->elf_ex.e_phnum; i++, elf_ppnt++) {
 		int elf_prot = 0, elf_flags;
 		unsigned long k, vaddr;
+		unsigned long total_size = 0;
 
 		if (elf_ppnt->p_type != PT_LOAD)
 			continue;

@@ -924,10 +925,16 @@ static int load_elf_binary(struct linux_binprm *bprm)
 #else
 			load_bias = ELF_PAGESTART(ELF_ET_DYN_BASE - vaddr);
 #endif
+			total_size = total_mapping_size(elf_phdata,
+							loc->elf_ex.e_phnum);
+			if (!total_size) {
+				error = -EINVAL;
+				goto out_free_dentry;
+			}
 		}
 
 		error = elf_map(bprm->file, load_bias + vaddr, elf_ppnt,
-				elf_prot, elf_flags, 0);
+				elf_prot, elf_flags, total_size);
 		if (BAD_ADDR(error)) {
 			retval = IS_ERR((void *)error) ?
 				PTR_ERR((void*)error) : -EINVAL;
Signature · 1 line
2.2.0.rc0.207.ga3a616c
#1 lore raw

Hi Michael,

Quoted 45 lines · Michael Davidson

With CONFIG_ARCH_BINFMT_ELF_RANDOMIZE_PIE enabled, and a normal top-down address allocation strategy, load_elf_binary() will attempt to map a PIE binary into an address range immediately below mm->mmap_base.

Unfortunately, load_elf_ binary() does not take account of the need to allocate sufficient space for the entire binary which means that, while the first PT_LOAD segment is mapped below mm->mmap_base, the subsequent PT_LOAD segment(s) end up being mapped above mm->mmap_base into the are that is supposed to be the "gap" between the stack and the binary.

Since the size of the "gap" on x86_64 is only guaranteed to be 128MB this means that binaries with large data segments > 128MB can end up mapping part of their data segment over their stack resulting in corruption of the stack (and the data segment once the binary starts to run).

Any PIE binary with a data segment > 128MB is vulnerable to this although address randomization means that the actual gap between the stack and the end of the binary is normally greater than 128MB. The larger the data segment of the binary the higher the probability of failure.

Fix this by calculating the total size of the binary in the same way as load_elf_interp().

Signed-off-by Michael Davidson <[email protected]>
---
 fs/binfmt_elf.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)
fs/binfmt_elf.c +1 -0
index 995986b..d925f55 100644
@@ -862,6 +862,7 @@ static int load_elf_binary(struct linux_binprm *bprm)
            i < loc->elf_ex.e_phnum; i++, elf_ppnt++) {
                int elf_prot = 0, elf_flags;
                unsigned long k, vaddr;
+               unsigned long total_size = 0;
 
                if (elf_ppnt->p_type != PT_LOAD)
                        continue;

fs/binfmt_elf.c static int load_elf_binary(struct linux_binprm *bprm) · line 931

Quoted 7 lines
fs/binfmt_elf.c +4 -0
@@ -924,10 +925,16 @@ static int load_elf_binary(struct linux_binprm *bprm)
 #else
                        load_bias = ELF_PAGESTART(ELF_ET_DYN_BASE - vaddr);
 #endif
+                       total_size = total_mapping_size(elf_phdata,
+                                                       loc->elf_ex.e_phnum);
+                       if (!total_size) {
+                               error = -EINVAL;

I was just printk debugging this function, and this stood out. Should that be retval instead of error?

Cheers
James

Quoted 18 lines
fs/binfmt_elf.c +3 -2
+                               goto out_free_dentry;
+                       }
                }
 
                error = elf_map(bprm->file, load_bias + vaddr, elf_ppnt,
-                               elf_prot, elf_flags, 0);
+                               elf_prot, elf_flags, total_size);
                if (BAD_ADDR(error)) {
                        retval = IS_ERR((void *)error) ?
                                PTR_ERR((void*)error) : -EINVAL;
--

2.2.0.rc0.207.ga3a616c

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

#2 lore raw

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

In reply to Michael Davidson

Hi,

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.

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

Thanks,
Sebastian

#3 lore raw

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

Quoted 6 lines · Sebastian Parschauer

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. :)

Quoted 4 lines

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.

Quoted 8 lines

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

Thanks!

-Kees

Signature · 2 lines
Kees Cook
Chrome OS Security
#4 lore raw

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

Quoted 10 lines · Kees Cook
Quoted 6 lines · Sebastian Parschauer

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. :-)

Quoted 38 lines
Quoted 4 lines

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.

Quoted 8 lines

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?

Quoted 10 lines

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

#5 lore raw

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

In reply to Kees Cook
Quoted 49 lines · Kees Cook
Quoted 6 lines · Sebastian Parschauer

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. :)

Quoted 4 lines

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.

Quoted 8 lines

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?

Quoted 10 lines

fbbc400f3924ce095b466c776dc294727ec0a202
82168140bc4cec7ec9bad39705518541149ff8b7
dd04cff1dceab18226853b555cf07914648a235f
1f0569df0b0285e7ec2432d804a4921b06a61618
ed6322746afb74c2509e2f3a6464182793b16eb9
8e89a356feb6f196824a72101861d931a97ac2d2
2b68f6caeac271620cd2f9362aeaed360e317df0
c6f5b001e65cdac592b65a08c5d2dd179cfba568
d1fd836dcf00d2028c700c7e44d2c23404062c90
204db6ed17743000691d930368a5abd6ea541c58

Given that 4.0-stable is now end-of-life, and these are all in 4.1, I'm not going to worry about these anymore, thanks.

greg k-h