[OpenWrt-Devel] kernel: MIPS: math-emu Write-protect delay slot emulation pages

Backport https://git.kernel.org/pub/scm/linux/kernel/git/mips/linux.git/commit/?id=adcc81f148d733b7e8e641300c5590a2cdc13bf3

"Mapping the delay slot emulation page as both writeable & executable
presents a security risk, in that if an exploit can write to & jump into
the page then it can be used as an easy way to execute arbitrary code.

Prevent this by mapping the page read-only for userland, and using
access_process_vm() with the FOLL_FORCE flag to write to it from
mips_dsemul().

This will likely be less efficient due to copy_to_user_page() performing
cache maintenance on a whole page, rather than a single line as in the
previous use of flush_cache_sigtramp(). However this delay slot
emulation code ought not to be running in any performance critical paths
anyway so this isn't really a problem, and we can probably do better in
copy_to_user_page() anyway in future.

A major advantage of this approach is that the fix is small & simple to
backport to stable kernels.

Reported-by: Andy Lutomirski <luto@kernel.org>
Signed-off-by: Paul Burton <paul.burton@mips.com>
Fixes: 432c6bacbd0c ("MIPS: Use per-mm page to execute branch delay slot instructions")"

Without patch:

cat /proc/self/maps
00400000-0047a000 r-xp 00000000 1f:03 1823       /bin/busybox
00489000-0048a000 r-xp 00079000 1f:03 1823       /bin/busybox
0048a000-0048b000 rwxp 0007a000 1f:03 1823       /bin/busybox
77ec8000-77eed000 r-xp 00000000 1f:03 2296       /lib/libgcc_s.so.1
77eed000-77eee000 rwxp 00015000 1f:03 2296       /lib/libgcc_s.so.1
77eee000-77f81000 r-xp 00000000 1f:03 2470       /lib/libc.so
77f90000-77f92000 rwxp 00092000 1f:03 2470       /lib/libc.so
77f92000-77f94000 rwxp 00000000 00:00 0
7f946000-7f967000 rw-p 00000000 00:00 0          [stack]
7fefb000-7fefc000 rwxp 00000000 00:00 0
7ffac000-7ffad000 r--p 00000000 00:00 0          [vvar]
7ffad000-7ffae000 r-xp 00000000 00:00 0          [vdso]

Patch applied:

cat /proc/self/maps
00400000-0047a000 r-xp 00000000 1f:03 1825       /bin/busybox
00489000-0048a000 r-xp 00079000 1f:03 1825       /bin/busybox
0048a000-0048b000 rwxp 0007a000 1f:03 1825       /bin/busybox
77ed0000-77ef5000 r-xp 00000000 1f:03 2298       /lib/libgcc_s.so.1
77ef5000-77ef6000 rwxp 00015000 1f:03 2298       /lib/libgcc_s.so.1
77ef6000-77f89000 r-xp 00000000 1f:03 2474       /lib/libc.so
77f98000-77f9a000 rwxp 00092000 1f:03 2474       /lib/libc.so
77f9a000-77f9c000 rwxp 00000000 00:00 0
7fbed000-7fc0e000 rw-p 00000000 00:00 0          [stack]
7fefb000-7fefc000 r-xp 00000000 00:00 0
7fff6000-7fff7000 r--p 00000000 00:00 0          [vvar]
7fff7000-7fff8000 r-xp 00000000 00:00 0          [vdso]

Note lack of write permission to 7fefb000-7fefc000

This has received minimal testing on ath79 4.14 Archer C7 v2 only

Signed-off-by: Kevin Darbyshire-Bryant <ldir@darbyshire-bryant.me.uk>
---
 ...e-protect-delay-slot-emulation-pages.patch | 119 ++++++++++++++++++
 ...e-protect-delay-slot-emulation-pages.patch | 119 ++++++++++++++++++
 ...e-protect-delay-slot-emulation-pages.patch | 119 ++++++++++++++++++
 3 files changed, 357 insertions(+)
 create mode 100644 target/linux/generic/backport-4.14/096-mips-math-emu-Write-protect-delay-slot-emulation-pages.patch
 create mode 100644 target/linux/generic/backport-4.19/096-mips-math-emu-Write-protect-delay-slot-emulation-pages.patch
 create mode 100644 target/linux/generic/backport-4.9/096-mips-math-emu-Write-protect-delay-slot-emulation-pages.patch

On Fri, Dec 21, 2018 at 9:21 AM Kevin 'ldir' Darbyshire-Bryant
<ldir@darbyshire-bryant.me.uk> wrote:
>
> Backport https://git.kernel.org/pub/scm/linux/kernel/git/mips/linux.git/commit/?id=adcc81f148d733b7e8e641300c5590a2cdc13bf3
>
> "Mapping the delay slot emulation page as both writeable & executable
> presents a security risk, in that if an exploit can write to & jump into
> the page then it can be used as an easy way to execute arbitrary code.
>
> Prevent this by mapping the page read-only for userland, and using
> access_process_vm() with the FOLL_FORCE flag to write to it from
> mips_dsemul().
>
> This will likely be less efficient due to copy_to_user_page() performing
> cache maintenance on a whole page, rather than a single line as in the
> previous use of flush_cache_sigtramp(). However this delay slot
> emulation code ought not to be running in any performance critical paths
> anyway so this isn't really a problem, and we can probably do better in
> copy_to_user_page() anyway in future.
>
> A major advantage of this approach is that the fix is small & simple to
> backport to stable kernels.
>
> Reported-by: Andy Lutomirski <luto@kernel.org>
> Signed-off-by: Paul Burton <paul.burton@mips.com>
> Fixes: 432c6bacbd0c ("MIPS: Use per-mm page to execute branch delay slot instructions")"
>
> Without patch:
>
> cat /proc/self/maps
> 00400000-0047a000 r-xp 00000000 1f:03 1823       /bin/busybox
> 00489000-0048a000 r-xp 00079000 1f:03 1823       /bin/busybox
> 0048a000-0048b000 rwxp 0007a000 1f:03 1823       /bin/busybox
> 77ec8000-77eed000 r-xp 00000000 1f:03 2296       /lib/libgcc_s.so.1
> 77eed000-77eee000 rwxp 00015000 1f:03 2296       /lib/libgcc_s.so.1
> 77eee000-77f81000 r-xp 00000000 1f:03 2470       /lib/libc.so
> 77f90000-77f92000 rwxp 00092000 1f:03 2470       /lib/libc.so
> 77f92000-77f94000 rwxp 00000000 00:00 0
> 7f946000-7f967000 rw-p 00000000 00:00 0          [stack]
> 7fefb000-7fefc000 rwxp 00000000 00:00 0
> 7ffac000-7ffad000 r--p 00000000 00:00 0          [vvar]
> 7ffad000-7ffae000 r-xp 00000000 00:00 0          [vdso]
>
> Patch applied:
>
> cat /proc/self/maps
> 00400000-0047a000 r-xp 00000000 1f:03 1825       /bin/busybox
> 00489000-0048a000 r-xp 00079000 1f:03 1825       /bin/busybox
> 0048a000-0048b000 rwxp 0007a000 1f:03 1825       /bin/busybox
> 77ed0000-77ef5000 r-xp 00000000 1f:03 2298       /lib/libgcc_s.so.1
> 77ef5000-77ef6000 rwxp 00015000 1f:03 2298       /lib/libgcc_s.so.1
> 77ef6000-77f89000 r-xp 00000000 1f:03 2474       /lib/libc.so
> 77f98000-77f9a000 rwxp 00092000 1f:03 2474       /lib/libc.so
> 77f9a000-77f9c000 rwxp 00000000 00:00 0
> 7fbed000-7fc0e000 rw-p 00000000 00:00 0          [stack]
> 7fefb000-7fefc000 r-xp 00000000 00:00 0
> 7fff6000-7fff7000 r--p 00000000 00:00 0          [vvar]
> 7fff7000-7fff8000 r-xp 00000000 00:00 0          [vdso]
>
> Note lack of write permission to 7fefb000-7fefc000
>
> This has received minimal testing on ath79 4.14 Archer C7 v2 only
>
> Signed-off-by: Kevin Darbyshire-Bryant <ldir@darbyshire-bryant.me.uk>
This will make it in the stable kernels within a few releases. Might
as well add in in the meantime.

> ---
>  ...e-protect-delay-slot-emulation-pages.patch | 119 ++++++++++++++++++
>  ...e-protect-delay-slot-emulation-pages.patch | 119 ++++++++++++++++++
>  ...e-protect-delay-slot-emulation-pages.patch | 119 ++++++++++++++++++
>  3 files changed, 357 insertions(+)
>  create mode 100644 target/linux/generic/backport-4.14/096-mips-math-emu-Write-protect-delay-slot-emulation-pages.patch
>  create mode 100644 target/linux/generic/backport-4.19/096-mips-math-emu-Write-protect-delay-slot-emulation-pages.patch
>  create mode 100644 target/linux/generic/backport-4.9/096-mips-math-emu-Write-protect-delay-slot-emulation-pages.patch
>
> diff --git a/target/linux/generic/backport-4.14/096-mips-math-emu-Write-protect-delay-slot-emulation-pages.patch b/target/linux/generic/backport-4.14/096-mips-math-emu-Write-protect-delay-slot-emulation-pages.patch
> new file mode 100644
> index 0000000000..f428285a64
> --- /dev/null
> +++ b/target/linux/generic/backport-4.14/096-mips-math-emu-Write-protect-delay-slot-emulation-pages.patch
> @@ -0,0 +1,119 @@
> +From adcc81f148d733b7e8e641300c5590a2cdc13bf3 Mon Sep 17 00:00:00 2001
> +From: Paul Burton <paul.burton@mips.com>
> +Date: Thu, 20 Dec 2018 17:45:43 +0000
> +Subject: MIPS: math-emu: Write-protect delay slot emulation pages
> +
> +Mapping the delay slot emulation page as both writeable & executable
> +presents a security risk, in that if an exploit can write to & jump into
> +the page then it can be used as an easy way to execute arbitrary code.
> +
> +Prevent this by mapping the page read-only for userland, and using
> +access_process_vm() with the FOLL_FORCE flag to write to it from
> +mips_dsemul().
> +
> +This will likely be less efficient due to copy_to_user_page() performing
> +cache maintenance on a whole page, rather than a single line as in the
> +previous use of flush_cache_sigtramp(). However this delay slot
> +emulation code ought not to be running in any performance critical paths
> +anyway so this isn't really a problem, and we can probably do better in
> +copy_to_user_page() anyway in future.
> +
> +A major advantage of this approach is that the fix is small & simple to
> +backport to stable kernels.
> +
> +Reported-by: Andy Lutomirski <luto@kernel.org>
> +Signed-off-by: Paul Burton <paul.burton@mips.com>
> +Fixes: 432c6bacbd0c ("MIPS: Use per-mm page to execute branch delay slot instructions")
> +Cc: stable@vger.kernel.org # v4.8+
> +Cc: linux-mips@vger.kernel.org
> +Cc: linux-kernel@vger.kernel.org
> +Cc: Rich Felker <dalias@libc.org>
> +Cc: David Daney <david.daney@cavium.com>
> +---
> + arch/mips/kernel/vdso.c     |  4 ++--
> + arch/mips/math-emu/dsemul.c | 38 ++++++++++++++++++++------------------
> + 2 files changed, 22 insertions(+), 20 deletions(-)
> +
> +--- a/arch/mips/kernel/vdso.c
> ++++ b/arch/mips/kernel/vdso.c
> +@@ -126,8 +126,8 @@ int arch_setup_additional_pages(struct l
> +
> +       /* Map delay slot emulation page */
> +       base = mmap_region(NULL, STACK_TOP, PAGE_SIZE,
> +-                         VM_READ|VM_WRITE|VM_EXEC|
> +-                         VM_MAYREAD|VM_MAYWRITE|VM_MAYEXEC,
> ++                         VM_READ | VM_EXEC |
> ++                         VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC,
> +                          0, NULL);
> +       if (IS_ERR_VALUE(base)) {
> +               ret = base;
> +--- a/arch/mips/math-emu/dsemul.c
> ++++ b/arch/mips/math-emu/dsemul.c
> +@@ -214,8 +214,9 @@ int mips_dsemul(struct pt_regs *regs, mi
> + {
> +       int isa16 = get_isa16_mode(regs->cp0_epc);
> +       mips_instruction break_math;
> +-      struct emuframe __user *fr;
> +-      int err, fr_idx;
> ++      unsigned long fr_uaddr;
> ++      struct emuframe fr;
> ++      int fr_idx, ret;
> +
> +       /* NOP is easy */
> +       if (ir == 0)
> +@@ -250,27 +251,31 @@ int mips_dsemul(struct pt_regs *regs, mi
> +               fr_idx = alloc_emuframe();
> +       if (fr_idx == BD_EMUFRAME_NONE)
> +               return SIGBUS;
> +-      fr = &dsemul_page()[fr_idx];
> +
> +       /* Retrieve the appropriately encoded break instruction */
> +       break_math = BREAK_MATH(isa16);
> +
> +       /* Write the instructions to the frame */
> +       if (isa16) {
> +-              err = __put_user(ir >> 16,
> +-                               (u16 __user *)(&fr->emul));
> +-              err |= __put_user(ir & 0xffff,
> +-                                (u16 __user *)((long)(&fr->emul) + 2));
> +-              err |= __put_user(break_math >> 16,
> +-                                (u16 __user *)(&fr->badinst));
> +-              err |= __put_user(break_math & 0xffff,
> +-                                (u16 __user *)((long)(&fr->badinst) + 2));
> ++              union mips_instruction _emul = {
> ++                      .halfword = { ir >> 16, ir }
> ++              };
> ++              union mips_instruction _badinst = {
> ++                      .halfword = { break_math >> 16, break_math }
> ++              };
> ++
> ++              fr.emul = _emul.word;
> ++              fr.badinst = _badinst.word;
> +       } else {
> +-              err = __put_user(ir, &fr->emul);
> +-              err |= __put_user(break_math, &fr->badinst);
> ++              fr.emul = ir;
> ++              fr.badinst = break_math;
> +       }
> +
> +-      if (unlikely(err)) {
> ++      /* Write the frame to user memory */
> ++      fr_uaddr = (unsigned long)&dsemul_page()[fr_idx];
> ++      ret = access_process_vm(current, fr_uaddr, &fr, sizeof(fr),
> ++                              FOLL_FORCE | FOLL_WRITE);
> ++      if (unlikely(ret != sizeof(fr))) {
> +               MIPS_FPU_EMU_INC_STATS(errors);
> +               free_emuframe(fr_idx, current->mm);
> +               return SIGBUS;
> +@@ -282,10 +287,7 @@ int mips_dsemul(struct pt_regs *regs, mi
> +       atomic_set(&current->thread.bd_emu_frame, fr_idx);
> +
> +       /* Change user register context to execute the frame */
> +-      regs->cp0_epc = (unsigned long)&fr->emul | isa16;
> +-
> +-      /* Ensure the icache observes our newly written frame */
> +-      flush_cache_sigtramp((unsigned long)&fr->emul);
> ++      regs->cp0_epc = fr_uaddr | isa16;
> +
> +       return 0;
> + }
> diff --git a/target/linux/generic/backport-4.19/096-mips-math-emu-Write-protect-delay-slot-emulation-pages.patch b/target/linux/generic/backport-4.19/096-mips-math-emu-Write-protect-delay-slot-emulation-pages.patch
> new file mode 100644
> index 0000000000..f428285a64
> --- /dev/null
> +++ b/target/linux/generic/backport-4.19/096-mips-math-emu-Write-protect-delay-slot-emulation-pages.patch
> @@ -0,0 +1,119 @@
> +From adcc81f148d733b7e8e641300c5590a2cdc13bf3 Mon Sep 17 00:00:00 2001
> +From: Paul Burton <paul.burton@mips.com>
> +Date: Thu, 20 Dec 2018 17:45:43 +0000
> +Subject: MIPS: math-emu: Write-protect delay slot emulation pages
> +
> +Mapping the delay slot emulation page as both writeable & executable
> +presents a security risk, in that if an exploit can write to & jump into
> +the page then it can be used as an easy way to execute arbitrary code.
> +
> +Prevent this by mapping the page read-only for userland, and using
> +access_process_vm() with the FOLL_FORCE flag to write to it from
> +mips_dsemul().
> +
> +This will likely be less efficient due to copy_to_user_page() performing
> +cache maintenance on a whole page, rather than a single line as in the
> +previous use of flush_cache_sigtramp(). However this delay slot
> +emulation code ought not to be running in any performance critical paths
> +anyway so this isn't really a problem, and we can probably do better in
> +copy_to_user_page() anyway in future.
> +
> +A major advantage of this approach is that the fix is small & simple to
> +backport to stable kernels.
> +
> +Reported-by: Andy Lutomirski <luto@kernel.org>
> +Signed-off-by: Paul Burton <paul.burton@mips.com>
> +Fixes: 432c6bacbd0c ("MIPS: Use per-mm page to execute branch delay slot instructions")
> +Cc: stable@vger.kernel.org # v4.8+
> +Cc: linux-mips@vger.kernel.org
> +Cc: linux-kernel@vger.kernel.org
> +Cc: Rich Felker <dalias@libc.org>
> +Cc: David Daney <david.daney@cavium.com>
> +---
> + arch/mips/kernel/vdso.c     |  4 ++--
> + arch/mips/math-emu/dsemul.c | 38 ++++++++++++++++++++------------------
> + 2 files changed, 22 insertions(+), 20 deletions(-)
> +
> +--- a/arch/mips/kernel/vdso.c
> ++++ b/arch/mips/kernel/vdso.c
> +@@ -126,8 +126,8 @@ int arch_setup_additional_pages(struct l
> +
> +       /* Map delay slot emulation page */
> +       base = mmap_region(NULL, STACK_TOP, PAGE_SIZE,
> +-                         VM_READ|VM_WRITE|VM_EXEC|
> +-                         VM_MAYREAD|VM_MAYWRITE|VM_MAYEXEC,
> ++                         VM_READ | VM_EXEC |
> ++                         VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC,
> +                          0, NULL);
> +       if (IS_ERR_VALUE(base)) {
> +               ret = base;
> +--- a/arch/mips/math-emu/dsemul.c
> ++++ b/arch/mips/math-emu/dsemul.c
> +@@ -214,8 +214,9 @@ int mips_dsemul(struct pt_regs *regs, mi
> + {
> +       int isa16 = get_isa16_mode(regs->cp0_epc);
> +       mips_instruction break_math;
> +-      struct emuframe __user *fr;
> +-      int err, fr_idx;
> ++      unsigned long fr_uaddr;
> ++      struct emuframe fr;
> ++      int fr_idx, ret;
> +
> +       /* NOP is easy */
> +       if (ir == 0)
> +@@ -250,27 +251,31 @@ int mips_dsemul(struct pt_regs *regs, mi
> +               fr_idx = alloc_emuframe();
> +       if (fr_idx == BD_EMUFRAME_NONE)
> +               return SIGBUS;
> +-      fr = &dsemul_page()[fr_idx];
> +
> +       /* Retrieve the appropriately encoded break instruction */
> +       break_math = BREAK_MATH(isa16);
> +
> +       /* Write the instructions to the frame */
> +       if (isa16) {
> +-              err = __put_user(ir >> 16,
> +-                               (u16 __user *)(&fr->emul));
> +-              err |= __put_user(ir & 0xffff,
> +-                                (u16 __user *)((long)(&fr->emul) + 2));
> +-              err |= __put_user(break_math >> 16,
> +-                                (u16 __user *)(&fr->badinst));
> +-              err |= __put_user(break_math & 0xffff,
> +-                                (u16 __user *)((long)(&fr->badinst) + 2));
> ++              union mips_instruction _emul = {
> ++                      .halfword = { ir >> 16, ir }
> ++              };
> ++              union mips_instruction _badinst = {
> ++                      .halfword = { break_math >> 16, break_math }
> ++              };
> ++
> ++              fr.emul = _emul.word;
> ++              fr.badinst = _badinst.word;
> +       } else {
> +-              err = __put_user(ir, &fr->emul);
> +-              err |= __put_user(break_math, &fr->badinst);
> ++              fr.emul = ir;
> ++              fr.badinst = break_math;
> +       }
> +
> +-      if (unlikely(err)) {
> ++      /* Write the frame to user memory */
> ++      fr_uaddr = (unsigned long)&dsemul_page()[fr_idx];
> ++      ret = access_process_vm(current, fr_uaddr, &fr, sizeof(fr),
> ++                              FOLL_FORCE | FOLL_WRITE);
> ++      if (unlikely(ret != sizeof(fr))) {
> +               MIPS_FPU_EMU_INC_STATS(errors);
> +               free_emuframe(fr_idx, current->mm);
> +               return SIGBUS;
> +@@ -282,10 +287,7 @@ int mips_dsemul(struct pt_regs *regs, mi
> +       atomic_set(&current->thread.bd_emu_frame, fr_idx);
> +
> +       /* Change user register context to execute the frame */
> +-      regs->cp0_epc = (unsigned long)&fr->emul | isa16;
> +-
> +-      /* Ensure the icache observes our newly written frame */
> +-      flush_cache_sigtramp((unsigned long)&fr->emul);
> ++      regs->cp0_epc = fr_uaddr | isa16;
> +
> +       return 0;
> + }
> diff --git a/target/linux/generic/backport-4.9/096-mips-math-emu-Write-protect-delay-slot-emulation-pages.patch b/target/linux/generic/backport-4.9/096-mips-math-emu-Write-protect-delay-slot-emulation-pages.patch
> new file mode 100644
> index 0000000000..69cc493bba
> --- /dev/null
> +++ b/target/linux/generic/backport-4.9/096-mips-math-emu-Write-protect-delay-slot-emulation-pages.patch
> @@ -0,0 +1,119 @@
> +From adcc81f148d733b7e8e641300c5590a2cdc13bf3 Mon Sep 17 00:00:00 2001
> +From: Paul Burton <paul.burton@mips.com>
> +Date: Thu, 20 Dec 2018 17:45:43 +0000
> +Subject: MIPS: math-emu: Write-protect delay slot emulation pages
> +
> +Mapping the delay slot emulation page as both writeable & executable
> +presents a security risk, in that if an exploit can write to & jump into
> +the page then it can be used as an easy way to execute arbitrary code.
> +
> +Prevent this by mapping the page read-only for userland, and using
> +access_process_vm() with the FOLL_FORCE flag to write to it from
> +mips_dsemul().
> +
> +This will likely be less efficient due to copy_to_user_page() performing
> +cache maintenance on a whole page, rather than a single line as in the
> +previous use of flush_cache_sigtramp(). However this delay slot
> +emulation code ought not to be running in any performance critical paths
> +anyway so this isn't really a problem, and we can probably do better in
> +copy_to_user_page() anyway in future.
> +
> +A major advantage of this approach is that the fix is small & simple to
> +backport to stable kernels.
> +
> +Reported-by: Andy Lutomirski <luto@kernel.org>
> +Signed-off-by: Paul Burton <paul.burton@mips.com>
> +Fixes: 432c6bacbd0c ("MIPS: Use per-mm page to execute branch delay slot instructions")
> +Cc: stable@vger.kernel.org # v4.8+
> +Cc: linux-mips@vger.kernel.org
> +Cc: linux-kernel@vger.kernel.org
> +Cc: Rich Felker <dalias@libc.org>
> +Cc: David Daney <david.daney@cavium.com>
> +---
> + arch/mips/kernel/vdso.c     |  4 ++--
> + arch/mips/math-emu/dsemul.c | 38 ++++++++++++++++++++------------------
> + 2 files changed, 22 insertions(+), 20 deletions(-)
> +
> +--- a/arch/mips/kernel/vdso.c
> ++++ b/arch/mips/kernel/vdso.c
> +@@ -111,8 +111,8 @@ int arch_setup_additional_pages(struct l
> +
> +       /* Map delay slot emulation page */
> +       base = mmap_region(NULL, STACK_TOP, PAGE_SIZE,
> +-                         VM_READ|VM_WRITE|VM_EXEC|
> +-                         VM_MAYREAD|VM_MAYWRITE|VM_MAYEXEC,
> ++                         VM_READ | VM_EXEC |
> ++                         VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC,
> +                          0);
> +       if (IS_ERR_VALUE(base)) {
> +               ret = base;
> +--- a/arch/mips/math-emu/dsemul.c
> ++++ b/arch/mips/math-emu/dsemul.c
> +@@ -211,8 +211,9 @@ int mips_dsemul(struct pt_regs *regs, mi
> + {
> +       int isa16 = get_isa16_mode(regs->cp0_epc);
> +       mips_instruction break_math;
> +-      struct emuframe __user *fr;
> +-      int err, fr_idx;
> ++      unsigned long fr_uaddr;
> ++      struct emuframe fr;
> ++      int fr_idx, ret;
> +
> +       /* NOP is easy */
> +       if (ir == 0)
> +@@ -247,27 +248,31 @@ int mips_dsemul(struct pt_regs *regs, mi
> +               fr_idx = alloc_emuframe();
> +       if (fr_idx == BD_EMUFRAME_NONE)
> +               return SIGBUS;
> +-      fr = &dsemul_page()[fr_idx];
> +
> +       /* Retrieve the appropriately encoded break instruction */
> +       break_math = BREAK_MATH(isa16);
> +
> +       /* Write the instructions to the frame */
> +       if (isa16) {
> +-              err = __put_user(ir >> 16,
> +-                               (u16 __user *)(&fr->emul));
> +-              err |= __put_user(ir & 0xffff,
> +-                                (u16 __user *)((long)(&fr->emul) + 2));
> +-              err |= __put_user(break_math >> 16,
> +-                                (u16 __user *)(&fr->badinst));
> +-              err |= __put_user(break_math & 0xffff,
> +-                                (u16 __user *)((long)(&fr->badinst) + 2));
> ++              union mips_instruction _emul = {
> ++                      .halfword = { ir >> 16, ir }
> ++              };
> ++              union mips_instruction _badinst = {
> ++                      .halfword = { break_math >> 16, break_math }
> ++              };
> ++
> ++              fr.emul = _emul.word;
> ++              fr.badinst = _badinst.word;
> +       } else {
> +-              err = __put_user(ir, &fr->emul);
> +-              err |= __put_user(break_math, &fr->badinst);
> ++              fr.emul = ir;
> ++              fr.badinst = break_math;
> +       }
> +
> +-      if (unlikely(err)) {
> ++      /* Write the frame to user memory */
> ++      fr_uaddr = (unsigned long)&dsemul_page()[fr_idx];
> ++      ret = access_process_vm(current, fr_uaddr, &fr, sizeof(fr),
> ++                              FOLL_FORCE | FOLL_WRITE);
> ++      if (unlikely(ret != sizeof(fr))) {
> +               MIPS_FPU_EMU_INC_STATS(errors);
> +               free_emuframe(fr_idx, current->mm);
> +               return SIGBUS;
> +@@ -279,10 +284,7 @@ int mips_dsemul(struct pt_regs *regs, mi
> +       atomic_set(&current->thread.bd_emu_frame, fr_idx);
> +
> +       /* Change user register context to execute the frame */
> +-      regs->cp0_epc = (unsigned long)&fr->emul | isa16;
> +-
> +-      /* Ensure the icache observes our newly written frame */
> +-      flush_cache_sigtramp((unsigned long)&fr->emul);
> ++      regs->cp0_epc = fr_uaddr | isa16;
> +
> +       return 0;
> + }
> --
> 2.17.2 (Apple Git-113)
>
>
> _______________________________________________
> openwrt-devel mailing list
> openwrt-devel@lists.openwrt.org
> https://lists.openwrt.org/mailman/listinfo/openwrt-devel

On Sat, 22 Dec 2018 at 01:21, Kevin 'ldir' Darbyshire-Bryant
<ldir@darbyshire-bryant.me.uk> wrote:
>
> Backport https://git.kernel.org/pub/scm/linux/kernel/git/mips/linux.git/commit/?id=adcc81f148d733b7e8e641300c5590a2cdc13bf3
>
> "Mapping the delay slot emulation page as both writeable & executable
> presents a security risk, in that if an exploit can write to & jump into
> the page then it can be used as an easy way to execute arbitrary code.
>
> Prevent this by mapping the page read-only for userland, and using
> access_process_vm() with the FOLL_FORCE flag to write to it from
> mips_dsemul().
>
> This will likely be less efficient due to copy_to_user_page() performing
> cache maintenance on a whole page, rather than a single line as in the
> previous use of flush_cache_sigtramp(). However this delay slot
> emulation code ought not to be running in any performance critical paths
> anyway so this isn't really a problem, and we can probably do better in
> copy_to_user_page() anyway in future.
>
> A major advantage of this approach is that the fix is small & simple to
> backport to stable kernels.
>
> Reported-by: Andy Lutomirski <luto@kernel.org>
> Signed-off-by: Paul Burton <paul.burton@mips.com>
> Fixes: 432c6bacbd0c ("MIPS: Use per-mm page to execute branch delay slot instructions")"
>
> Without patch:
>
> cat /proc/self/maps
> 00400000-0047a000 r-xp 00000000 1f:03 1823       /bin/busybox
> 00489000-0048a000 r-xp 00079000 1f:03 1823       /bin/busybox
> 0048a000-0048b000 rwxp 0007a000 1f:03 1823       /bin/busybox
> 77ec8000-77eed000 r-xp 00000000 1f:03 2296       /lib/libgcc_s.so.1
> 77eed000-77eee000 rwxp 00015000 1f:03 2296       /lib/libgcc_s.so.1
> 77eee000-77f81000 r-xp 00000000 1f:03 2470       /lib/libc.so
> 77f90000-77f92000 rwxp 00092000 1f:03 2470       /lib/libc.so
> 77f92000-77f94000 rwxp 00000000 00:00 0
> 7f946000-7f967000 rw-p 00000000 00:00 0          [stack]
> 7fefb000-7fefc000 rwxp 00000000 00:00 0
> 7ffac000-7ffad000 r--p 00000000 00:00 0          [vvar]
> 7ffad000-7ffae000 r-xp 00000000 00:00 0          [vdso]

Hi,

I must miss something.  After reading another thread on mips security,
I was thinking that all segments with w and x permission set were
problematic:  the same attacker can write and execute shellcode there,
right?  Sorry, if the answer is too apparent ;(

Regards,
                yousong

On Fri, Dec 21, 2018 at 8:05 PM Yousong Zhou <yszhou4tech@gmail.com> wrote:
>
> On Sat, 22 Dec 2018 at 01:21, Kevin 'ldir' Darbyshire-Bryant
> <ldir@darbyshire-bryant.me.uk> wrote:
> >
> > Backport https://git.kernel.org/pub/scm/linux/kernel/git/mips/linux.git/commit/?id=adcc81f148d733b7e8e641300c5590a2cdc13bf3
> >
> > "Mapping the delay slot emulation page as both writeable & executable
> > presents a security risk, in that if an exploit can write to & jump into
> > the page then it can be used as an easy way to execute arbitrary code.
> >
> > Prevent this by mapping the page read-only for userland, and using
> > access_process_vm() with the FOLL_FORCE flag to write to it from
> > mips_dsemul().
> >
> > This will likely be less efficient due to copy_to_user_page() performing
> > cache maintenance on a whole page, rather than a single line as in the
> > previous use of flush_cache_sigtramp(). However this delay slot
> > emulation code ought not to be running in any performance critical paths
> > anyway so this isn't really a problem, and we can probably do better in
> > copy_to_user_page() anyway in future.
> >
> > A major advantage of this approach is that the fix is small & simple to
> > backport to stable kernels.
> >
> > Reported-by: Andy Lutomirski <luto@kernel.org>
> > Signed-off-by: Paul Burton <paul.burton@mips.com>
> > Fixes: 432c6bacbd0c ("MIPS: Use per-mm page to execute branch delay slot instructions")"
> >
> > Without patch:
> >
> > cat /proc/self/maps
> > 00400000-0047a000 r-xp 00000000 1f:03 1823       /bin/busybox
> > 00489000-0048a000 r-xp 00079000 1f:03 1823       /bin/busybox
> > 0048a000-0048b000 rwxp 0007a000 1f:03 1823       /bin/busybox
> > 77ec8000-77eed000 r-xp 00000000 1f:03 2296       /lib/libgcc_s.so.1
> > 77eed000-77eee000 rwxp 00015000 1f:03 2296       /lib/libgcc_s.so.1
> > 77eee000-77f81000 r-xp 00000000 1f:03 2470       /lib/libc.so
> > 77f90000-77f92000 rwxp 00092000 1f:03 2470       /lib/libc.so
> > 77f92000-77f94000 rwxp 00000000 00:00 0
> > 7f946000-7f967000 rw-p 00000000 00:00 0          [stack]
> > 7fefb000-7fefc000 rwxp 00000000 00:00 0
> > 7ffac000-7ffad000 r--p 00000000 00:00 0          [vvar]
> > 7ffad000-7ffae000 r-xp 00000000 00:00 0          [vdso]
>
> Hi,
>
> I must miss something.  After reading another thread on mips security,
> I was thinking that all segments with w and x permission set were
> problematic:  the same attacker can write and execute shellcode there,
> right?  Sorry, if the answer is too apparent ;(
Right. This is a fix for one of those sections. Not all of them.

It will take quite a bit of work to fix all of them.
>
> Regards,
>                 yousong
>
> _______________________________________________
> openwrt-devel mailing list
> openwrt-devel@lists.openwrt.org
> https://lists.openwrt.org/mailman/listinfo/openwrt-devel

> On 22 Dec 2018, at 04:04, Yousong Zhou <yszhou4tech@gmail.com> wrote:
> 
> On Sat, 22 Dec 2018 at 01:21, Kevin 'ldir' Darbyshire-Bryant
> <ldir@darbyshire-bryant.me.uk> wrote:
>> 
>> Backport https://git.kernel.org/pub/scm/linux/kernel/git/mips/linux.git/commit/?id=adcc81f148d733b7e8e641300c5590a2cdc13bf3
>> 
>> "Mapping the delay slot emulation page as both writeable & executable
>> presents a security risk, in that if an exploit can write to & jump into
>> the page then it can be used as an easy way to execute arbitrary code.
>> 
>> Prevent this by mapping the page read-only for userland, and using
>> access_process_vm() with the FOLL_FORCE flag to write to it from
>> mips_dsemul().
>> 
>> This will likely be less efficient due to copy_to_user_page() performing
>> cache maintenance on a whole page, rather than a single line as in the
>> previous use of flush_cache_sigtramp(). However this delay slot
>> emulation code ought not to be running in any performance critical paths
>> anyway so this isn't really a problem, and we can probably do better in
>> copy_to_user_page() anyway in future.
>> 
>> A major advantage of this approach is that the fix is small & simple to
>> backport to stable kernels.
>> 
>> Reported-by: Andy Lutomirski <luto@kernel.org>
>> Signed-off-by: Paul Burton <paul.burton@mips.com>
>> Fixes: 432c6bacbd0c ("MIPS: Use per-mm page to execute branch delay slot instructions")"
>> 
>> Without patch:
>> 
>> cat /proc/self/maps
>> 00400000-0047a000 r-xp 00000000 1f:03 1823       /bin/busybox
>> 00489000-0048a000 r-xp 00079000 1f:03 1823       /bin/busybox
>> 0048a000-0048b000 rwxp 0007a000 1f:03 1823       /bin/busybox
>> 77ec8000-77eed000 r-xp 00000000 1f:03 2296       /lib/libgcc_s.so.1
>> 77eed000-77eee000 rwxp 00015000 1f:03 2296       /lib/libgcc_s.so.1
>> 77eee000-77f81000 r-xp 00000000 1f:03 2470       /lib/libc.so
>> 77f90000-77f92000 rwxp 00092000 1f:03 2470       /lib/libc.so
>> 77f92000-77f94000 rwxp 00000000 00:00 0
>> 7f946000-7f967000 rw-p 00000000 00:00 0          [stack]
>> 7fefb000-7fefc000 rwxp 00000000 00:00 0
>> 7ffac000-7ffad000 r--p 00000000 00:00 0          [vvar]
>> 7ffad000-7ffae000 r-xp 00000000 00:00 0          [vdso]
> 
> Hi,
> 
> I must miss something.  After reading another thread on mips security,
> I was thinking that all segments with w and x permission set were
> problematic:  the same attacker can write and execute shellcode there,
> right?  Sorry, if the answer is too apparent ;(
> 
> Regards,
>                yousong

Hi Yousong,

My limited understanding goes something like this:  Most of the other segments address ranges change on each execution due to ASLR, thus whilst they can be written to things are harder for an attacker because locations change.  The math emulation page was especially bad because a) user space had write permission to it and b) it was always in a fixed location.  The patch removes user space write permission to the page thus the process should receive a SIGSEGV if it attempts to do so.

Cheers,

Kevin D-B

012C ACB2 28C6 C53E 9775  9123 B3A2 389B 9DE2 334A

On Sat, 22 Dec 2018 at 16:03, Kevin 'ldir' Darbyshire-Bryant
<ldir@darbyshire-bryant.me.uk> wrote:
>
>
>
> > On 22 Dec 2018, at 04:04, Yousong Zhou <yszhou4tech@gmail.com> wrote:
> >
> > On Sat, 22 Dec 2018 at 01:21, Kevin 'ldir' Darbyshire-Bryant
> > <ldir@darbyshire-bryant.me.uk> wrote:
> >>
> >> Backport https://git.kernel.org/pub/scm/linux/kernel/git/mips/linux.git/commit/?id=adcc81f148d733b7e8e641300c5590a2cdc13bf3
> >>
> >> "Mapping the delay slot emulation page as both writeable & executable
> >> presents a security risk, in that if an exploit can write to & jump into
> >> the page then it can be used as an easy way to execute arbitrary code.
> >>
> >> Prevent this by mapping the page read-only for userland, and using
> >> access_process_vm() with the FOLL_FORCE flag to write to it from
> >> mips_dsemul().
> >>
> >> This will likely be less efficient due to copy_to_user_page() performing
> >> cache maintenance on a whole page, rather than a single line as in the
> >> previous use of flush_cache_sigtramp(). However this delay slot
> >> emulation code ought not to be running in any performance critical paths
> >> anyway so this isn't really a problem, and we can probably do better in
> >> copy_to_user_page() anyway in future.
> >>
> >> A major advantage of this approach is that the fix is small & simple to
> >> backport to stable kernels.
> >>
> >> Reported-by: Andy Lutomirski <luto@kernel.org>
> >> Signed-off-by: Paul Burton <paul.burton@mips.com>
> >> Fixes: 432c6bacbd0c ("MIPS: Use per-mm page to execute branch delay slot instructions")"
> >>
> >> Without patch:
> >>
> >> cat /proc/self/maps
> >> 00400000-0047a000 r-xp 00000000 1f:03 1823       /bin/busybox
> >> 00489000-0048a000 r-xp 00079000 1f:03 1823       /bin/busybox
> >> 0048a000-0048b000 rwxp 0007a000 1f:03 1823       /bin/busybox
> >> 77ec8000-77eed000 r-xp 00000000 1f:03 2296       /lib/libgcc_s.so.1
> >> 77eed000-77eee000 rwxp 00015000 1f:03 2296       /lib/libgcc_s.so.1
> >> 77eee000-77f81000 r-xp 00000000 1f:03 2470       /lib/libc.so
> >> 77f90000-77f92000 rwxp 00092000 1f:03 2470       /lib/libc.so
> >> 77f92000-77f94000 rwxp 00000000 00:00 0
> >> 7f946000-7f967000 rw-p 00000000 00:00 0          [stack]
> >> 7fefb000-7fefc000 rwxp 00000000 00:00 0
> >> 7ffac000-7ffad000 r--p 00000000 00:00 0          [vvar]
> >> 7ffad000-7ffae000 r-xp 00000000 00:00 0          [vdso]
> >
> > Hi,
> >
> > I must miss something.  After reading another thread on mips security,
> > I was thinking that all segments with w and x permission set were
> > problematic:  the same attacker can write and execute shellcode there,
> > right?  Sorry, if the answer is too apparent ;(
> >
> > Regards,
> >                yousong
>
> Hi Yousong,
>
> My limited understanding goes something like this:  Most of the other segments address ranges change on each execution due to ASLR, thus whilst they can be written to things are harder for an attacker because locations change.  The math emulation page was especially bad because a) user space had write permission to it and b) it was always in a fixed location.  The patch removes user space write permission to the page thus the process should receive a SIGSEGV if it attempts to do so.
>
> Cheers,
>
> Kevin D-B

Thanks for the confirmation ;) . Indeed, I just did a quick read on
the kernel code, that region is mapped at STACK_TOP, essentially a
constant.

It's also worth noting that segments of the executable itself is also
not randomized.  I checked OpenWrt x86/64, malta/be, and CentOS7, they
are all the same.  It's quite a surprise to me.  I always assumed
randomization was applied to all segments.

Regards,
                yousong

On 12/22/18 10:26 AM, Yousong Zhou wrote:
> On Sat, 22 Dec 2018 at 16:03, Kevin 'ldir' Darbyshire-Bryant
> <ldir@darbyshire-bryant.me.uk> wrote:
>>
>>
>>
>>> On 22 Dec 2018, at 04:04, Yousong Zhou <yszhou4tech@gmail.com> wrote:
>>>
>>> On Sat, 22 Dec 2018 at 01:21, Kevin 'ldir' Darbyshire-Bryant
>>> <ldir@darbyshire-bryant.me.uk> wrote:
>>>>
>>>> Backport https://git.kernel.org/pub/scm/linux/kernel/git/mips/linux.git/commit/?id=adcc81f148d733b7e8e641300c5590a2cdc13bf3
>>>>
>>>> "Mapping the delay slot emulation page as both writeable & executable
>>>> presents a security risk, in that if an exploit can write to & jump into
>>>> the page then it can be used as an easy way to execute arbitrary code.
>>>>
>>>> Prevent this by mapping the page read-only for userland, and using
>>>> access_process_vm() with the FOLL_FORCE flag to write to it from
>>>> mips_dsemul().
>>>>
>>>> This will likely be less efficient due to copy_to_user_page() performing
>>>> cache maintenance on a whole page, rather than a single line as in the
>>>> previous use of flush_cache_sigtramp(). However this delay slot
>>>> emulation code ought not to be running in any performance critical paths
>>>> anyway so this isn't really a problem, and we can probably do better in
>>>> copy_to_user_page() anyway in future.
>>>>
>>>> A major advantage of this approach is that the fix is small & simple to
>>>> backport to stable kernels.
>>>>
>>>> Reported-by: Andy Lutomirski <luto@kernel.org>
>>>> Signed-off-by: Paul Burton <paul.burton@mips.com>
>>>> Fixes: 432c6bacbd0c ("MIPS: Use per-mm page to execute branch delay slot instructions")"
>>>>
>>>> Without patch:
>>>>
>>>> cat /proc/self/maps
>>>> 00400000-0047a000 r-xp 00000000 1f:03 1823       /bin/busybox
>>>> 00489000-0048a000 r-xp 00079000 1f:03 1823       /bin/busybox
>>>> 0048a000-0048b000 rwxp 0007a000 1f:03 1823       /bin/busybox
>>>> 77ec8000-77eed000 r-xp 00000000 1f:03 2296       /lib/libgcc_s.so.1
>>>> 77eed000-77eee000 rwxp 00015000 1f:03 2296       /lib/libgcc_s.so.1
>>>> 77eee000-77f81000 r-xp 00000000 1f:03 2470       /lib/libc.so
>>>> 77f90000-77f92000 rwxp 00092000 1f:03 2470       /lib/libc.so
>>>> 77f92000-77f94000 rwxp 00000000 00:00 0
>>>> 7f946000-7f967000 rw-p 00000000 00:00 0          [stack]
>>>> 7fefb000-7fefc000 rwxp 00000000 00:00 0
>>>> 7ffac000-7ffad000 r--p 00000000 00:00 0          [vvar]
>>>> 7ffad000-7ffae000 r-xp 00000000 00:00 0          [vdso]
>>>
>>> Hi,
>>>
>>> I must miss something.  After reading another thread on mips security,
>>> I was thinking that all segments with w and x permission set were
>>> problematic:  the same attacker can write and execute shellcode there,
>>> right?  Sorry, if the answer is too apparent ;(
>>>
>>> Regards,
>>>                 yousong
>>
>> Hi Yousong,
>>
>> My limited understanding goes something like this:  Most of the other segments address ranges change on each execution due to ASLR, thus whilst they can be written to things are harder for an attacker because locations change.  The math emulation page was especially bad because a) user space had write permission to it and b) it was always in a fixed location.  The patch removes user space write permission to the page thus the process should receive a SIGSEGV if it attempts to do so.
>>
>> Cheers,
>>
>> Kevin D-B
> 
> Thanks for the confirmation ;) . Indeed, I just did a quick read on
> the kernel code, that region is mapped at STACK_TOP, essentially a
> constant.
> 
> It's also worth noting that segments of the executable itself is also
> not randomized.  I checked OpenWrt x86/64, malta/be, and CentOS7, they
> are all the same.  It's quite a surprise to me.  I always assumed
> randomization was applied to all segments.
> 
> Regards,
>                  yousong

Hi Yousong,

ASLR is currently not activated by default in OpenWrt, so the binary 
itself is not randomized. Activate CONFIG_PKG_ASLR_PIE to compile 
Openwrt with ASLR, but this increases the size of the binary.

I haven't understood why some parts of the busybox binary and other 
binaries are mapped rwx, when I look into it with readelf no section is 
mapped rwx, but it looks like some sections are ending at an not page 
aligned offset and the next section starts directly after that. I assume 
that Linux merges the permissions when one page needs different permissions.

I am still not sure if the common mips CPUs (24Kec, 74Kec) support 
restricting execution on pages anyway.

Huake

> On 22 Dec 2018, at 18:28, Hauke Mehrtens <hauke@hauke-m.de> wrote:
> 
> 
> Hi Yousong,
> 
> ASLR is currently not activated by default in OpenWrt, so the binary itself is not randomized. Activate CONFIG_PKG_ASLR_PIE to compile Openwrt with ASLR, but this increases the size of the binary.
> 
> I haven't understood why some parts of the busybox binary and other binaries are mapped rwx, when I look into it with readelf no section is mapped rwx, but it looks like some sections are ending at an not page aligned offset and the next section starts directly after that. I assume that Linux merges the permissions when one page needs different permissions.
> 
> I am still not sure if the common mips CPUs (24Kec, 74Kec) support restricting execution on pages anyway.
> 
> Huake

At the risk of going further down the rabbit hole/off topic, if you set the cpu_has_rixi to 1 in  target/linux/ath79/patches-4.14/0014-MIPS-ath79-finetune-cpu-overrides.patch and with PKG_ASLR_PIE [=y]

you get:
cat /proc/self/maps 
00400000-0047a000 r-xp 00000000 1f:03 1825       /bin/busybox
00489000-0048a000 r--p 00079000 1f:03 1825       /bin/busybox
0048a000-0048b000 rw-p 0007a000 1f:03 1825       /bin/busybox
77e38000-77e5d000 r-xp 00000000 1f:03 2298       /lib/libgcc_s.so.1
77e5d000-77e5e000 rw-p 00015000 1f:03 2298       /lib/libgcc_s.so.1
77e5e000-77ef1000 r-xp 00000000 1f:03 2474       /lib/libc.so
77f00000-77f02000 rw-p 00092000 1f:03 2474       /lib/libc.so
77f02000-77f04000 rw-p 00000000 00:00 0 
7f9bd000-7f9de000 rw-p 00000000 00:00 0          [stack]
7fefb000-7fefc000 r-xp 00000000 00:00 0 
7ff68000-7ff69000 r--p 00000000 00:00 0          [vvar]
7ff69000-7ff6a000 r-xp 00000000 00:00 0          [vdso]


The archer hasn’t blown up…….yet

Cheers,

Kevin D-B

012C ACB2 28C6 C53E 9775  9123 B3A2 389B 9DE2 334A

On Sat, Dec 22, 2018 at 1:14 PM Kevin 'ldir' Darbyshire-Bryant
<ldir@darbyshire-bryant.me.uk> wrote:
>
>
>
> > On 22 Dec 2018, at 18:28, Hauke Mehrtens <hauke@hauke-m.de> wrote:
> >
> >
> > Hi Yousong,
> >
> > ASLR is currently not activated by default in OpenWrt, so the binary itself is not randomized. Activate CONFIG_PKG_ASLR_PIE to compile Openwrt with ASLR, but this increases the size of the binary.
> >
> > I haven't understood why some parts of the busybox binary and other binaries are mapped rwx, when I look into it with readelf no section is mapped rwx, but it looks like some sections are ending at an not page aligned offset and the next section starts directly after that. I assume that Linux merges the permissions when one page needs different permissions.
> >
> > I am still not sure if the common mips CPUs (24Kec, 74Kec) support restricting execution on pages anyway.
> >
> > Huake
>
> At the risk of going further down the rabbit hole/off topic, if you set the cpu_has_rixi to 1 in  target/linux/ath79/patches-4.14/0014-MIPS-ath79-finetune-cpu-overrides.patch and with PKG_ASLR_PIE [=y]
Hauke mentioned on IRC that CPU_MIPSR2 in the kernel's Kconfig enables
CPU_HAS_RIXI. grep the openwrt for RIXI to see what I mean.

HOWEVER, bcm47xx, ar71xx, ath79, mt7621, and a few others set
cpu_has_rixi to 0. This is significant as several CPUs in OpenWrt like
mt7620 should probably also be setting it if mt7621 is correct for
example.

This has to be bogus. I will definitely be run testing mt7621 device
to see if anything happens.
>
> you get:
> cat /proc/self/maps
> 00400000-0047a000 r-xp 00000000 1f:03 1825       /bin/busybox
> 00489000-0048a000 r--p 00079000 1f:03 1825       /bin/busybox
> 0048a000-0048b000 rw-p 0007a000 1f:03 1825       /bin/busybox
> 77e38000-77e5d000 r-xp 00000000 1f:03 2298       /lib/libgcc_s.so.1
> 77e5d000-77e5e000 rw-p 00015000 1f:03 2298       /lib/libgcc_s.so.1
> 77e5e000-77ef1000 r-xp 00000000 1f:03 2474       /lib/libc.so
> 77f00000-77f02000 rw-p 00092000 1f:03 2474       /lib/libc.so
> 77f02000-77f04000 rw-p 00000000 00:00 0
> 7f9bd000-7f9de000 rw-p 00000000 00:00 0          [stack]
> 7fefb000-7fefc000 r-xp 00000000 00:00 0
> 7ff68000-7ff69000 r--p 00000000 00:00 0          [vvar]
> 7ff69000-7ff6a000 r-xp 00000000 00:00 0          [vdso]
>
>
> The archer hasn’t blown up…….yet
>
> Cheers,
>
> Kevin D-B
>
> 012C ACB2 28C6 C53E 9775  9123 B3A2 389B 9DE2 334A
>
> _______________________________________________
> openwrt-devel mailing list
> openwrt-devel@lists.openwrt.org
> https://lists.openwrt.org/mailman/listinfo/openwrt-devel

I’d suggest putting the champagne away for the moment.  I’m not convinced and I’ll explain why after a bit more checking.

Cheers,

Kevin D-B

012C ACB2 28C6 C53E 9775  9123 B3A2 389B 9DE2 334A

> On 23 Dec 2018, at 09:43, Kevin 'ldir' Darbyshire-Bryant <ldir@darbyshire-bryant.me.uk> wrote:
> 
> I’d suggest putting the champagne away for the moment.  I’m not convinced and I’ll explain why after a bit more checking.

TL;DR - I don’t think the mips processor, certainly in the Archer c7 supports ri/xi.

I dropped the cpu_has_rixi 0 override in 0014-MIPS-ath79-finetune-cpu-overrides.patch.  I then patched the kernel to dump a little bit more info from /proc/cpuinfo:

diff --git a/arch/mips/kernel/proc.c b/arch/mips/kernel/proc.c
index b2de408a2..3ae85c792 100644
--- a/arch/mips/kernel/proc.c
+++ b/arch/mips/kernel/proc.c
@@ -126,6 +126,9 @@ static int show_cpuinfo(struct seq_file *m, void *v)
        if (cpu_has_xpa)        seq_printf(m, "%s", " xpa");
        seq_printf(m, "\n");
 
+       seq_printf(m, "rixi used:\t: %s\n", cpu_has_rixi ?  "yes" : "no");
+       seq_printf(m, "config3:\t: %X %X\n", (read_c0_config3() >> 16), read_c0_config3());
+
        if (cpu_has_mmips) {
                seq_printf(m, "micromips kernel\t: %s\n",
                      (read_c0_config3() & MIPS_CONF3_ISA_OE) ?  "yes" : "no");


cat /proc/cpuinfo 
system type		: Qualcomm Atheros QCA9558 ver 1 rev 0
machine			: TP-Link Archer C7 Version 2
processor		: 0
cpu model		: MIPS 74Kc V5.0
BogoMIPS		: 358.80
wait instruction	: yes
microsecond timers	: yes
tlb_entries		: 32
extra interrupt vector	: yes
hardware watchpoint	: yes, count: 4, address/irw mask: [0x0ffc, 0x0ffc, 0x0ffb, 0x0ffb]
isa			: mips1 mips2 mips32r1 mips32r2
ASEs implemented	: mips16 dsp dsp2
rixi used:	: no
config3:	: 0 2E28
shadow register sets	: 1
kscratch registers	: 0
package			: 0
core			: 0
VCED exceptions		: not available
VCEI exceptions		: not available

According to the doc Hauke linked on page 235 here: https://s3-eu-west-1.amazonaws.com/downloads-mips/I7200/I7200+product+launch/MIPS_nanoMIPS32_PRA_06_09_MD01251.pdf bit 12 of config3 is the flag of interest.  0x2E28 does not set bit 12, so "The RIE and XIE bits are not implemented within the PageGrain register."

There is a remaining mystery related to the config3 register.  The documentation claims it to be 32 bits wide yet none of the top 16 bits are set (and I even shifted them down in case the printf couldn’t cope) so I remain confused.

In terms of the ath79-finetune-cpu-overrides.patch, why do it? Optimisation.  The cpu_has_foo options are macros which if forced to 0/1 enable the compile to exclude/include code at build time…so like many things Openwrt it’s a size thing…. well as far as I understand it.

I’m now concentrating on the festive period in the comfort that at least I’ve had a go at backporting the fpu writeable page fix with I think success (yet to be committed) and I have a little more grasp of what rixi means.

Kevin

On Sat, 22 Dec 2018 at 01:21, Kevin 'ldir' Darbyshire-Bryant
<ldir@darbyshire-bryant.me.uk> wrote:
>
> Backport https://git.kernel.org/pub/scm/linux/kernel/git/mips/linux.git/commit/?id=adcc81f148d733b7e8e641300c5590a2cdc13bf3
>
> "Mapping the delay slot emulation page as both writeable & executable
> presents a security risk, in that if an exploit can write to & jump into
> the page then it can be used as an easy way to execute arbitrary code.
>
> Prevent this by mapping the page read-only for userland, and using
> access_process_vm() with the FOLL_FORCE flag to write to it from
> mips_dsemul().
>
> This will likely be less efficient due to copy_to_user_page() performing
> cache maintenance on a whole page, rather than a single line as in the
> previous use of flush_cache_sigtramp(). However this delay slot
> emulation code ought not to be running in any performance critical paths
> anyway so this isn't really a problem, and we can probably do better in
> copy_to_user_page() anyway in future.
>
> A major advantage of this approach is that the fix is small & simple to
> backport to stable kernels.
>
> Reported-by: Andy Lutomirski <luto@kernel.org>
> Signed-off-by: Paul Burton <paul.burton@mips.com>
> Fixes: 432c6bacbd0c ("MIPS: Use per-mm page to execute branch delay slot instructions")"
>
> Without patch:
>
> cat /proc/self/maps
> 00400000-0047a000 r-xp 00000000 1f:03 1823       /bin/busybox
> 00489000-0048a000 r-xp 00079000 1f:03 1823       /bin/busybox
> 0048a000-0048b000 rwxp 0007a000 1f:03 1823       /bin/busybox
> 77ec8000-77eed000 r-xp 00000000 1f:03 2296       /lib/libgcc_s.so.1
> 77eed000-77eee000 rwxp 00015000 1f:03 2296       /lib/libgcc_s.so.1
> 77eee000-77f81000 r-xp 00000000 1f:03 2470       /lib/libc.so
> 77f90000-77f92000 rwxp 00092000 1f:03 2470       /lib/libc.so
> 77f92000-77f94000 rwxp 00000000 00:00 0
> 7f946000-7f967000 rw-p 00000000 00:00 0          [stack]
> 7fefb000-7fefc000 rwxp 00000000 00:00 0
> 7ffac000-7ffad000 r--p 00000000 00:00 0          [vvar]
> 7ffad000-7ffae000 r-xp 00000000 00:00 0          [vdso]
>
> Patch applied:
>
> cat /proc/self/maps
> 00400000-0047a000 r-xp 00000000 1f:03 1825       /bin/busybox
> 00489000-0048a000 r-xp 00079000 1f:03 1825       /bin/busybox
> 0048a000-0048b000 rwxp 0007a000 1f:03 1825       /bin/busybox
> 77ed0000-77ef5000 r-xp 00000000 1f:03 2298       /lib/libgcc_s.so.1
> 77ef5000-77ef6000 rwxp 00015000 1f:03 2298       /lib/libgcc_s.so.1
> 77ef6000-77f89000 r-xp 00000000 1f:03 2474       /lib/libc.so
> 77f98000-77f9a000 rwxp 00092000 1f:03 2474       /lib/libc.so
> 77f9a000-77f9c000 rwxp 00000000 00:00 0
> 7fbed000-7fc0e000 rw-p 00000000 00:00 0          [stack]
> 7fefb000-7fefc000 r-xp 00000000 00:00 0
> 7fff6000-7fff7000 r--p 00000000 00:00 0          [vvar]
> 7fff7000-7fff8000 r-xp 00000000 00:00 0          [vdso]
>
> Note lack of write permission to 7fefb000-7fefc000
>
> This has received minimal testing on ath79 4.14 Archer C7 v2 only
>
> Signed-off-by: Kevin Darbyshire-Bryant <ldir@darbyshire-bryant.me.uk>

Hi, Kevin

I just took another look at the build system.  On OpenWrt, we have a
pending patch 304-mips_disable_fpu.patch that makes fpu emulation for
mips an optional feature.  We have most mips targets have that feature
disabled.   The absence should be fine as we tell toolchain to use
soft-float explicitly for the whole build.

So another approach would be enhancing the disable-fpu patch to also
not mapping the dsemu page at all.

diff --git a/arch/mips/kernel/process.c b/arch/mips/kernel/process.c
index d4f7fd4550e1..b55676040e22 100644
--- a/arch/mips/kernel/process.c
+++ b/arch/mips/kernel/process.c
@@ -651,8 +651,10 @@ unsigned long mips_stack_top(void)
 {
  unsigned long top = TASK_SIZE & PAGE_MASK;

- /* One page for branch delay slot "emulation" */
- top -= PAGE_SIZE;
+ if (IS_ENABLED(CONFIG_MIPS_FPU_EMULATOR)) {
+ /* One page for branch delay slot "emulation" */
+ top -= PAGE_SIZE;
+ }

  /* Space for the VDSO, data page & GIC user page */
  top -= PAGE_ALIGN(current->thread.abi->vdso->size);
diff --git a/arch/mips/kernel/vdso.c b/arch/mips/kernel/vdso.c
index 48a9c6b90e07..1852440bb43e 100644
--- a/arch/mips/kernel/vdso.c
+++ b/arch/mips/kernel/vdso.c
@@ -124,14 +124,16 @@ int arch_setup_additional_pages(struct
linux_binprm *bprm, int uses_interp)
  if (down_write_killable(&mm->mmap_sem))
  return -EINTR;

- /* Map delay slot emulation page */
- base = mmap_region(NULL, STACK_TOP, PAGE_SIZE,
-    VM_READ|VM_WRITE|VM_EXEC|
-    VM_MAYREAD|VM_MAYWRITE|VM_MAYEXEC,
-    0, NULL);
- if (IS_ERR_VALUE(base)) {
- ret = base;
- goto out;
+ if (IS_ENABLED(CONFIG_MIPS_FPU_EMULATOR)) {
+ /* Map delay slot emulation page */
+ base = mmap_region(NULL, STACK_TOP, PAGE_SIZE,
+    VM_READ|VM_WRITE|VM_EXEC|
+    VM_MAYREAD|VM_MAYWRITE|VM_MAYEXEC,
+    0, NULL);
+ if (IS_ERR_VALUE(base)) {
+ ret = base;
+ goto out;
+ }
  }

  /*

Regards,
                yousong

> On 24 Dec 2018, at 03:53, Yousong Zhou <yszhou4tech@gmail.com> wrote:
> 
>> 
> Hi, Kevin
> 
> I just took another look at the build system.  On OpenWrt, we have a
> pending patch 304-mips_disable_fpu.patch that makes fpu emulation for
> mips an optional feature.  We have most mips targets have that feature
> disabled.   The absence should be fine as we tell toolchain to use
> soft-float explicitly for the whole build.
> 
> So another approach would be enhancing the disable-fpu patch to also
> not mapping the dsemu page at all.
<snip>

ha - you can’t exploit a page that doesn’t exist :-)

Just pushed the backport to openwrt and I have your ‘don’t allocate the page’ patch as an additional commit in my staging tree here https://git.openwrt.org/?p=openwrt/staging/ldir.git;a=summary

It’s all running on my box ok so far

cat /proc/self/maps
00400000-0047a000 r-xp 00000000 1f:03 1825       /bin/busybox
00489000-0048a000 r-xp 00079000 1f:03 1825       /bin/busybox
0048a000-0048b000 rwxp 0007a000 1f:03 1825       /bin/busybox
77e9e000-77ec3000 r-xp 00000000 1f:03 2298       /lib/libgcc_s.so.1
77ec3000-77ec4000 rwxp 00015000 1f:03 2298       /lib/libgcc_s.so.1
77ec4000-77f57000 r-xp 00000000 1f:03 2474       /lib/libc.so
77f66000-77f68000 rwxp 00092000 1f:03 2474       /lib/libc.so
77f68000-77f6a000 rwxp 00000000 00:00 0 
7f744000-7f765000 rw-p 00000000 00:00 0          [stack]
7ff88000-7ff89000 r--p 00000000 00:00 0          [vvar]
7ff89000-7ff8a000 r-xp 00000000 00:00 0          [vdso]

Nothing 7xxxxxxx related has a static address :-)


Cheers,

Kevin D-B

012C ACB2 28C6 C53E 9775  9123 B3A2 389B 9DE2 334A

On Mon, Dec 24, 2018 at 2:16 PM Kevin 'ldir' Darbyshire-Bryant
<ldir@darbyshire-bryant.me.uk> wrote:
>
>
>
> > On 24 Dec 2018, at 03:53, Yousong Zhou <yszhou4tech@gmail.com> wrote:
> >
> >>
> > Hi, Kevin
> >
> > I just took another look at the build system.  On OpenWrt, we have a
> > pending patch 304-mips_disable_fpu.patch that makes fpu emulation for
> > mips an optional feature.  We have most mips targets have that feature
> > disabled.   The absence should be fine as we tell toolchain to use
> > soft-float explicitly for the whole build.
> >
> > So another approach would be enhancing the disable-fpu patch to also
> > not mapping the dsemu page at all.
> <snip>
>
> ha - you can’t exploit a page that doesn’t exist :-)
All of this work reminds me of this old pull request:
https://github.com/openwrt/openwrt/pull/629/commits
>
> Just pushed the backport to openwrt and I have your ‘don’t allocate the page’ patch as an additional commit in my staging tree here https://git.openwrt.org/?p=openwrt/staging/ldir.git;a=summary
>
> It’s all running on my box ok so far
>
> cat /proc/self/maps
> 00400000-0047a000 r-xp 00000000 1f:03 1825       /bin/busybox
> 00489000-0048a000 r-xp 00079000 1f:03 1825       /bin/busybox
> 0048a000-0048b000 rwxp 0007a000 1f:03 1825       /bin/busybox
> 77e9e000-77ec3000 r-xp 00000000 1f:03 2298       /lib/libgcc_s.so.1
> 77ec3000-77ec4000 rwxp 00015000 1f:03 2298       /lib/libgcc_s.so.1
> 77ec4000-77f57000 r-xp 00000000 1f:03 2474       /lib/libc.so
> 77f66000-77f68000 rwxp 00092000 1f:03 2474       /lib/libc.so
> 77f68000-77f6a000 rwxp 00000000 00:00 0
> 7f744000-7f765000 rw-p 00000000 00:00 0          [stack]
> 7ff88000-7ff89000 r--p 00000000 00:00 0          [vvar]
> 7ff89000-7ff8a000 r-xp 00000000 00:00 0          [vdso]
I recommend trying out something that uses floating point
instructions. Say, mp3 decoding or whatnot.

I also wonder if it would be worth experimenting with enabling
floating point on several targets.
>
> Nothing 7xxxxxxx related has a static address :-)
>
>
> Cheers,
>
> Kevin D-B
>
> 012C ACB2 28C6 C53E 9775  9123 B3A2 389B 9DE2 334A
>
> _______________________________________________
> openwrt-devel mailing list
> openwrt-devel@lists.openwrt.org
> https://lists.openwrt.org/mailman/listinfo/openwrt-devel

On Sun, Dec 23, 2018 at 2:27 AM Kevin 'ldir' Darbyshire-Bryant
<ldir@darbyshire-bryant.me.uk> wrote:
>
>
>
> > On 23 Dec 2018, at 09:43, Kevin 'ldir' Darbyshire-Bryant <ldir@darbyshire-bryant.me.uk> wrote:
> >
> > I’d suggest putting the champagne away for the moment.  I’m not convinced and I’ll explain why after a bit more checking.
>
> TL;DR - I don’t think the mips processor, certainly in the Archer c7 supports ri/xi.
I tried out Hauke's patch on mt7621. Both rixiex and rixi show up.
>
> I dropped the cpu_has_rixi 0 override in 0014-MIPS-ath79-finetune-cpu-overrides.patch.  I then patched the kernel to dump a little bit more info from /proc/cpuinfo:
>
> diff --git a/arch/mips/kernel/proc.c b/arch/mips/kernel/proc.c
> index b2de408a2..3ae85c792 100644
> --- a/arch/mips/kernel/proc.c
> +++ b/arch/mips/kernel/proc.c
> @@ -126,6 +126,9 @@ static int show_cpuinfo(struct seq_file *m, void *v)
>         if (cpu_has_xpa)        seq_printf(m, "%s", " xpa");
>         seq_printf(m, "\n");
>
> +       seq_printf(m, "rixi used:\t: %s\n", cpu_has_rixi ?  "yes" : "no");
> +       seq_printf(m, "config3:\t: %X %X\n", (read_c0_config3() >> 16), read_c0_config3());
> +
>         if (cpu_has_mmips) {
>                 seq_printf(m, "micromips kernel\t: %s\n",
>                       (read_c0_config3() & MIPS_CONF3_ISA_OE) ?  "yes" : "no");
>
>
> cat /proc/cpuinfo
> system type             : Qualcomm Atheros QCA9558 ver 1 rev 0
> machine                 : TP-Link Archer C7 Version 2
> processor               : 0
> cpu model               : MIPS 74Kc V5.0
> BogoMIPS                : 358.80
> wait instruction        : yes
> microsecond timers      : yes
> tlb_entries             : 32
> extra interrupt vector  : yes
> hardware watchpoint     : yes, count: 4, address/irw mask: [0x0ffc, 0x0ffc, 0x0ffb, 0x0ffb]
> isa                     : mips1 mips2 mips32r1 mips32r2
> ASEs implemented        : mips16 dsp dsp2
> rixi used:      : no
> config3:        : 0 2E28
> shadow register sets    : 1
> kscratch registers      : 0
> package                 : 0
> core                    : 0
> VCED exceptions         : not available
> VCEI exceptions         : not available
>
> According to the doc Hauke linked on page 235 here: https://s3-eu-west-1.amazonaws.com/downloads-mips/I7200/I7200+product+launch/MIPS_nanoMIPS32_PRA_06_09_MD01251.pdf bit 12 of config3 is the flag of interest.  0x2E28 does not set bit 12, so "The RIE and XIE bits are not implemented within the PageGrain register."
>
> There is a remaining mystery related to the config3 register.  The documentation claims it to be 32 bits wide yet none of the top 16 bits are set (and I even shifted them down in case the printf couldn’t cope) so I remain confused.
>
> In terms of the ath79-finetune-cpu-overrides.patch, why do it? Optimisation.  The cpu_has_foo options are macros which if forced to 0/1 enable the compile to exclude/include code at build time…so like many things Openwrt it’s a size thing…. well as far as I understand it.
>
> I’m now concentrating on the festive period in the comfort that at least I’ve had a go at backporting the fpu writeable page fix with I think success (yet to be committed) and I have a little more grasp of what rixi means.
>
> Kevin
>
>