From: Kees Cook <keescook@chromium.org> To: Ingo Molnar <mingo@kernel.org> Cc: Kees Cook <keescook@chromium.org>, Thomas Garnier <thgarnie@google.com>, Thomas Gleixner <tglx@linutronix.de>, Russell King <linux@armlinux.org.uk>, Catalin Marinas <catalin.marinas@arm.com>, Will Deacon <will.deacon@arm.com>, Andy Lutomirski <luto@amacapital.net>, Will Drewry <wad@chromium.org>, Al Viro <viro@zeniv.linux.org.uk>, Dave Martin <Dave.Martin@arm.com>, Pratyush Anand <panand@redhat.com>, Dave Hansen <dave.hansen@intel.com>, Arnd Bergmann <arnd@arndb.de>, David Howells <dhowells@redhat.com>, Yonghong Song <yhs@fb.com>, linux-arm-kernel@lists.infradead.org, linux-api@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH 3/4] arm/syscalls: Optimize address limit check Date: Thu, 7 Sep 2017 08:30:46 -0700 [thread overview] Message-ID: <1504798247-48833-4-git-send-email-keescook@chromium.org> (raw) In-Reply-To: <1504798247-48833-1-git-send-email-keescook@chromium.org> From: Thomas Garnier <thgarnie@google.com> Disable the generic address limit check in favor of an architecture specific optimized implementation. The generic implementation using pending work flags did not work well with ARM and alignment faults. The address limit is checked on each syscall return path to user-mode path as well as the irq user-mode return function. If the address limit was changed, a function is called to report data corruption (stopping the kernel or process based on configuration). The address limit check has to be done before any pending work because they can reset the address limit and the process is killed using a SIGKILL signal. For example the lkdtm address limit check does not work because the signal to kill the process will reset the user-mode address limit. Signed-off-by: Thomas Garnier <thgarnie@google.com> Reviewed-by: Kees Cook <keescook@chromium.org> Tested-by: Kees Cook <keescook@chromium.org> Tested-by: Leonard Crestez <leonard.crestez@nxp.com> Signed-off-by: Kees Cook <keescook@chromium.org> --- arch/arm/kernel/entry-common.S | 11 +++++++++++ arch/arm/kernel/signal.c | 7 +++++++ 2 files changed, 18 insertions(+) diff --git a/arch/arm/kernel/entry-common.S b/arch/arm/kernel/entry-common.S index eb5cd77bf1d8..126fafc725bc 100644 --- a/arch/arm/kernel/entry-common.S +++ b/arch/arm/kernel/entry-common.S @@ -12,6 +12,7 @@ #include <asm/unistd.h> #include <asm/ftrace.h> #include <asm/unwind.h> +#include <asm/memory.h> #ifdef CONFIG_AEABI #include <asm/unistd-oabi.h> #endif @@ -40,10 +41,14 @@ ret_fast_syscall: UNWIND(.fnstart ) UNWIND(.cantunwind ) disable_irq_notrace @ disable interrupts + ldr r2, [tsk, #TI_ADDR_LIMIT] + cmp r2, #TASK_SIZE + blne addr_limit_check_failed ldr r1, [tsk, #TI_FLAGS] @ re-check for syscall tracing tst r1, #_TIF_SYSCALL_WORK | _TIF_WORK_MASK bne fast_work_pending + /* perform architecture specific actions before user return */ arch_ret_to_user r1, lr @@ -66,6 +71,9 @@ ret_fast_syscall: UNWIND(.cantunwind ) str r0, [sp, #S_R0 + S_OFF]! @ save returned r0 disable_irq_notrace @ disable interrupts + ldr r2, [tsk, #TI_ADDR_LIMIT] + cmp r2, #TASK_SIZE + blne addr_limit_check_failed ldr r1, [tsk, #TI_FLAGS] @ re-check for syscall tracing tst r1, #_TIF_SYSCALL_WORK | _TIF_WORK_MASK beq no_work_pending @@ -98,6 +106,9 @@ ENTRY(ret_to_user) ret_slow_syscall: disable_irq_notrace @ disable interrupts ENTRY(ret_to_user_from_irq) + ldr r2, [tsk, #TI_ADDR_LIMIT] + cmp r2, #TASK_SIZE + blne addr_limit_check_failed ldr r1, [tsk, #TI_FLAGS] tst r1, #_TIF_WORK_MASK bne slow_work_pending diff --git a/arch/arm/kernel/signal.c b/arch/arm/kernel/signal.c index 5814298ef0b7..b67ae12503f3 100644 --- a/arch/arm/kernel/signal.c +++ b/arch/arm/kernel/signal.c @@ -14,6 +14,7 @@ #include <linux/uaccess.h> #include <linux/tracehook.h> #include <linux/uprobes.h> +#include <linux/syscalls.h> #include <asm/elf.h> #include <asm/cacheflush.h> @@ -673,3 +674,9 @@ struct page *get_signal_page(void) return page; } + +/* Defer to generic check */ +asmlinkage void addr_limit_check_failed(void) +{ + addr_limit_user_check(); +} -- 2.7.4
WARNING: multiple messages have this Message-ID (diff)
From: keescook@chromium.org (Kees Cook) To: linux-arm-kernel@lists.infradead.org Subject: [PATCH 3/4] arm/syscalls: Optimize address limit check Date: Thu, 7 Sep 2017 08:30:46 -0700 [thread overview] Message-ID: <1504798247-48833-4-git-send-email-keescook@chromium.org> (raw) In-Reply-To: <1504798247-48833-1-git-send-email-keescook@chromium.org> From: Thomas Garnier <thgarnie@google.com> Disable the generic address limit check in favor of an architecture specific optimized implementation. The generic implementation using pending work flags did not work well with ARM and alignment faults. The address limit is checked on each syscall return path to user-mode path as well as the irq user-mode return function. If the address limit was changed, a function is called to report data corruption (stopping the kernel or process based on configuration). The address limit check has to be done before any pending work because they can reset the address limit and the process is killed using a SIGKILL signal. For example the lkdtm address limit check does not work because the signal to kill the process will reset the user-mode address limit. Signed-off-by: Thomas Garnier <thgarnie@google.com> Reviewed-by: Kees Cook <keescook@chromium.org> Tested-by: Kees Cook <keescook@chromium.org> Tested-by: Leonard Crestez <leonard.crestez@nxp.com> Signed-off-by: Kees Cook <keescook@chromium.org> --- arch/arm/kernel/entry-common.S | 11 +++++++++++ arch/arm/kernel/signal.c | 7 +++++++ 2 files changed, 18 insertions(+) diff --git a/arch/arm/kernel/entry-common.S b/arch/arm/kernel/entry-common.S index eb5cd77bf1d8..126fafc725bc 100644 --- a/arch/arm/kernel/entry-common.S +++ b/arch/arm/kernel/entry-common.S @@ -12,6 +12,7 @@ #include <asm/unistd.h> #include <asm/ftrace.h> #include <asm/unwind.h> +#include <asm/memory.h> #ifdef CONFIG_AEABI #include <asm/unistd-oabi.h> #endif @@ -40,10 +41,14 @@ ret_fast_syscall: UNWIND(.fnstart ) UNWIND(.cantunwind ) disable_irq_notrace @ disable interrupts + ldr r2, [tsk, #TI_ADDR_LIMIT] + cmp r2, #TASK_SIZE + blne addr_limit_check_failed ldr r1, [tsk, #TI_FLAGS] @ re-check for syscall tracing tst r1, #_TIF_SYSCALL_WORK | _TIF_WORK_MASK bne fast_work_pending + /* perform architecture specific actions before user return */ arch_ret_to_user r1, lr @@ -66,6 +71,9 @@ ret_fast_syscall: UNWIND(.cantunwind ) str r0, [sp, #S_R0 + S_OFF]! @ save returned r0 disable_irq_notrace @ disable interrupts + ldr r2, [tsk, #TI_ADDR_LIMIT] + cmp r2, #TASK_SIZE + blne addr_limit_check_failed ldr r1, [tsk, #TI_FLAGS] @ re-check for syscall tracing tst r1, #_TIF_SYSCALL_WORK | _TIF_WORK_MASK beq no_work_pending @@ -98,6 +106,9 @@ ENTRY(ret_to_user) ret_slow_syscall: disable_irq_notrace @ disable interrupts ENTRY(ret_to_user_from_irq) + ldr r2, [tsk, #TI_ADDR_LIMIT] + cmp r2, #TASK_SIZE + blne addr_limit_check_failed ldr r1, [tsk, #TI_FLAGS] tst r1, #_TIF_WORK_MASK bne slow_work_pending diff --git a/arch/arm/kernel/signal.c b/arch/arm/kernel/signal.c index 5814298ef0b7..b67ae12503f3 100644 --- a/arch/arm/kernel/signal.c +++ b/arch/arm/kernel/signal.c @@ -14,6 +14,7 @@ #include <linux/uaccess.h> #include <linux/tracehook.h> #include <linux/uprobes.h> +#include <linux/syscalls.h> #include <asm/elf.h> #include <asm/cacheflush.h> @@ -673,3 +674,9 @@ struct page *get_signal_page(void) return page; } + +/* Defer to generic check */ +asmlinkage void addr_limit_check_failed(void) +{ + addr_limit_user_check(); +} -- 2.7.4
next prev parent reply other threads:[~2017-09-07 15:32 UTC|newest] Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top 2017-09-07 15:30 [PATCH 0/4] Fix check address limit on user-mode Kees Cook 2017-09-07 15:30 ` Kees Cook 2017-09-07 15:30 ` [PATCH 1/4] syscalls: Use CHECK_DATA_CORRUPTION for addr_limit_user_check Kees Cook 2017-09-07 15:30 ` Kees Cook 2017-09-17 17:53 ` [tip:core/urgent] " tip-bot for Thomas Garnier 2017-09-07 15:30 ` [PATCH 2/4] Revert "arm/syscalls: Check address limit on user-mode return" Kees Cook 2017-09-07 15:30 ` Kees Cook 2017-09-17 17:54 ` [tip:core/urgent] " tip-bot for Thomas Garnier 2017-09-07 15:30 ` Kees Cook [this message] 2017-09-07 15:30 ` [PATCH 3/4] arm/syscalls: Optimize address limit check Kees Cook 2017-09-17 17:54 ` [tip:core/urgent] " tip-bot for Thomas Garnier 2017-09-07 15:30 ` [PATCH 4/4] arm64/syscalls: Move address limit check in loop Kees Cook 2017-09-07 15:30 ` Kees Cook 2017-09-12 18:27 ` Will Deacon 2017-09-12 18:27 ` Will Deacon 2017-09-12 18:28 ` Kees Cook 2017-09-12 18:28 ` Kees Cook 2017-09-12 18:28 ` Kees Cook 2017-09-13 8:00 ` Ingo Molnar 2017-09-13 8:00 ` Ingo Molnar 2017-09-13 8:00 ` Ingo Molnar 2017-09-17 17:54 ` [tip:core/urgent] " tip-bot for Thomas Garnier
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=1504798247-48833-4-git-send-email-keescook@chromium.org \ --to=keescook@chromium.org \ --cc=Dave.Martin@arm.com \ --cc=arnd@arndb.de \ --cc=catalin.marinas@arm.com \ --cc=dave.hansen@intel.com \ --cc=dhowells@redhat.com \ --cc=linux-api@vger.kernel.org \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux@armlinux.org.uk \ --cc=luto@amacapital.net \ --cc=mingo@kernel.org \ --cc=panand@redhat.com \ --cc=tglx@linutronix.de \ --cc=thgarnie@google.com \ --cc=viro@zeniv.linux.org.uk \ --cc=wad@chromium.org \ --cc=will.deacon@arm.com \ --cc=yhs@fb.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.