From: Catalin Marinas <catalin.marinas@arm.com>
To: Thomas Garnier <thgarnie@google.com>
Cc: "Martin Schwidefsky" <schwidefsky@de.ibm.com>,
"Heiko Carstens" <heiko.carstens@de.ibm.com>,
"Arnd Bergmann" <arnd@arndb.de>,
"Dave Hansen" <dave.hansen@intel.com>,
"Andrew Morton" <akpm@linux-foundation.org>,
"David Howells" <dhowells@redhat.com>,
"René Nyffenegger" <mail@renenyffenegger.ch>,
"Paul E . McKenney" <paulmck@linux.vnet.ibm.com>,
"Ingo Molnar" <mingo@kernel.org>,
"Thomas Gleixner" <tglx@linutronix.de>,
"Oleg Nesterov" <oleg@redhat.com>,
"Stephen Smalley" <sds@tycho.nsa.gov>,
"Pavel Tikhomirov" <ptikhomirov@virtuozzo.com>,
"Ingo Molnar" <mingo@redhat.com>,
"H . Peter Anvin" <hpa@zytor.com>,
"Andy Lutomirski" <luto@kernel.org>,
"Paolo Bonzini" <pbonzini@redhat.com>,
"Kees Cook" <keescook@chromium.org>,
"Rik van Riel" <riel@redhat.com>,
"Josh Poimboeuf" <jpoimboe@redhat.com>,
"Borislav Petkov" <bp@alien8.de>,
"Brian Gerst" <brgerst@gmail.com>
Subject: Re: [PATCH v7 4/4] arm64/syscalls: Architecture specific pre-usermode check
Date: Mon, 10 Apr 2017 18:12:40 +0100 [thread overview]
Message-ID: <20170410171240.GE30647@e104818-lin.cambridge.arm.com> (raw)
In-Reply-To: <20170410164420.64003-4-thgarnie@google.com>
On Mon, Apr 10, 2017 at 09:44:20AM -0700, Thomas Garnier wrote:
> Disable the generic pre-usermode check in favor of an optimized
> implementation. This patch adds specific checks on user-mode return path
> to make it faster and smaller.
>
> The address limit is checked on each syscall return path to user-mode.
> If it was changed, a generic handler is called to stop the kernel on an
> explicit check.
>
> Signed-off-by: Thomas Garnier <thgarnie@google.com>
> ---
> Based on next-20170410
> ---
> arch/arm64/Kconfig | 1 +
> arch/arm64/kernel/entry.S | 13 +++++++++++++
> 2 files changed, 14 insertions(+)
>
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 9b8fcab7da56..3f9e8e7d9376 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -24,6 +24,7 @@ config ARM64
> select ARCH_WANT_COMPAT_IPC_PARSE_VERSION
> select ARCH_WANT_FRAME_POINTERS
> select ARCH_HAS_UBSAN_SANITIZE_ALL
> + select ARCH_NO_SYSCALL_VERIFY_PRE_USERMODE_STATE
> select ARM_AMBA
> select ARM_ARCH_TIMER
> select ARM_GIC
> diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
> index 43512d4d7df2..bdd094c7837a 100644
> --- a/arch/arm64/kernel/entry.S
> +++ b/arch/arm64/kernel/entry.S
> @@ -744,6 +744,8 @@ ENDPROC(cpu_switch_to)
> ret_fast_syscall:
> disable_irq // disable interrupts
> str x0, [sp, #S_X0] // returned x0
> + ldr x2, [tsk, #TSK_TI_ADDR_LIMIT] // check addr limit change
> + tbnz x2, #63, addr_limit_fail
> ldr x1, [tsk, #TSK_TI_FLAGS] // re-check for syscall tracing
> and x2, x1, #_TIF_SYSCALL_WORK
> cbnz x2, ret_fast_syscall_trace
> @@ -771,6 +773,9 @@ work_pending:
> */
> ret_to_user:
> disable_irq // disable interrupts
> + ldr x2, [tsk, #TSK_TI_ADDR_LIMIT] // check addr limit change
> + tbnz x2, #63, addr_limit_fail
> +
Nitpick: unnecessary empty line added.
> ldr x1, [tsk, #TSK_TI_FLAGS]
> and x2, x1, #_TIF_WORK_MASK
> cbnz x2, work_pending
> @@ -780,6 +785,14 @@ finish_ret_to_user:
> ENDPROC(ret_to_user)
>
> /*
> + * Address limit was incorrect before returning in user-mode which could be
> + * used to elevate privileges. Call the generic handler to stop the kernel on
> + * the appropriate check. This function does not return.
> + */
> +addr_limit_fail:
> + bl address_limit_check_failed
You could do a "b address_limit_check_failed" rather than "bl" since we
don't care about returning here (and a nitpick: insert tab between "b"
and "address_limit_check_failed").
With the above:
Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>
next prev parent reply other threads:[~2017-04-10 17:12 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-04-10 16:44 [PATCH v7 1/4] syscalls: Restore address limit after a syscall Thomas Garnier
2017-04-10 16:44 ` [PATCH v7 3/4] arm/syscalls: Architecture specific pre-usermode check Thomas Garnier
2017-04-10 16:44 ` [PATCH v7 4/4] arm64/syscalls: " Thomas Garnier
2017-04-10 17:12 ` Catalin Marinas [this message]
[not found] ` <20170410171240.GE30647-M2fw3Uu6cmfZROr8t4l/smS4ubULX0JqMm0uRHvK7Nw@public.gmane.org>
2017-04-10 20:06 ` Thomas Garnier
[not found] ` <20170410200622.8654-1-thgarnie-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
2017-04-10 20:09 ` Thomas Garnier
2017-04-10 20:07 ` Thomas Garnier
[not found] ` <20170410164420.64003-1-thgarnie-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
2017-04-10 16:44 ` [PATCH v7 2/4] x86/syscalls: " Thomas Garnier
2017-04-24 23:57 ` [PATCH v7 1/4] syscalls: Restore address limit after a syscall Kees Cook
[not found] ` <CAGXu5j+J67zC9tcncacHTDvXKMF3HZuPCivk-Uz4J6h-cEv-qQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-04-25 6:23 ` Ingo Molnar
[not found] ` <20170425062324.pdpi5v7ypobw74ki-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2017-04-25 14:12 ` Thomas Garnier
2017-04-25 6:33 ` Ingo Molnar
[not found] ` <20170425063305.hwjuxupa37rwe6zj-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2017-04-25 14:18 ` Thomas Garnier
2017-04-26 8:12 ` Ingo Molnar
[not found] ` <20170426081229.6wnugrs7w3at4xry-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2017-04-26 14:09 ` 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=20170410171240.GE30647@e104818-lin.cambridge.arm.com \
--to=catalin.marinas@arm.com \
--cc=akpm@linux-foundation.org \
--cc=arnd@arndb.de \
--cc=bp@alien8.de \
--cc=brgerst@gmail.com \
--cc=dave.hansen@intel.com \
--cc=dhowells@redhat.com \
--cc=heiko.carstens@de.ibm.com \
--cc=hpa@zytor.com \
--cc=jpoimboe@redhat.com \
--cc=keescook@chromium.org \
--cc=luto@kernel.org \
--cc=mail@renenyffenegger.ch \
--cc=mingo@kernel.org \
--cc=mingo@redhat.com \
--cc=oleg@redhat.com \
--cc=paulmck@linux.vnet.ibm.com \
--cc=pbonzini@redhat.com \
--cc=ptikhomirov@virtuozzo.com \
--cc=riel@redhat.com \
--cc=schwidefsky@de.ibm.com \
--cc=sds@tycho.nsa.gov \
--cc=tglx@linutronix.de \
--cc=thgarnie@google.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: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).