From: Russell King - ARM Linux <linux@armlinux.org.uk>
To: Thomas Garnier <thgarnie@google.com>
Cc: "Mark Rutland" <mark.rutland@arm.com>,
"David Howells" <dhowells@redhat.com>,
"Dave Hansen" <dave.hansen@intel.com>,
"Arnd Bergmann" <arnd@arndb.de>,
"Al Viro" <viro@zeniv.linux.org.uk>,
"René Nyffenegger" <mail@renenyffenegger.ch>,
"Andrew Morton" <akpm@linux-foundation.org>,
"Kees Cook" <keescook@chromium.org>,
"Paul E . McKenney" <paulmck@linux.vnet.ibm.com>,
"David S . Miller" <davem@davemloft.net>,
"Andy Lutomirski" <luto@kernel.org>,
"Ard Biesheuvel" <ard.biesheuvel@linaro.org>,
"Nicolas Pitre" <nicolas.pitre@linaro.org>,
"Petr Mladek" <pmladek@suse.com>,
"Sebastian Andrzej Siewior" <bigeasy@linutronix.de>,
"Sergey Senozhatsky" <sergey.senozhatsky@gmail.com>,
"Helge Deller" <deller@gmx.de>, "Rik van Riel" <riel@redhat.com>,
"Ingo Molnar" <mingo@kernel.org>,
"Oleg Nesterov" <oleg@redhat.com>,
"John Stultz" <john.stultz@linaro.org>,
"Thomas Gleixner" <tglx@linutronix.de>
Subject: Re: [PATCH v2 4/4] arm64/syscalls: Specific usage of verify_pre_usermode_state
Date: Thu, 9 Mar 2017 17:05:53 +0000 [thread overview]
Message-ID: <20170309170553.GO21222@n2100.armlinux.org.uk> (raw)
In-Reply-To: <CAJcbSZEBu1KnXc-aROvp+EbLpDSAcJV8PUCmNj05OCOtCfSqfA@mail.gmail.com>
On Thu, Mar 09, 2017 at 08:35:30AM -0800, Thomas Garnier wrote:
> On Thu, Mar 9, 2017 at 8:26 AM, Russell King - ARM Linux
> <linux@armlinux.org.uk> wrote:
> > I wouldn't call what you've done on ARM an "optimisation", because my
> > comment about making the fast path worthless still stands.
>
> Why does it still stands on the latest proposal?
It's still having to needlessly save stuff when there's nothing wrong.
Remember, syscalls are a fast path, so the minimum we can do is good.
Calling into C functions is not ideal, because they will tend to be
_very_ expensive compared to hand crafted assembly, especially for
something like this.
It's possible to check the address limit in just three instructions,
which is way less than will be incurred by calling a C function.
Note: This patch is completely untested.
arch/arm/kernel/entry-common.S | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/arch/arm/kernel/entry-common.S b/arch/arm/kernel/entry-common.S
index eb5cd77bf1d8..6b43c6d73117 100644
--- a/arch/arm/kernel/entry-common.S
+++ b/arch/arm/kernel/entry-common.S
@@ -41,8 +41,11 @@
UNWIND(.cantunwind )
disable_irq_notrace @ disable interrupts
ldr r1, [tsk, #TI_FLAGS] @ re-check for syscall tracing
+ ldr r2, [tsk, #TI_ADDR_LIMIT]
tst r1, #_TIF_SYSCALL_WORK | _TIF_WORK_MASK
bne fast_work_pending
+ cmp r2, #TASK_SIZE
+ blgt addr_limit_fail
/* perform architecture specific actions before user return */
arch_ret_to_user r1, lr
@@ -67,6 +70,7 @@ ENDPROC(ret_fast_syscall)
str r0, [sp, #S_R0 + S_OFF]! @ save returned r0
disable_irq_notrace @ disable interrupts
ldr r1, [tsk, #TI_FLAGS] @ re-check for syscall tracing
+ ldr r2, [tsk, #TI_ADDR_LIMIT]
tst r1, #_TIF_SYSCALL_WORK | _TIF_WORK_MASK
beq no_work_pending
UNWIND(.fnend )
@@ -82,6 +86,7 @@ ENDPROC(ret_fast_syscall)
mov r2, why @ 'syscall'
bl do_work_pending
cmp r0, #0
+ ldreq r2, [tsk, #TI_ADDR_LIMIT]
beq no_work_pending
movlt scno, #(__NR_restart_syscall - __NR_SYSCALL_BASE)
ldmia sp, {r0 - r6} @ have to reload r0 - r6
@@ -99,9 +104,12 @@ ENTRY(ret_to_user)
disable_irq_notrace @ disable interrupts
ENTRY(ret_to_user_from_irq)
ldr r1, [tsk, #TI_FLAGS]
+ ldr r2, [tsk, #TI_ADDR_LIMIT]
tst r1, #_TIF_WORK_MASK
bne slow_work_pending
no_work_pending:
+ cmp r2, #TASK_SIZE
+ blgt addr_limit_fail
asm_trace_hardirqs_on save = 0
/* perform architecture specific actions before user return */
@@ -125,6 +133,16 @@ ENTRY(ret_from_fork)
b ret_slow_syscall
ENDPROC(ret_from_fork)
+addr_limit_fail:
+#ifdef CONFIG_BUG_ON_DATA_CORRUPTION
+ stmfd sp!, {r0, lr}
+ bl verify_pre_usermode_state
+ ldmfd sp!, {r0, lr}
+#endif
+ mov r2, #TASK_SIZE
+ str r2, [tsk, #TI_ADDR_LIMIT]
+ ret lr
+
/*=============================================================================
* SWI handler
*-----------------------------------------------------------------------------
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
WARNING: multiple messages have this Message-ID (diff)
From: linux@armlinux.org.uk (Russell King - ARM Linux)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2 4/4] arm64/syscalls: Specific usage of verify_pre_usermode_state
Date: Thu, 9 Mar 2017 17:05:53 +0000 [thread overview]
Message-ID: <20170309170553.GO21222@n2100.armlinux.org.uk> (raw)
In-Reply-To: <CAJcbSZEBu1KnXc-aROvp+EbLpDSAcJV8PUCmNj05OCOtCfSqfA@mail.gmail.com>
On Thu, Mar 09, 2017 at 08:35:30AM -0800, Thomas Garnier wrote:
> On Thu, Mar 9, 2017 at 8:26 AM, Russell King - ARM Linux
> <linux@armlinux.org.uk> wrote:
> > I wouldn't call what you've done on ARM an "optimisation", because my
> > comment about making the fast path worthless still stands.
>
> Why does it still stands on the latest proposal?
It's still having to needlessly save stuff when there's nothing wrong.
Remember, syscalls are a fast path, so the minimum we can do is good.
Calling into C functions is not ideal, because they will tend to be
_very_ expensive compared to hand crafted assembly, especially for
something like this.
It's possible to check the address limit in just three instructions,
which is way less than will be incurred by calling a C function.
Note: This patch is completely untested.
arch/arm/kernel/entry-common.S | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/arch/arm/kernel/entry-common.S b/arch/arm/kernel/entry-common.S
index eb5cd77bf1d8..6b43c6d73117 100644
--- a/arch/arm/kernel/entry-common.S
+++ b/arch/arm/kernel/entry-common.S
@@ -41,8 +41,11 @@
UNWIND(.cantunwind )
disable_irq_notrace @ disable interrupts
ldr r1, [tsk, #TI_FLAGS] @ re-check for syscall tracing
+ ldr r2, [tsk, #TI_ADDR_LIMIT]
tst r1, #_TIF_SYSCALL_WORK | _TIF_WORK_MASK
bne fast_work_pending
+ cmp r2, #TASK_SIZE
+ blgt addr_limit_fail
/* perform architecture specific actions before user return */
arch_ret_to_user r1, lr
@@ -67,6 +70,7 @@ ENDPROC(ret_fast_syscall)
str r0, [sp, #S_R0 + S_OFF]! @ save returned r0
disable_irq_notrace @ disable interrupts
ldr r1, [tsk, #TI_FLAGS] @ re-check for syscall tracing
+ ldr r2, [tsk, #TI_ADDR_LIMIT]
tst r1, #_TIF_SYSCALL_WORK | _TIF_WORK_MASK
beq no_work_pending
UNWIND(.fnend )
@@ -82,6 +86,7 @@ ENDPROC(ret_fast_syscall)
mov r2, why @ 'syscall'
bl do_work_pending
cmp r0, #0
+ ldreq r2, [tsk, #TI_ADDR_LIMIT]
beq no_work_pending
movlt scno, #(__NR_restart_syscall - __NR_SYSCALL_BASE)
ldmia sp, {r0 - r6} @ have to reload r0 - r6
@@ -99,9 +104,12 @@ ENTRY(ret_to_user)
disable_irq_notrace @ disable interrupts
ENTRY(ret_to_user_from_irq)
ldr r1, [tsk, #TI_FLAGS]
+ ldr r2, [tsk, #TI_ADDR_LIMIT]
tst r1, #_TIF_WORK_MASK
bne slow_work_pending
no_work_pending:
+ cmp r2, #TASK_SIZE
+ blgt addr_limit_fail
asm_trace_hardirqs_on save = 0
/* perform architecture specific actions before user return */
@@ -125,6 +133,16 @@ ENTRY(ret_from_fork)
b ret_slow_syscall
ENDPROC(ret_from_fork)
+addr_limit_fail:
+#ifdef CONFIG_BUG_ON_DATA_CORRUPTION
+ stmfd sp!, {r0, lr}
+ bl verify_pre_usermode_state
+ ldmfd sp!, {r0, lr}
+#endif
+ mov r2, #TASK_SIZE
+ str r2, [tsk, #TI_ADDR_LIMIT]
+ ret lr
+
/*=============================================================================
* SWI handler
*-----------------------------------------------------------------------------
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
WARNING: multiple messages have this Message-ID (diff)
From: Russell King - ARM Linux <linux@armlinux.org.uk>
To: Thomas Garnier <thgarnie@google.com>
Cc: "Mark Rutland" <mark.rutland@arm.com>,
"David Howells" <dhowells@redhat.com>,
"Dave Hansen" <dave.hansen@intel.com>,
"Arnd Bergmann" <arnd@arndb.de>,
"Al Viro" <viro@zeniv.linux.org.uk>,
"René Nyffenegger" <mail@renenyffenegger.ch>,
"Andrew Morton" <akpm@linux-foundation.org>,
"Kees Cook" <keescook@chromium.org>,
"Paul E . McKenney" <paulmck@linux.vnet.ibm.com>,
"David S . Miller" <davem@davemloft.net>,
"Andy Lutomirski" <luto@kernel.org>,
"Ard Biesheuvel" <ard.biesheuvel@linaro.org>,
"Nicolas Pitre" <nicolas.pitre@linaro.org>,
"Petr Mladek" <pmladek@suse.com>,
"Sebastian Andrzej Siewior" <bigeasy@linutronix.de>,
"Sergey Senozhatsky" <sergey.senozhatsky@gmail.com>,
"Helge Deller" <deller@gmx.de>, "Rik van Riel" <riel@redhat.com>,
"Ingo Molnar" <mingo@kernel.org>,
"Oleg Nesterov" <oleg@redhat.com>,
"John Stultz" <john.stultz@linaro.org>,
"Thomas Gleixner" <tglx@linutronix.de>,
"Pavel Tikhomirov" <ptikhomirov@virtuozzo.com>,
"Frederic Weisbecker" <fweisbec@gmail.com>,
"Stephen Smalley" <sds@tycho.nsa.gov>,
"Stanislav Kinsburskiy" <skinsbursky@virtuozzo.com>,
"Ingo Molnar" <mingo@redhat.com>,
"H . Peter Anvin" <hpa@zytor.com>,
"Paolo Bonzini" <pbonzini@redhat.com>,
"Borislav Petkov" <bp@alien8.de>,
"Josh Poimboeuf" <jpoimboe@redhat.com>,
"Brian Gerst" <brgerst@gmail.com>,
"Jan Beulich" <JBeulich@suse.com>,
"Christian Borntraeger" <borntraeger@de.ibm.com>,
"Luis R . Rodriguez" <mcgrof@kernel.org>,
"He Chen" <he.chen@linux.intel.com>,
"Will Deacon" <will.deacon@arm.com>,
"Catalin Marinas" <catalin.marinas@arm.com>,
"James Morse" <james.morse@arm.com>,
"Pratyush Anand" <panand@redhat.com>,
"Vladimir Murzin" <vladimir.murzin@arm.com>,
"Chris Metcalf" <cmetcalf@mellanox.com>,
"Andre Przywara" <andre.przywara@arm.com>,
"Linux API" <linux-api@vger.kernel.org>,
LKML <linux-kernel@vger.kernel.org>,
"the arch/x86 maintainers" <x86@kernel.org>,
linux-arm-kernel@lists.infradead.org,
"Kernel Hardening" <kernel-hardening@lists.openwall.com>
Subject: [kernel-hardening] Re: [PATCH v2 4/4] arm64/syscalls: Specific usage of verify_pre_usermode_state
Date: Thu, 9 Mar 2017 17:05:53 +0000 [thread overview]
Message-ID: <20170309170553.GO21222@n2100.armlinux.org.uk> (raw)
In-Reply-To: <CAJcbSZEBu1KnXc-aROvp+EbLpDSAcJV8PUCmNj05OCOtCfSqfA@mail.gmail.com>
On Thu, Mar 09, 2017 at 08:35:30AM -0800, Thomas Garnier wrote:
> On Thu, Mar 9, 2017 at 8:26 AM, Russell King - ARM Linux
> <linux@armlinux.org.uk> wrote:
> > I wouldn't call what you've done on ARM an "optimisation", because my
> > comment about making the fast path worthless still stands.
>
> Why does it still stands on the latest proposal?
It's still having to needlessly save stuff when there's nothing wrong.
Remember, syscalls are a fast path, so the minimum we can do is good.
Calling into C functions is not ideal, because they will tend to be
_very_ expensive compared to hand crafted assembly, especially for
something like this.
It's possible to check the address limit in just three instructions,
which is way less than will be incurred by calling a C function.
Note: This patch is completely untested.
arch/arm/kernel/entry-common.S | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/arch/arm/kernel/entry-common.S b/arch/arm/kernel/entry-common.S
index eb5cd77bf1d8..6b43c6d73117 100644
--- a/arch/arm/kernel/entry-common.S
+++ b/arch/arm/kernel/entry-common.S
@@ -41,8 +41,11 @@
UNWIND(.cantunwind )
disable_irq_notrace @ disable interrupts
ldr r1, [tsk, #TI_FLAGS] @ re-check for syscall tracing
+ ldr r2, [tsk, #TI_ADDR_LIMIT]
tst r1, #_TIF_SYSCALL_WORK | _TIF_WORK_MASK
bne fast_work_pending
+ cmp r2, #TASK_SIZE
+ blgt addr_limit_fail
/* perform architecture specific actions before user return */
arch_ret_to_user r1, lr
@@ -67,6 +70,7 @@ ENDPROC(ret_fast_syscall)
str r0, [sp, #S_R0 + S_OFF]! @ save returned r0
disable_irq_notrace @ disable interrupts
ldr r1, [tsk, #TI_FLAGS] @ re-check for syscall tracing
+ ldr r2, [tsk, #TI_ADDR_LIMIT]
tst r1, #_TIF_SYSCALL_WORK | _TIF_WORK_MASK
beq no_work_pending
UNWIND(.fnend )
@@ -82,6 +86,7 @@ ENDPROC(ret_fast_syscall)
mov r2, why @ 'syscall'
bl do_work_pending
cmp r0, #0
+ ldreq r2, [tsk, #TI_ADDR_LIMIT]
beq no_work_pending
movlt scno, #(__NR_restart_syscall - __NR_SYSCALL_BASE)
ldmia sp, {r0 - r6} @ have to reload r0 - r6
@@ -99,9 +104,12 @@ ENTRY(ret_to_user)
disable_irq_notrace @ disable interrupts
ENTRY(ret_to_user_from_irq)
ldr r1, [tsk, #TI_FLAGS]
+ ldr r2, [tsk, #TI_ADDR_LIMIT]
tst r1, #_TIF_WORK_MASK
bne slow_work_pending
no_work_pending:
+ cmp r2, #TASK_SIZE
+ blgt addr_limit_fail
asm_trace_hardirqs_on save = 0
/* perform architecture specific actions before user return */
@@ -125,6 +133,16 @@ ENTRY(ret_from_fork)
b ret_slow_syscall
ENDPROC(ret_from_fork)
+addr_limit_fail:
+#ifdef CONFIG_BUG_ON_DATA_CORRUPTION
+ stmfd sp!, {r0, lr}
+ bl verify_pre_usermode_state
+ ldmfd sp!, {r0, lr}
+#endif
+ mov r2, #TASK_SIZE
+ str r2, [tsk, #TI_ADDR_LIMIT]
+ ret lr
+
/*=============================================================================
* SWI handler
*-----------------------------------------------------------------------------
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
next prev parent reply other threads:[~2017-03-09 17:05 UTC|newest]
Thread overview: 60+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-03-09 1:24 [PATCH v2 1/4] syscalls: Restore address limit after a syscall Thomas Garnier
2017-03-09 1:24 ` [kernel-hardening] " Thomas Garnier
2017-03-09 1:24 ` [PATCH v2 2/4] x86/syscalls: Specific usage of verify_pre_usermode_state Thomas Garnier
2017-03-09 1:24 ` [kernel-hardening] " Thomas Garnier
2017-03-09 1:24 ` [PATCH v2 3/4] arm/syscalls: " Thomas Garnier
2017-03-09 1:24 ` [kernel-hardening] " Thomas Garnier
2017-03-09 1:24 ` [PATCH v2 4/4] arm64/syscalls: " Thomas Garnier
2017-03-09 1:24 ` [kernel-hardening] " Thomas Garnier
2017-03-09 12:23 ` Mark Rutland
2017-03-09 12:23 ` [kernel-hardening] " Mark Rutland
2017-03-09 12:23 ` Mark Rutland
2017-03-09 15:56 ` Thomas Garnier
2017-03-09 15:56 ` [kernel-hardening] " Thomas Garnier
2017-03-09 15:56 ` Thomas Garnier
2017-03-09 16:05 ` Mark Rutland
2017-03-09 16:05 ` [kernel-hardening] " Mark Rutland
2017-03-09 16:05 ` Mark Rutland
2017-03-09 16:19 ` Thomas Garnier
2017-03-09 16:19 ` [kernel-hardening] " Thomas Garnier
2017-03-09 16:19 ` Thomas Garnier
2017-03-09 16:26 ` Russell King - ARM Linux
2017-03-09 16:26 ` [kernel-hardening] " Russell King - ARM Linux
2017-03-09 16:26 ` Russell King - ARM Linux
2017-03-09 16:35 ` Thomas Garnier
2017-03-09 16:35 ` [kernel-hardening] " Thomas Garnier
2017-03-09 16:35 ` Thomas Garnier
2017-03-09 17:05 ` Russell King - ARM Linux [this message]
2017-03-09 17:05 ` [kernel-hardening] " Russell King - ARM Linux
2017-03-09 17:05 ` Russell King - ARM Linux
2017-03-09 8:42 ` [PATCH v2 1/4] syscalls: Restore address limit after a syscall Borislav Petkov
2017-03-09 8:42 ` [kernel-hardening] " Borislav Petkov
2017-03-09 8:42 ` Borislav Petkov
2017-03-09 15:48 ` Thomas Garnier
2017-03-09 15:48 ` [kernel-hardening] " Thomas Garnier
2017-03-09 15:48 ` Thomas Garnier
2017-03-09 17:27 ` Andy Lutomirski
2017-03-09 17:27 ` [kernel-hardening] " Andy Lutomirski
2017-03-09 17:41 ` Thomas Garnier
2017-03-09 17:41 ` [kernel-hardening] " Thomas Garnier
2017-03-09 10:39 ` Sergey Senozhatsky
2017-03-09 10:39 ` [kernel-hardening] " Sergey Senozhatsky
2017-03-09 12:09 ` Mark Rutland
2017-03-09 12:09 ` [kernel-hardening] " Mark Rutland
2017-03-09 12:09 ` Mark Rutland
2017-03-09 13:44 ` Russell King - ARM Linux
2017-03-09 13:44 ` [kernel-hardening] " Russell King - ARM Linux
2017-03-09 13:44 ` Russell King - ARM Linux
2017-03-09 15:21 ` Mark Rutland
2017-03-09 15:21 ` [kernel-hardening] " Mark Rutland
2017-03-09 15:21 ` Mark Rutland
2017-03-09 15:54 ` Thomas Garnier
2017-03-09 15:54 ` [kernel-hardening] " Thomas Garnier
2017-03-09 15:54 ` Thomas Garnier
2017-03-09 15:52 ` Thomas Garnier
2017-03-09 15:52 ` [kernel-hardening] " Thomas Garnier
2017-03-09 15:52 ` Thomas Garnier
2017-03-09 12:32 ` Christian Borntraeger
2017-03-09 12:32 ` [kernel-hardening] " Christian Borntraeger
2017-03-09 15:53 ` Thomas Garnier
2017-03-09 15:53 ` [kernel-hardening] " 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=20170309170553.GO21222@n2100.armlinux.org.uk \
--to=linux@armlinux.org.uk \
--cc=akpm@linux-foundation.org \
--cc=ard.biesheuvel@linaro.org \
--cc=arnd@arndb.de \
--cc=bigeasy@linutronix.de \
--cc=dave.hansen@intel.com \
--cc=davem@davemloft.net \
--cc=deller@gmx.de \
--cc=dhowells@redhat.com \
--cc=john.stultz@linaro.org \
--cc=keescook@chromium.org \
--cc=luto@kernel.org \
--cc=mail@renenyffenegger.ch \
--cc=mark.rutland@arm.com \
--cc=mingo@kernel.org \
--cc=nicolas.pitre@linaro.org \
--cc=oleg@redhat.com \
--cc=paulmck@linux.vnet.ibm.com \
--cc=pmladek@suse.com \
--cc=riel@redhat.com \
--cc=sergey.senozhatsky@gmail.com \
--cc=tglx@linutronix.de \
--cc=thgarnie@google.com \
--cc=viro@zeniv.linux.org.uk \
/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 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.