On 32-bit powerpc the ELF PLT sections of binaries (built with --bss-plt,
or with a toolchain which defaults to it) look like this:
[17] .sbss NOBITS 0002aff8 01aff8 000014 00 WA 0 0 4
[18] .plt NOBITS 0002b00c 01aff8 000084 00 WAX 0 0 4
[19] .bss NOBITS 0002b090 01aff8 0000a4 00 WA 0 0 4
Which results in an ELF load header:
Type Offset VirtAddr PhysAddr FileSiz MemSiz Flg Align
LOAD 0x019c70 0x00029c70 0x00029c70 0x01388 0x014c4 RWE 0x10000
This is all correct, the load region containing the PLT is marked as
executable. Note that the PLT starts at 0002b00c but the file mapping ends at
0002aff8, so the PLT falls in the 0 fill section described by the load header,
and after a page boundary.
Unfortunately the generic ELF loader ignores the X bit in the load headers
when it creates the 0 filled non-file backed mappings. It assumes all of these
mappings are RW BSS sections, which is not the case for PPC.
gcc/ld has an option (--secure-plt) to not do this, this is said to incur
a small performance penalty.
Currently, to support 32-bit binaries with PLT in BSS kernel maps *entire
brk area* with executable rights for all binaries, even --secure-plt ones.
Stop doing that.
Teach the ELF loader to check the X bit in the relevant load header
and create 0 filled anonymous mappings that are executable
if the load header requests that.
The patch was originally posted in 2012 by Jason Gunthorpe
and apparently ignored:
https://lkml.org/lkml/2012/9/30/138
Lightly run-tested.
Signed-off-by: Jason Gunthorpe <[email protected]>
Signed-off-by: Denys Vlasenko <[email protected]>
Acked-by: Kees Cook <[email protected]>
Acked-by: Michael Ellerman <[email protected]>
Tested-by: Jason Gunthorpe <[email protected]>
CC: Andrew Morton <[email protected]>
CC: Benjamin Herrenschmidt <[email protected]>
CC: Paul Mackerras <[email protected]>
CC: "Aneesh Kumar K.V" <[email protected]>
CC: Kees Cook <[email protected]>
CC: Oleg Nesterov <[email protected]>
CC: Michael Ellerman <[email protected]>
CC: Florian Weimer <[email protected]>
CC: [email protected]
CC: [email protected]
CC: [email protected]
---
Changes since v6:
* rebased to current Linus tree
* sending to akpm
Changes since v5:
* made do_brk_flags() error out if any bits other than VM_EXEC are set.
(Kees Cook: "With this, I'd be happy to Ack.")
See https://patchwork.ozlabs.org/patch/661595/
Changes since v4:
* if (current->personality & READ_IMPLIES_EXEC), still use VM_EXEC
for 32-bit executables.
Changes since v3:
* typo fix in commit message
* rebased to current Linus tree
Changes since v2:
* moved capability to map with VM_EXEC into vm_brk_flags()
Changes since v1:
* wrapped lines to not exceed 79 chars
* improved comment
* expanded CC list
arch/powerpc/include/asm/page.h | 4 +++-
fs/binfmt_elf.c | 30 ++++++++++++++++++++++--------
include/linux/mm.h | 1 +
mm/mmap.c | 24 +++++++++++++++++++-----
4 files changed, 45 insertions(+), 14 deletions(-)
diff --git a/arch/powerpc/include/asm/page.h b/arch/powerpc/include/asm/page.h
index 56398e7..17d3d2c 100644
--- a/arch/powerpc/include/asm/page.h
+++ b/arch/powerpc/include/asm/page.h
@@ -230,7 +230,9 @@ extern long long virt_phys_offset;
* and needs to be executable. This means the whole heap ends
* up being executable.
*/
-#define VM_DATA_DEFAULT_FLAGS32 (VM_READ | VM_WRITE | VM_EXEC | \
+#define VM_DATA_DEFAULT_FLAGS32 \
+ (((current->personality & READ_IMPLIES_EXEC) ? VM_EXEC : 0) | \
+ VM_READ | VM_WRITE | \
VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
#define VM_DATA_DEFAULT_FLAGS64 (VM_READ | VM_WRITE | \
diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index 2472af2..065134b 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -91,12 +91,18 @@ static struct linux_binfmt elf_format = {
#define BAD_ADDR(x) ((unsigned long)(x) >= TASK_SIZE)
-static int set_brk(unsigned long start, unsigned long end)
+static int set_brk(unsigned long start, unsigned long end, int prot)
{
start = ELF_PAGEALIGN(start);
end = ELF_PAGEALIGN(end);
if (end > start) {
- int error = vm_brk(start, end - start);
+ /*
+ * Map the last of the bss segment.
+ * If the header is requesting these pages to be
+ * executable, honour that (ppc32 needs this).
+ */
+ int error = vm_brk_flags(start, end - start,
+ prot & PROT_EXEC ? VM_EXEC : 0);
if (error)
return error;
}
@@ -524,6 +530,7 @@ static unsigned long load_elf_interp(struct elfhdr *interp_elf_ex,
unsigned long load_addr = 0;
int load_addr_set = 0;
unsigned long last_bss = 0, elf_bss = 0;
+ int bss_prot = 0;
unsigned long error = ~0UL;
unsigned long total_size;
int i;
@@ -606,8 +613,10 @@ static unsigned long load_elf_interp(struct elfhdr *interp_elf_ex,
* elf_bss and last_bss is the bss section.
*/
k = load_addr + eppnt->p_vaddr + eppnt->p_memsz;
- if (k > last_bss)
+ if (k > last_bss) {
last_bss = k;
+ bss_prot = elf_prot;
+ }
}
}
@@ -623,13 +632,14 @@ static unsigned long load_elf_interp(struct elfhdr *interp_elf_ex,
/*
* Next, align both the file and mem bss up to the page size,
* since this is where elf_bss was just zeroed up to, and where
- * last_bss will end after the vm_brk() below.
+ * last_bss will end after the vm_brk_flags() below.
*/
elf_bss = ELF_PAGEALIGN(elf_bss);
last_bss = ELF_PAGEALIGN(last_bss);
/* Finally, if there is still more bss to allocate, do it. */
if (last_bss > elf_bss) {
- error = vm_brk(elf_bss, last_bss - elf_bss);
+ error = vm_brk_flags(elf_bss, last_bss - elf_bss,
+ bss_prot & PROT_EXEC ? VM_EXEC : 0);
if (error)
goto out;
}
@@ -674,6 +684,7 @@ static int load_elf_binary(struct linux_binprm *bprm)
unsigned long error;
struct elf_phdr *elf_ppnt, *elf_phdata, *interp_elf_phdata = NULL;
unsigned long elf_bss, elf_brk;
+ int bss_prot = 0;
int retval, i;
unsigned long elf_entry;
unsigned long interp_load_addr = 0;
@@ -882,7 +893,8 @@ static int load_elf_binary(struct linux_binprm *bprm)
before this one. Map anonymous pages, if needed,
and clear the area. */
retval = set_brk(elf_bss + load_bias,
- elf_brk + load_bias);
+ elf_brk + load_bias,
+ bss_prot);
if (retval)
goto out_free_dentry;
nbyte = ELF_PAGEOFFSET(elf_bss);
@@ -976,8 +988,10 @@ static int load_elf_binary(struct linux_binprm *bprm)
if (end_data < k)
end_data = k;
k = elf_ppnt->p_vaddr + elf_ppnt->p_memsz;
- if (k > elf_brk)
+ if (k > elf_brk) {
+ bss_prot = elf_prot;
elf_brk = k;
+ }
}
loc->elf_ex.e_entry += load_bias;
@@ -993,7 +1007,7 @@ static int load_elf_binary(struct linux_binprm *bprm)
* mapping in the interpreter, to make sure it doesn't wind
* up getting placed where the bss needs to go.
*/
- retval = set_brk(elf_bss, elf_brk);
+ retval = set_brk(elf_bss, elf_brk, bss_prot);
if (retval)
goto out_free_dentry;
if (likely(elf_bss != elf_brk) && unlikely(padzero(elf_bss))) {
diff --git a/include/linux/mm.h b/include/linux/mm.h
index a92c8d7..2ac7e5e 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -2053,6 +2053,7 @@ static inline void mm_populate(unsigned long addr, unsigned long len) {}
/* These take the mm semaphore themselves */
extern int __must_check vm_brk(unsigned long, unsigned long);
+extern int __must_check vm_brk_flags(unsigned long, unsigned long, unsigned long);
extern int vm_munmap(unsigned long, size_t);
extern unsigned long __must_check vm_mmap(struct file *, unsigned long,
unsigned long, unsigned long,
diff --git a/mm/mmap.c b/mm/mmap.c
index 1af87c1..4adcf2c 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -2806,11 +2806,11 @@ static inline void verify_mm_writelocked(struct mm_struct *mm)
* anonymous maps. eventually we may be able to do some
* brk-specific accounting here.
*/
-static int do_brk(unsigned long addr, unsigned long request)
+static int do_brk_flags(unsigned long addr, unsigned long request, unsigned long flags)
{
struct mm_struct *mm = current->mm;
struct vm_area_struct *vma, *prev;
- unsigned long flags, len;
+ unsigned long len;
struct rb_node **rb_link, *rb_parent;
pgoff_t pgoff = addr >> PAGE_SHIFT;
int error;
@@ -2821,7 +2821,10 @@ static int do_brk(unsigned long addr, unsigned long request)
if (!len)
return 0;
- flags = VM_DATA_DEFAULT_FLAGS | VM_ACCOUNT | mm->def_flags;
+ /* Until we need other flags, refuse anything except VM_EXEC. */
+ if ((flags & (~VM_EXEC)) != 0)
+ return -EINVAL;
+ flags |= VM_DATA_DEFAULT_FLAGS | VM_ACCOUNT | mm->def_flags;
error = get_unmapped_area(NULL, addr, len, 0, MAP_FIXED);
if (offset_in_page(error))
@@ -2889,7 +2892,12 @@ static int do_brk(unsigned long addr, unsigned long request)
return 0;
}
-int vm_brk(unsigned long addr, unsigned long len)
+static int do_brk(unsigned long addr, unsigned long len)
+{
+ return do_brk_flags(addr, len, 0);
+}
+
+int vm_brk_flags(unsigned long addr, unsigned long len, unsigned long flags)
{
struct mm_struct *mm = current->mm;
int ret;
@@ -2898,13 +2906,19 @@ int vm_brk(unsigned long addr, unsigned long len)
if (down_write_killable(&mm->mmap_sem))
return -EINTR;
- ret = do_brk(addr, len);
+ ret = do_brk_flags(addr, len, flags);
populate = ((mm->def_flags & VM_LOCKED) != 0);
up_write(&mm->mmap_sem);
if (populate && !ret)
mm_populate(addr, len);
return ret;
}
+EXPORT_SYMBOL(vm_brk_flags);
+
+int vm_brk(unsigned long addr, unsigned long len)
+{
+ return vm_brk_flags(addr, len, 0);
+}
EXPORT_SYMBOL(vm_brk);
/* Release all mmaps. */
--
2.9.2
On Wed, Nov 9, 2016 at 9:06 AM, Denys Vlasenko <[email protected]> wrote:
> On 32-bit powerpc the ELF PLT sections of binaries (built with --bss-plt,
> or with a toolchain which defaults to it) look like this:
>
> [17] .sbss NOBITS 0002aff8 01aff8 000014 00 WA 0 0 4
> [18] .plt NOBITS 0002b00c 01aff8 000084 00 WAX 0 0 4
> [19] .bss NOBITS 0002b090 01aff8 0000a4 00 WA 0 0 4
>
> Which results in an ELF load header:
>
> Type Offset VirtAddr PhysAddr FileSiz MemSiz Flg Align
> LOAD 0x019c70 0x00029c70 0x00029c70 0x01388 0x014c4 RWE 0x10000
>
> This is all correct, the load region containing the PLT is marked as
> executable. Note that the PLT starts at 0002b00c but the file mapping ends at
> 0002aff8, so the PLT falls in the 0 fill section described by the load header,
> and after a page boundary.
>
> Unfortunately the generic ELF loader ignores the X bit in the load headers
> when it creates the 0 filled non-file backed mappings. It assumes all of these
> mappings are RW BSS sections, which is not the case for PPC.
>
> gcc/ld has an option (--secure-plt) to not do this, this is said to incur
> a small performance penalty.
>
> Currently, to support 32-bit binaries with PLT in BSS kernel maps *entire
> brk area* with executable rights for all binaries, even --secure-plt ones.
>
> Stop doing that.
>
> Teach the ELF loader to check the X bit in the relevant load header
> and create 0 filled anonymous mappings that are executable
> if the load header requests that.
>
> The patch was originally posted in 2012 by Jason Gunthorpe
> and apparently ignored:
>
> https://lkml.org/lkml/2012/9/30/138
>
> Lightly run-tested.
>
> Signed-off-by: Jason Gunthorpe <[email protected]>
> Signed-off-by: Denys Vlasenko <[email protected]>
> Acked-by: Kees Cook <[email protected]>
> Acked-by: Michael Ellerman <[email protected]>
> Tested-by: Jason Gunthorpe <[email protected]>
> CC: Andrew Morton <[email protected]>
> CC: Benjamin Herrenschmidt <[email protected]>
> CC: Paul Mackerras <[email protected]>
> CC: "Aneesh Kumar K.V" <[email protected]>
> CC: Kees Cook <[email protected]>
> CC: Oleg Nesterov <[email protected]>
> CC: Michael Ellerman <[email protected]>
> CC: Florian Weimer <[email protected]>
> CC: [email protected]
> CC: [email protected]
> CC: [email protected]
Hi,
Friendly ping to Andrew... I'd like to get this landed in -mm...
-Kees
> ---
> Changes since v6:
> * rebased to current Linus tree
> * sending to akpm
>
> Changes since v5:
> * made do_brk_flags() error out if any bits other than VM_EXEC are set.
> (Kees Cook: "With this, I'd be happy to Ack.")
> See https://patchwork.ozlabs.org/patch/661595/
>
> Changes since v4:
> * if (current->personality & READ_IMPLIES_EXEC), still use VM_EXEC
> for 32-bit executables.
>
> Changes since v3:
> * typo fix in commit message
> * rebased to current Linus tree
>
> Changes since v2:
> * moved capability to map with VM_EXEC into vm_brk_flags()
>
> Changes since v1:
> * wrapped lines to not exceed 79 chars
> * improved comment
> * expanded CC list
>
> arch/powerpc/include/asm/page.h | 4 +++-
> fs/binfmt_elf.c | 30 ++++++++++++++++++++++--------
> include/linux/mm.h | 1 +
> mm/mmap.c | 24 +++++++++++++++++++-----
> 4 files changed, 45 insertions(+), 14 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/page.h b/arch/powerpc/include/asm/page.h
> index 56398e7..17d3d2c 100644
> --- a/arch/powerpc/include/asm/page.h
> +++ b/arch/powerpc/include/asm/page.h
> @@ -230,7 +230,9 @@ extern long long virt_phys_offset;
> * and needs to be executable. This means the whole heap ends
> * up being executable.
> */
> -#define VM_DATA_DEFAULT_FLAGS32 (VM_READ | VM_WRITE | VM_EXEC | \
> +#define VM_DATA_DEFAULT_FLAGS32 \
> + (((current->personality & READ_IMPLIES_EXEC) ? VM_EXEC : 0) | \
> + VM_READ | VM_WRITE | \
> VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
>
> #define VM_DATA_DEFAULT_FLAGS64 (VM_READ | VM_WRITE | \
> diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
> index 2472af2..065134b 100644
> --- a/fs/binfmt_elf.c
> +++ b/fs/binfmt_elf.c
> @@ -91,12 +91,18 @@ static struct linux_binfmt elf_format = {
>
> #define BAD_ADDR(x) ((unsigned long)(x) >= TASK_SIZE)
>
> -static int set_brk(unsigned long start, unsigned long end)
> +static int set_brk(unsigned long start, unsigned long end, int prot)
> {
> start = ELF_PAGEALIGN(start);
> end = ELF_PAGEALIGN(end);
> if (end > start) {
> - int error = vm_brk(start, end - start);
> + /*
> + * Map the last of the bss segment.
> + * If the header is requesting these pages to be
> + * executable, honour that (ppc32 needs this).
> + */
> + int error = vm_brk_flags(start, end - start,
> + prot & PROT_EXEC ? VM_EXEC : 0);
> if (error)
> return error;
> }
> @@ -524,6 +530,7 @@ static unsigned long load_elf_interp(struct elfhdr *interp_elf_ex,
> unsigned long load_addr = 0;
> int load_addr_set = 0;
> unsigned long last_bss = 0, elf_bss = 0;
> + int bss_prot = 0;
> unsigned long error = ~0UL;
> unsigned long total_size;
> int i;
> @@ -606,8 +613,10 @@ static unsigned long load_elf_interp(struct elfhdr *interp_elf_ex,
> * elf_bss and last_bss is the bss section.
> */
> k = load_addr + eppnt->p_vaddr + eppnt->p_memsz;
> - if (k > last_bss)
> + if (k > last_bss) {
> last_bss = k;
> + bss_prot = elf_prot;
> + }
> }
> }
>
> @@ -623,13 +632,14 @@ static unsigned long load_elf_interp(struct elfhdr *interp_elf_ex,
> /*
> * Next, align both the file and mem bss up to the page size,
> * since this is where elf_bss was just zeroed up to, and where
> - * last_bss will end after the vm_brk() below.
> + * last_bss will end after the vm_brk_flags() below.
> */
> elf_bss = ELF_PAGEALIGN(elf_bss);
> last_bss = ELF_PAGEALIGN(last_bss);
> /* Finally, if there is still more bss to allocate, do it. */
> if (last_bss > elf_bss) {
> - error = vm_brk(elf_bss, last_bss - elf_bss);
> + error = vm_brk_flags(elf_bss, last_bss - elf_bss,
> + bss_prot & PROT_EXEC ? VM_EXEC : 0);
> if (error)
> goto out;
> }
> @@ -674,6 +684,7 @@ static int load_elf_binary(struct linux_binprm *bprm)
> unsigned long error;
> struct elf_phdr *elf_ppnt, *elf_phdata, *interp_elf_phdata = NULL;
> unsigned long elf_bss, elf_brk;
> + int bss_prot = 0;
> int retval, i;
> unsigned long elf_entry;
> unsigned long interp_load_addr = 0;
> @@ -882,7 +893,8 @@ static int load_elf_binary(struct linux_binprm *bprm)
> before this one. Map anonymous pages, if needed,
> and clear the area. */
> retval = set_brk(elf_bss + load_bias,
> - elf_brk + load_bias);
> + elf_brk + load_bias,
> + bss_prot);
> if (retval)
> goto out_free_dentry;
> nbyte = ELF_PAGEOFFSET(elf_bss);
> @@ -976,8 +988,10 @@ static int load_elf_binary(struct linux_binprm *bprm)
> if (end_data < k)
> end_data = k;
> k = elf_ppnt->p_vaddr + elf_ppnt->p_memsz;
> - if (k > elf_brk)
> + if (k > elf_brk) {
> + bss_prot = elf_prot;
> elf_brk = k;
> + }
> }
>
> loc->elf_ex.e_entry += load_bias;
> @@ -993,7 +1007,7 @@ static int load_elf_binary(struct linux_binprm *bprm)
> * mapping in the interpreter, to make sure it doesn't wind
> * up getting placed where the bss needs to go.
> */
> - retval = set_brk(elf_bss, elf_brk);
> + retval = set_brk(elf_bss, elf_brk, bss_prot);
> if (retval)
> goto out_free_dentry;
> if (likely(elf_bss != elf_brk) && unlikely(padzero(elf_bss))) {
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index a92c8d7..2ac7e5e 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -2053,6 +2053,7 @@ static inline void mm_populate(unsigned long addr, unsigned long len) {}
>
> /* These take the mm semaphore themselves */
> extern int __must_check vm_brk(unsigned long, unsigned long);
> +extern int __must_check vm_brk_flags(unsigned long, unsigned long, unsigned long);
> extern int vm_munmap(unsigned long, size_t);
> extern unsigned long __must_check vm_mmap(struct file *, unsigned long,
> unsigned long, unsigned long,
> diff --git a/mm/mmap.c b/mm/mmap.c
> index 1af87c1..4adcf2c 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -2806,11 +2806,11 @@ static inline void verify_mm_writelocked(struct mm_struct *mm)
> * anonymous maps. eventually we may be able to do some
> * brk-specific accounting here.
> */
> -static int do_brk(unsigned long addr, unsigned long request)
> +static int do_brk_flags(unsigned long addr, unsigned long request, unsigned long flags)
> {
> struct mm_struct *mm = current->mm;
> struct vm_area_struct *vma, *prev;
> - unsigned long flags, len;
> + unsigned long len;
> struct rb_node **rb_link, *rb_parent;
> pgoff_t pgoff = addr >> PAGE_SHIFT;
> int error;
> @@ -2821,7 +2821,10 @@ static int do_brk(unsigned long addr, unsigned long request)
> if (!len)
> return 0;
>
> - flags = VM_DATA_DEFAULT_FLAGS | VM_ACCOUNT | mm->def_flags;
> + /* Until we need other flags, refuse anything except VM_EXEC. */
> + if ((flags & (~VM_EXEC)) != 0)
> + return -EINVAL;
> + flags |= VM_DATA_DEFAULT_FLAGS | VM_ACCOUNT | mm->def_flags;
>
> error = get_unmapped_area(NULL, addr, len, 0, MAP_FIXED);
> if (offset_in_page(error))
> @@ -2889,7 +2892,12 @@ static int do_brk(unsigned long addr, unsigned long request)
> return 0;
> }
>
> -int vm_brk(unsigned long addr, unsigned long len)
> +static int do_brk(unsigned long addr, unsigned long len)
> +{
> + return do_brk_flags(addr, len, 0);
> +}
> +
> +int vm_brk_flags(unsigned long addr, unsigned long len, unsigned long flags)
> {
> struct mm_struct *mm = current->mm;
> int ret;
> @@ -2898,13 +2906,19 @@ int vm_brk(unsigned long addr, unsigned long len)
> if (down_write_killable(&mm->mmap_sem))
> return -EINTR;
>
> - ret = do_brk(addr, len);
> + ret = do_brk_flags(addr, len, flags);
> populate = ((mm->def_flags & VM_LOCKED) != 0);
> up_write(&mm->mmap_sem);
> if (populate && !ret)
> mm_populate(addr, len);
> return ret;
> }
> +EXPORT_SYMBOL(vm_brk_flags);
> +
> +int vm_brk(unsigned long addr, unsigned long len)
> +{
> + return vm_brk_flags(addr, len, 0);
> +}
> EXPORT_SYMBOL(vm_brk);
>
> /* Release all mmaps. */
> --
> 2.9.2
>
--
Kees Cook
Nexus Security
On Wed, Nov 9, 2016 at 9:06 AM, Denys Vlasenko <[email protected]> wrote:
> On 32-bit powerpc the ELF PLT sections of binaries (built with --bss-plt,
> or with a toolchain which defaults to it) look like this:
>
> [17] .sbss NOBITS 0002aff8 01aff8 000014 00 WA 0 0 4
> [18] .plt NOBITS 0002b00c 01aff8 000084 00 WAX 0 0 4
> [19] .bss NOBITS 0002b090 01aff8 0000a4 00 WA 0 0 4
>
> Which results in an ELF load header:
>
> Type Offset VirtAddr PhysAddr FileSiz MemSiz Flg Align
> LOAD 0x019c70 0x00029c70 0x00029c70 0x01388 0x014c4 RWE 0x10000
>
> This is all correct, the load region containing the PLT is marked as
> executable. Note that the PLT starts at 0002b00c but the file mapping ends at
> 0002aff8, so the PLT falls in the 0 fill section described by the load header,
> and after a page boundary.
>
> Unfortunately the generic ELF loader ignores the X bit in the load headers
> when it creates the 0 filled non-file backed mappings. It assumes all of these
> mappings are RW BSS sections, which is not the case for PPC.
>
> gcc/ld has an option (--secure-plt) to not do this, this is said to incur
> a small performance penalty.
>
> Currently, to support 32-bit binaries with PLT in BSS kernel maps *entire
> brk area* with executable rights for all binaries, even --secure-plt ones.
Can you resend this patch with /proc/$pid/maps output showing the
before/after effects of this change also included here? Showing that
reduction in executable mapping should illustrate the benefit of
avoiding having the execute bit set on the brk area (which is a clear
security weakness, having writable/executable code in the process's
memory segment, especially when it was _not_ requested by the ELF
headers).
Thanks!
-Kees
>
> Stop doing that.
>
> Teach the ELF loader to check the X bit in the relevant load header
> and create 0 filled anonymous mappings that are executable
> if the load header requests that.
>
> The patch was originally posted in 2012 by Jason Gunthorpe
> and apparently ignored:
>
> https://lkml.org/lkml/2012/9/30/138
>
> Lightly run-tested.
>
> Signed-off-by: Jason Gunthorpe <[email protected]>
> Signed-off-by: Denys Vlasenko <[email protected]>
> Acked-by: Kees Cook <[email protected]>
> Acked-by: Michael Ellerman <[email protected]>
> Tested-by: Jason Gunthorpe <[email protected]>
> CC: Andrew Morton <[email protected]>
> CC: Benjamin Herrenschmidt <[email protected]>
> CC: Paul Mackerras <[email protected]>
> CC: "Aneesh Kumar K.V" <[email protected]>
> CC: Kees Cook <[email protected]>
> CC: Oleg Nesterov <[email protected]>
> CC: Michael Ellerman <[email protected]>
> CC: Florian Weimer <[email protected]>
> CC: [email protected]
> CC: [email protected]
> CC: [email protected]
> ---
> Changes since v6:
> * rebased to current Linus tree
> * sending to akpm
>
> Changes since v5:
> * made do_brk_flags() error out if any bits other than VM_EXEC are set.
> (Kees Cook: "With this, I'd be happy to Ack.")
> See https://patchwork.ozlabs.org/patch/661595/
>
> Changes since v4:
> * if (current->personality & READ_IMPLIES_EXEC), still use VM_EXEC
> for 32-bit executables.
>
> Changes since v3:
> * typo fix in commit message
> * rebased to current Linus tree
>
> Changes since v2:
> * moved capability to map with VM_EXEC into vm_brk_flags()
>
> Changes since v1:
> * wrapped lines to not exceed 79 chars
> * improved comment
> * expanded CC list
>
> arch/powerpc/include/asm/page.h | 4 +++-
> fs/binfmt_elf.c | 30 ++++++++++++++++++++++--------
> include/linux/mm.h | 1 +
> mm/mmap.c | 24 +++++++++++++++++++-----
> 4 files changed, 45 insertions(+), 14 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/page.h b/arch/powerpc/include/asm/page.h
> index 56398e7..17d3d2c 100644
> --- a/arch/powerpc/include/asm/page.h
> +++ b/arch/powerpc/include/asm/page.h
> @@ -230,7 +230,9 @@ extern long long virt_phys_offset;
> * and needs to be executable. This means the whole heap ends
> * up being executable.
> */
> -#define VM_DATA_DEFAULT_FLAGS32 (VM_READ | VM_WRITE | VM_EXEC | \
> +#define VM_DATA_DEFAULT_FLAGS32 \
> + (((current->personality & READ_IMPLIES_EXEC) ? VM_EXEC : 0) | \
> + VM_READ | VM_WRITE | \
> VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
>
> #define VM_DATA_DEFAULT_FLAGS64 (VM_READ | VM_WRITE | \
> diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
> index 2472af2..065134b 100644
> --- a/fs/binfmt_elf.c
> +++ b/fs/binfmt_elf.c
> @@ -91,12 +91,18 @@ static struct linux_binfmt elf_format = {
>
> #define BAD_ADDR(x) ((unsigned long)(x) >= TASK_SIZE)
>
> -static int set_brk(unsigned long start, unsigned long end)
> +static int set_brk(unsigned long start, unsigned long end, int prot)
> {
> start = ELF_PAGEALIGN(start);
> end = ELF_PAGEALIGN(end);
> if (end > start) {
> - int error = vm_brk(start, end - start);
> + /*
> + * Map the last of the bss segment.
> + * If the header is requesting these pages to be
> + * executable, honour that (ppc32 needs this).
> + */
> + int error = vm_brk_flags(start, end - start,
> + prot & PROT_EXEC ? VM_EXEC : 0);
> if (error)
> return error;
> }
> @@ -524,6 +530,7 @@ static unsigned long load_elf_interp(struct elfhdr *interp_elf_ex,
> unsigned long load_addr = 0;
> int load_addr_set = 0;
> unsigned long last_bss = 0, elf_bss = 0;
> + int bss_prot = 0;
> unsigned long error = ~0UL;
> unsigned long total_size;
> int i;
> @@ -606,8 +613,10 @@ static unsigned long load_elf_interp(struct elfhdr *interp_elf_ex,
> * elf_bss and last_bss is the bss section.
> */
> k = load_addr + eppnt->p_vaddr + eppnt->p_memsz;
> - if (k > last_bss)
> + if (k > last_bss) {
> last_bss = k;
> + bss_prot = elf_prot;
> + }
> }
> }
>
> @@ -623,13 +632,14 @@ static unsigned long load_elf_interp(struct elfhdr *interp_elf_ex,
> /*
> * Next, align both the file and mem bss up to the page size,
> * since this is where elf_bss was just zeroed up to, and where
> - * last_bss will end after the vm_brk() below.
> + * last_bss will end after the vm_brk_flags() below.
> */
> elf_bss = ELF_PAGEALIGN(elf_bss);
> last_bss = ELF_PAGEALIGN(last_bss);
> /* Finally, if there is still more bss to allocate, do it. */
> if (last_bss > elf_bss) {
> - error = vm_brk(elf_bss, last_bss - elf_bss);
> + error = vm_brk_flags(elf_bss, last_bss - elf_bss,
> + bss_prot & PROT_EXEC ? VM_EXEC : 0);
> if (error)
> goto out;
> }
> @@ -674,6 +684,7 @@ static int load_elf_binary(struct linux_binprm *bprm)
> unsigned long error;
> struct elf_phdr *elf_ppnt, *elf_phdata, *interp_elf_phdata = NULL;
> unsigned long elf_bss, elf_brk;
> + int bss_prot = 0;
> int retval, i;
> unsigned long elf_entry;
> unsigned long interp_load_addr = 0;
> @@ -882,7 +893,8 @@ static int load_elf_binary(struct linux_binprm *bprm)
> before this one. Map anonymous pages, if needed,
> and clear the area. */
> retval = set_brk(elf_bss + load_bias,
> - elf_brk + load_bias);
> + elf_brk + load_bias,
> + bss_prot);
> if (retval)
> goto out_free_dentry;
> nbyte = ELF_PAGEOFFSET(elf_bss);
> @@ -976,8 +988,10 @@ static int load_elf_binary(struct linux_binprm *bprm)
> if (end_data < k)
> end_data = k;
> k = elf_ppnt->p_vaddr + elf_ppnt->p_memsz;
> - if (k > elf_brk)
> + if (k > elf_brk) {
> + bss_prot = elf_prot;
> elf_brk = k;
> + }
> }
>
> loc->elf_ex.e_entry += load_bias;
> @@ -993,7 +1007,7 @@ static int load_elf_binary(struct linux_binprm *bprm)
> * mapping in the interpreter, to make sure it doesn't wind
> * up getting placed where the bss needs to go.
> */
> - retval = set_brk(elf_bss, elf_brk);
> + retval = set_brk(elf_bss, elf_brk, bss_prot);
> if (retval)
> goto out_free_dentry;
> if (likely(elf_bss != elf_brk) && unlikely(padzero(elf_bss))) {
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index a92c8d7..2ac7e5e 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -2053,6 +2053,7 @@ static inline void mm_populate(unsigned long addr, unsigned long len) {}
>
> /* These take the mm semaphore themselves */
> extern int __must_check vm_brk(unsigned long, unsigned long);
> +extern int __must_check vm_brk_flags(unsigned long, unsigned long, unsigned long);
> extern int vm_munmap(unsigned long, size_t);
> extern unsigned long __must_check vm_mmap(struct file *, unsigned long,
> unsigned long, unsigned long,
> diff --git a/mm/mmap.c b/mm/mmap.c
> index 1af87c1..4adcf2c 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -2806,11 +2806,11 @@ static inline void verify_mm_writelocked(struct mm_struct *mm)
> * anonymous maps. eventually we may be able to do some
> * brk-specific accounting here.
> */
> -static int do_brk(unsigned long addr, unsigned long request)
> +static int do_brk_flags(unsigned long addr, unsigned long request, unsigned long flags)
> {
> struct mm_struct *mm = current->mm;
> struct vm_area_struct *vma, *prev;
> - unsigned long flags, len;
> + unsigned long len;
> struct rb_node **rb_link, *rb_parent;
> pgoff_t pgoff = addr >> PAGE_SHIFT;
> int error;
> @@ -2821,7 +2821,10 @@ static int do_brk(unsigned long addr, unsigned long request)
> if (!len)
> return 0;
>
> - flags = VM_DATA_DEFAULT_FLAGS | VM_ACCOUNT | mm->def_flags;
> + /* Until we need other flags, refuse anything except VM_EXEC. */
> + if ((flags & (~VM_EXEC)) != 0)
> + return -EINVAL;
> + flags |= VM_DATA_DEFAULT_FLAGS | VM_ACCOUNT | mm->def_flags;
>
> error = get_unmapped_area(NULL, addr, len, 0, MAP_FIXED);
> if (offset_in_page(error))
> @@ -2889,7 +2892,12 @@ static int do_brk(unsigned long addr, unsigned long request)
> return 0;
> }
>
> -int vm_brk(unsigned long addr, unsigned long len)
> +static int do_brk(unsigned long addr, unsigned long len)
> +{
> + return do_brk_flags(addr, len, 0);
> +}
> +
> +int vm_brk_flags(unsigned long addr, unsigned long len, unsigned long flags)
> {
> struct mm_struct *mm = current->mm;
> int ret;
> @@ -2898,13 +2906,19 @@ int vm_brk(unsigned long addr, unsigned long len)
> if (down_write_killable(&mm->mmap_sem))
> return -EINTR;
>
> - ret = do_brk(addr, len);
> + ret = do_brk_flags(addr, len, flags);
> populate = ((mm->def_flags & VM_LOCKED) != 0);
> up_write(&mm->mmap_sem);
> if (populate && !ret)
> mm_populate(addr, len);
> return ret;
> }
> +EXPORT_SYMBOL(vm_brk_flags);
> +
> +int vm_brk(unsigned long addr, unsigned long len)
> +{
> + return vm_brk_flags(addr, len, 0);
> +}
> EXPORT_SYMBOL(vm_brk);
>
> /* Release all mmaps. */
> --
> 2.9.2
>
--
Kees Cook
Nexus Security
On Wed, Dec 07, 2016 at 02:15:27PM -0800, Kees Cook wrote:
> Can you resend this patch with /proc/$pid/maps output showing the
> before/after effects of this change also included here? Showing that
> reduction in executable mapping should illustrate the benefit of
> avoiding having the execute bit set on the brk area (which is a clear
> security weakness, having writable/executable code in the process's
> memory segment, especially when it was _not_ requested by the ELF
> headers).
Denys, I'll leave it to you to re-sumbit again..
I suggest this reworded commit message:
powerpc: Do not make the entire heap executable
gcc has a configure option to generate ELF files that are W^X:
--enable-secureplt
However the PPC32 kernel is hardwired to add PROT_EXEC to the heap and
BSS segments, totally defeating this security protection.
This is done because the common ELF loader does not properly respect
the load header permissions when mapping a region that is not file
backed.
For example, non-secure PPC creates these segments:
[21] .data PROGBITS 10061254 051254 001868 00 WA 0 0 4
[22] .got PROGBITS 10062abc 052abc 000014 04 WAX 0 0 4
[23] .sdata PROGBITS 10062ad0 052ad0 000058 00 WA 0 0 4
[24] .sbss NOBITS 10062b28 052b28 000040 00 WA 0 0 8
[25] .plt NOBITS 10062b68 052b28 000d38 00 WAX 0 0 4
[26] .bss NOBITS 100638a0 052b28 006424 00 WA 0 0 16
Which results in an ELF load header covering those segments:
Type Offset VirtAddr PhysAddr FileSiz MemSiz Flg Align
LOAD 0x051160 0x10061160 0x10061160 0x019c8 0x08b64 RWE 0x10000
If we just remove the PROT_EXEC PPC32 hack then the kernel will do
something like:
10061000-10063000 rwxp 00051000 07:00 3208784 /app/bin/busybox.dynamic
10063000-1006a000 rw-p 00000000 00:00 0
104a6000-104c7000 rw-p 00000000 00:00 0 [heap]
The 2nd mapping is the anonymous 0 filled pages requested by FileSiz <
MemSiz, but it is not executable. This causes instant crashes of
normal PPC32 ELFs as their PLT and GOTs must be executable.
This patches fixes the above bug in the ELF loader and removes the
forced PROT_EXEC from PPC32.
The improvement is seen in /proc/PID/maps. After the patch a normal
ELF looks like:
10061000-10063000 rwxp 00051000 07:00 3208784 /app/bin/busybox.dynamic
10063000-1006a000 rwxp 00000000 00:00 0
104a6000-104c7000 rw-p 00000000 00:00 0 [heap]
while a secure-PLT ELF looks like:
10050000-10052000 rw-p 00050000 07:00 3208780 /app/bin/busybox.dynamic
10052000-10059000 rw-p 00000000 00:00 0
10740000-10761000 rw-p 00000000 00:00 0 [heap]
Critically, with this patch applied all of the maps within a
secure-plt ELF are W^X.
While before this patch we see undeseriable W&X.
normal ELF:
10061000-10063000 rwxp 00051000 07:00 3208784 /app/bin/busybox.dynamic
10063000-1006a000 rwxp 00000000 00:00 0
104a6000-104c7000 rwxp 00000000 00:00 0 [heap]
secure-plt ELF:
10050000-10052000 rw-p 00050000 07:00 3208780 /app/bin/busybox.dynamic
10052000-10059000 rwxp 00000000 00:00 0
104c7000-104e8000 rwxp 00000000 00:00 0 [heap]
-----------------
Andrew/Kees/Denys:
Here are full dumps from my PPC405 system.
Before patch (secure-plt):
00100000-00103000 r-xp 00000000 00:00 0 [vdso]
0fe25000-0fe30000 r-xp 00000000 07:00 6557504 /app/lib/libnss_files-2.23.so
0fe30000-0fe44000 ---p 0000b000 07:00 6557504 /app/lib/libnss_files-2.23.so
0fe44000-0fe45000 r--p 0000f000 07:00 6557504 /app/lib/libnss_files-2.23.so
0fe45000-0fe46000 rw-p 00010000 07:00 6557504 /app/lib/libnss_files-2.23.so
0fe46000-0fe4c000 rw-p 00000000 00:00 0
0fe5c000-0ffd2000 r-xp 00000000 07:00 4094672 /app/lib/libc-2.23.so
0ffd2000-0ffe8000 ---p 00176000 07:00 4094672 /app/lib/libc-2.23.so
0ffe8000-0ffec000 r--p 0017c000 07:00 4094672 /app/lib/libc-2.23.so
0ffec000-0ffed000 rw-p 00180000 07:00 4094672 /app/lib/libc-2.23.so
0ffed000-0fff0000 rw-p 00000000 00:00 0
10000000-1004e000 r-xp 00000000 07:00 3208780 /app/bin/busybox.dynamic
10050000-10052000 rw-p 00050000 07:00 3208780 /app/bin/busybox.dynamic
10052000-10059000 rwxp 00000000 00:00 0
104c7000-104e8000 rwxp 00000000 00:00 0 [heap]
b7ab1000-b7ad2000 r-xp 00000000 07:00 4009940 /app/lib/ld-2.23.so
b7aee000-b7af0000 rw-p 00000000 00:00 0
b7af0000-b7af1000 r--p 0002f000 07:00 4009940 /app/lib/ld-2.23.so
b7af1000-b7af2000 rw-p 00030000 07:00 4009940 /app/lib/ld-2.23.so
bfee1000-bff02000 rw-p 00000000 00:00 0 [stack]
After Patch (secure-plt):
00100000-00103000 r-xp 00000000 00:00 0 [vdso]
0fe25000-0fe30000 r-xp 00000000 07:00 6557504 /app/lib/libnss_files-2.23.so
0fe30000-0fe44000 ---p 0000b000 07:00 6557504 /app/lib/libnss_files-2.23.so
0fe44000-0fe45000 r--p 0000f000 07:00 6557504 /app/lib/libnss_files-2.23.so
0fe45000-0fe46000 rw-p 00010000 07:00 6557504 /app/lib/libnss_files-2.23.so
0fe46000-0fe4c000 rw-p 00000000 00:00 0
0fe5c000-0ffd2000 r-xp 00000000 07:00 4094672 /app/lib/libc-2.23.so
0ffd2000-0ffe8000 ---p 00176000 07:00 4094672 /app/lib/libc-2.23.so
0ffe8000-0ffec000 r--p 0017c000 07:00 4094672 /app/lib/libc-2.23.so
0ffec000-0ffed000 rw-p 00180000 07:00 4094672 /app/lib/libc-2.23.so
0ffed000-0fff0000 rw-p 00000000 00:00 0
10000000-1004e000 r-xp 00000000 07:00 3208780 /app/bin/busybox.dynamic
10050000-10052000 rw-p 00050000 07:00 3208780 /app/bin/busybox.dynamic
10052000-10059000 rw-p 00000000 00:00 0
10740000-10761000 rw-p 00000000 00:00 0 [heap]
b7bff000-b7c20000 r-xp 00000000 07:00 4009940 /app/lib/ld-2.23.so
b7c3c000-b7c3e000 rw-p 00000000 00:00 0
b7c3e000-b7c3f000 r--p 0002f000 07:00 4009940 /app/lib/ld-2.23.so
b7c3f000-b7c40000 rw-p 00030000 07:00 4009940 /app/lib/ld-2.23.so
bf891000-bf8b2000 rw-p 00000000 00:00 0 [stack]
Program Headers (secure-plt):
Type Offset VirtAddr PhysAddr FileSiz MemSiz Flg Align
PHDR 0x000034 0x10000034 0x10000034 0x00100 0x00100 R E 0x4
INTERP 0x000134 0x10000134 0x10000134 0x0000d 0x0000d R 0x1
[Requesting program interpreter: /lib/ld.so.1]
LOAD 0x000000 0x10000000 0x10000000 0x4da28 0x4da28 R E 0x10000
LOAD 0x050000 0x10050000 0x10050000 0x01e44 0x08288 RW 0x10000
DYNAMIC 0x050020 0x10050020 0x10050020 0x000d0 0x000d0 RW 0x4
NOTE 0x000144 0x10000144 0x10000144 0x00020 0x00020 R 0x4
GNU_EH_FRAME 0x04d8f0 0x1004d8f0 0x1004d8f0 0x00034 0x00034 R 0x4
GNU_STACK 0x000000 0x00000000 0x00000000 0x00000 0x00000 RW 0x10
Before patch (normal):
00100000-00103000 r-xp 00000000 00:00 0 [vdso]
0fe25000-0fe30000 r-xp 00000000 07:00 6564140 /app/lib/libnss_files-2.23.so
0fe30000-0fe44000 ---p 0000b000 07:00 6564140 /app/lib/libnss_files-2.23.so
0fe44000-0fe45000 r--p 0000f000 07:00 6564140 /app/lib/libnss_files-2.23.so
0fe45000-0fe46000 rw-p 00010000 07:00 6564140 /app/lib/libnss_files-2.23.so
0fe46000-0fe4c000 rw-p 00000000 00:00 0
0fe5c000-0ffd2000 r-xp 00000000 07:00 4101308 /app/lib/libc-2.23.so
0ffd2000-0ffe8000 ---p 00176000 07:00 4101308 /app/lib/libc-2.23.so
0ffe8000-0ffec000 r--p 0017c000 07:00 4101308 /app/lib/libc-2.23.so
0ffec000-0ffed000 rw-p 00180000 07:00 4101308 /app/lib/libc-2.23.so
0ffed000-0fff0000 rw-p 00000000 00:00 0
10000000-10052000 r-xp 00000000 07:00 3208784 /app/bin/busybox.dynamic
10061000-10063000 rwxp 00051000 07:00 3208784 /app/bin/busybox.dynamic
10063000-1006a000 rwxp 00000000 00:00 0
101f2000-10213000 rwxp 00000000 00:00 0 [heap]
b7e9a000-b7ebb000 r-xp 00000000 07:00 4016576 /app/lib/ld-2.23.so
b7ed7000-b7ed9000 rw-p 00000000 00:00 0
b7ed9000-b7eda000 r--p 0002f000 07:00 4016576 /app/lib/ld-2.23.so
b7eda000-b7edb000 rw-p 00030000 07:00 4016576 /app/lib/ld-2.23.so
bf8d4000-bf8f5000 rw-p 00000000 00:00 0 [stack]
After patch (normal):
00100000-00103000 r-xp 00000000 00:00 0 [vdso]
0fe25000-0fe30000 r-xp 00000000 07:00 6564140 /app/lib/libnss_files-2.23.so
0fe30000-0fe44000 ---p 0000b000 07:00 6564140 /app/lib/libnss_files-2.23.so
0fe44000-0fe45000 r--p 0000f000 07:00 6564140 /app/lib/libnss_files-2.23.so
0fe45000-0fe46000 rw-p 00010000 07:00 6564140 /app/lib/libnss_files-2.23.so
0fe46000-0fe4c000 rw-p 00000000 00:00 0
0fe5c000-0ffd2000 r-xp 00000000 07:00 4101308 /app/lib/libc-2.23.so
0ffd2000-0ffe8000 ---p 00176000 07:00 4101308 /app/lib/libc-2.23.so
0ffe8000-0ffec000 r--p 0017c000 07:00 4101308 /app/lib/libc-2.23.so
0ffec000-0ffed000 rw-p 00180000 07:00 4101308 /app/lib/libc-2.23.so
0ffed000-0fff0000 rw-p 00000000 00:00 0
10000000-10052000 r-xp 00000000 07:00 3208784 /app/bin/busybox.dynamic
10061000-10063000 rwxp 00051000 07:00 3208784 /app/bin/busybox.dynamic
10063000-1006a000 rwxp 00000000 00:00 0
104a6000-104c7000 rw-p 00000000 00:00 0 [heap]
b7aab000-b7acc000 r-xp 00000000 07:00 4016576 /app/lib/ld-2.23.so
b7ae8000-b7aea000 rw-p 00000000 00:00 0
b7aea000-b7aeb000 r--p 0002f000 07:00 4016576 /app/lib/ld-2.23.so
b7aeb000-b7aec000 rw-p 00030000 07:00 4016576 /app/lib/ld-2.23.so
bf9e0000-bfa01000 rw-p 00000000 00:00 0 [stack]
Program Headers (normal):
Type Offset VirtAddr PhysAddr FileSiz MemSiz Flg Align
PHDR 0x000034 0x10000034 0x10000034 0x00100 0x00100 R E 0x4
INTERP 0x000134 0x10000134 0x10000134 0x0000d 0x0000d R 0x1
[Requesting program interpreter: /lib/ld.so.1]
LOAD 0x000000 0x10000000 0x10000000 0x51160 0x51160 R E 0x10000
LOAD 0x051160 0x10061160 0x10061160 0x019c8 0x08b64 RWE 0x10000
DYNAMIC 0x05118c 0x1006118c 0x1006118c 0x000c8 0x000c8 RW 0x4
NOTE 0x000144 0x10000144 0x10000144 0x00020 0x00020 R 0x4
GNU_EH_FRAME 0x050d88 0x10050d88 0x10050d88 0x000ac 0x000ac R 0x4
GNU_STACK 0x000000 0x00000000 0x00000000 0x00000 0x00000 RW 0x4
Jason