Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1240075 > unrolled thread
| Started by | Andy Lutomirski <luto@kernel.org> |
|---|---|
| First post | 2015-10-06 03:00 +0200 |
| Last post | 2015-10-07 18:20 +0200 |
| Articles | 8 — 6 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
[PATCH v2 05/36] x86/entry/64/compat: Fix SYSENTER's NT flag before user memory access Andy Lutomirski <luto@kernel.org> - 2015-10-06 03:00 +0200
Re: [PATCH v2 05/36] x86/entry/64/compat: Fix SYSENTER's NT flag before user memory access Borislav Petkov <bp@alien8.de> - 2015-10-07 13:20 +0200
Re: [PATCH v2 05/36] x86/entry/64/compat: Fix SYSENTER's NT flag before user memory access Brian Gerst <brgerst@gmail.com> - 2015-10-07 16:40 +0200
Re: [PATCH v2 05/36] x86/entry/64/compat: Fix SYSENTER's NT flag before user memory access Borislav Petkov <bp@alien8.de> - 2015-10-07 17:10 +0200
[PATCH] x86/entry/64/compat: Document sysenter_fix_flags's reason for existence Borislav Petkov <bp@alien8.de> - 2015-10-09 19:10 +0200
Re: [PATCH] x86/entry/64/compat: Document sysenter_fix_flags's reason for existence Andy Lutomirski <luto@amacapital.net> - 2015-10-09 21:10 +0200
[tip:x86/asm] x86/entry/64/compat: Document sysenter_fix_flags' s reason for existence tip-bot for Borislav Petkov <tipbot@zytor.com> - 2015-10-11 11:20 +0200
[tip:x86/asm] x86/entry/64/compat: Fix SYSENTER' s NT flag before user memory access tip-bot for Andy Lutomirski <tipbot@zytor.com> - 2015-10-07 18:20 +0200
| From | Andy Lutomirski <luto@kernel.org> |
|---|---|
| Date | 2015-10-06 03:00 +0200 |
| Subject | [PATCH v2 05/36] x86/entry/64/compat: Fix SYSENTER's NT flag before user memory access |
| Message-ID | <qgoLN-7cw-31@gated-at.bofh.it> |
Clearing NT is part of the prologue, whereas loading up arg6 makes more sense to think about as part of syscall processing. Reorder them. Signed-off-by: Andy Lutomirski <luto@kernel.org> --- arch/x86/entry/entry_64_compat.S | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/arch/x86/entry/entry_64_compat.S b/arch/x86/entry/entry_64_compat.S index a9360d40fb7f..aa76864a8a6b 100644 --- a/arch/x86/entry/entry_64_compat.S +++ b/arch/x86/entry/entry_64_compat.S @@ -89,15 +89,6 @@ ENTRY(entry_SYSENTER_compat) sub $(10*8), %rsp /* pt_regs->r8-11, bp, bx, r12-15 not saved */ /* - * no need to do an access_ok check here because rbp has been - * 32-bit zero extended - */ - ASM_STAC -1: movl (%rbp), %ebp - _ASM_EXTABLE(1b, ia32_badarg) - ASM_CLAC - - /* * Sysenter doesn't filter flags, so we need to clear NT * ourselves. To save a few cycles, we can check whether * NT was set instead of doing an unconditional popfq. @@ -106,6 +97,15 @@ ENTRY(entry_SYSENTER_compat) jnz sysenter_fix_flags sysenter_flags_fixed: + /* + * no need to do an access_ok check here because rbp has been + * 32-bit zero extended + */ + ASM_STAC +1: movl (%rbp), %ebp + _ASM_EXTABLE(1b, ia32_badarg) + ASM_CLAC + orl $TS_COMPAT, ASM_THREAD_INFO(TI_status, %rsp, SIZEOF_PTREGS) testl $_TIF_WORK_SYSCALL_ENTRY, ASM_THREAD_INFO(TI_flags, %rsp, SIZEOF_PTREGS) jnz sysenter_tracesys -- 2.4.3 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [next] | [standalone]
| From | Borislav Petkov <bp@alien8.de> |
|---|---|
| Date | 2015-10-07 13:20 +0200 |
| Subject | Re: [PATCH v2 05/36] x86/entry/64/compat: Fix SYSENTER's NT flag before user memory access |
| Message-ID | <qgUVj-39p-9@gated-at.bofh.it> |
| In reply to | #1240075 |
On Mon, Oct 05, 2015 at 05:47:53PM -0700, Andy Lutomirski wrote:
> Clearing NT is part of the prologue, whereas loading up arg6 makes
> more sense to think about as part of syscall processing. Reorder
> them.
>
> Signed-off-by: Andy Lutomirski <luto@kernel.org>
> ---
> arch/x86/entry/entry_64_compat.S | 18 +++++++++---------
> 1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/arch/x86/entry/entry_64_compat.S b/arch/x86/entry/entry_64_compat.S
> index a9360d40fb7f..aa76864a8a6b 100644
> --- a/arch/x86/entry/entry_64_compat.S
> +++ b/arch/x86/entry/entry_64_compat.S
> @@ -89,15 +89,6 @@ ENTRY(entry_SYSENTER_compat)
> sub $(10*8), %rsp /* pt_regs->r8-11, bp, bx, r12-15 not saved */
>
> /*
> - * no need to do an access_ok check here because rbp has been
> - * 32-bit zero extended
> - */
> - ASM_STAC
> -1: movl (%rbp), %ebp
> - _ASM_EXTABLE(1b, ia32_badarg)
> - ASM_CLAC
> -
> - /*
> * Sysenter doesn't filter flags, so we need to clear NT
> * ourselves. To save a few cycles, we can check whether
> * NT was set instead of doing an unconditional popfq.
> @@ -106,6 +97,15 @@ ENTRY(entry_SYSENTER_compat)
> jnz sysenter_fix_flags
> sysenter_flags_fixed:
Btw, do I see it correctly that we can save us this jumping to
sysenter_fix_flags and back to sysenter_flags_fixed? Label is jumped to
only once and the couple of insns there can be behind a JZ...
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Brian Gerst <brgerst@gmail.com> |
|---|---|
| Date | 2015-10-07 16:40 +0200 |
| Subject | Re: [PATCH v2 05/36] x86/entry/64/compat: Fix SYSENTER's NT flag before user memory access |
| Message-ID | <qgY2S-7BV-39@gated-at.bofh.it> |
| In reply to | #1241380 |
On Wed, Oct 7, 2015 at 7:10 AM, Borislav Petkov <bp@alien8.de> wrote: > On Mon, Oct 05, 2015 at 05:47:53PM -0700, Andy Lutomirski wrote: >> Clearing NT is part of the prologue, whereas loading up arg6 makes >> more sense to think about as part of syscall processing. Reorder >> them. >> >> Signed-off-by: Andy Lutomirski <luto@kernel.org> >> --- >> arch/x86/entry/entry_64_compat.S | 18 +++++++++--------- >> 1 file changed, 9 insertions(+), 9 deletions(-) >> >> diff --git a/arch/x86/entry/entry_64_compat.S b/arch/x86/entry/entry_64_compat.S >> index a9360d40fb7f..aa76864a8a6b 100644 >> --- a/arch/x86/entry/entry_64_compat.S >> +++ b/arch/x86/entry/entry_64_compat.S >> @@ -89,15 +89,6 @@ ENTRY(entry_SYSENTER_compat) >> sub $(10*8), %rsp /* pt_regs->r8-11, bp, bx, r12-15 not saved */ >> >> /* >> - * no need to do an access_ok check here because rbp has been >> - * 32-bit zero extended >> - */ >> - ASM_STAC >> -1: movl (%rbp), %ebp >> - _ASM_EXTABLE(1b, ia32_badarg) >> - ASM_CLAC >> - >> - /* >> * Sysenter doesn't filter flags, so we need to clear NT >> * ourselves. To save a few cycles, we can check whether >> * NT was set instead of doing an unconditional popfq. >> @@ -106,6 +97,15 @@ ENTRY(entry_SYSENTER_compat) >> jnz sysenter_fix_flags >> sysenter_flags_fixed: > > Btw, do I see it correctly that we can save us this jumping to > sysenter_fix_flags and back to sysenter_flags_fixed? Label is jumped to > only once and the couple of insns there can be behind a JZ... This is an optimization, since NT is unlikely to be set. The forward branch is usually predicted as not taken, and it keeps these rarely used instructions out of the hot cache. -- Brian Gerst -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Borislav Petkov <bp@alien8.de> |
|---|---|
| Date | 2015-10-07 17:10 +0200 |
| Subject | Re: [PATCH v2 05/36] x86/entry/64/compat: Fix SYSENTER's NT flag before user memory access |
| Message-ID | <qgYvU-8pw-5@gated-at.bofh.it> |
| In reply to | #1241568 |
On Wed, Oct 07, 2015 at 10:33:10AM -0400, Brian Gerst wrote:
> This is an optimization, since NT is unlikely to be set. The forward
> branch is usually predicted as not taken, and it keeps these rarely
> used instructions out of the hot cache.
Ah, slick. Because branch predictors mark new branches as not taken and
this particular one will remain non-taken in the majority of the cases.
I think this warrants a mention in the comment above it though...
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Borislav Petkov <bp@alien8.de> |
|---|---|
| Date | 2015-10-09 19:10 +0200 |
| Subject | [PATCH] x86/entry/64/compat: Document sysenter_fix_flags's reason for existence |
| Message-ID | <qhJl8-8pE-21@gated-at.bofh.it> |
| In reply to | #1241581 |
From: Borislav Petkov <bp@suse.de>
Date: Fri, 9 Oct 2015 19:04:09 +0200
Subject: [PATCH] x86/entry/64/compat: Document sysenter_fix_flags's reason for
existence
The code under the label can normally be inline, without the jumping
back and forth but the latter is an optimization. Document that.
Signed-off-by: Borislav Petkov <bp@suse.de>
---
arch/x86/entry/entry_64_compat.S | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/arch/x86/entry/entry_64_compat.S b/arch/x86/entry/entry_64_compat.S
index cf9641cd4796..92b0b27b43c6 100644
--- a/arch/x86/entry/entry_64_compat.S
+++ b/arch/x86/entry/entry_64_compat.S
@@ -102,6 +102,12 @@ ENTRY(entry_SYSENTER_compat)
* NT was set instead of doing an unconditional popfq.
* This needs to happen before enabling interrupts so that
* we don't get preempted with NT set.
+ *
+ * NB.: sysenter_fix_flags is a label with the code under it moved
+ * out-of-line as an optimization: NT is unlikely to be set in the
+ * majority of the cases and instead of polluting the I$ unnecessarily,
+ * we're keeping that code behind a branch which will predict as
+ * not-taken and therefore its instructions won't be fetched.
*/
testl $X86_EFLAGS_NT, EFLAGS(%rsp)
jnz sysenter_fix_flags
--
2.3.5
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Andy Lutomirski <luto@amacapital.net> |
|---|---|
| Date | 2015-10-09 21:10 +0200 |
| Subject | Re: [PATCH] x86/entry/64/compat: Document sysenter_fix_flags's reason for existence |
| Message-ID | <qhLdg-2EM-15@gated-at.bofh.it> |
| In reply to | #1243541 |
On Fri, Oct 9, 2015 at 10:08 AM, Borislav Petkov <bp@alien8.de> wrote: > From: Borislav Petkov <bp@suse.de> > Date: Fri, 9 Oct 2015 19:04:09 +0200 > Subject: [PATCH] x86/entry/64/compat: Document sysenter_fix_flags's reason for > existence > > The code under the label can normally be inline, without the jumping > back and forth but the latter is an optimization. Document that. Acked-by: Andy Lutomirski <luto@kernel.org> --Andy -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | tip-bot for Borislav Petkov <tipbot@zytor.com> |
|---|---|
| Date | 2015-10-11 11:20 +0200 |
| Subject | [tip:x86/asm] x86/entry/64/compat: Document sysenter_fix_flags' s reason for existence |
| Message-ID | <qikXo-3XV-17@gated-at.bofh.it> |
| In reply to | #1243541 |
Commit-ID: 374a3a3916a70fc6236bc2b8f8ac02548a128a54 Gitweb: http://git.kernel.org/tip/374a3a3916a70fc6236bc2b8f8ac02548a128a54 Author: Borislav Petkov <bp@alien8.de> AuthorDate: Fri, 9 Oct 2015 19:08:59 +0200 Committer: Ingo Molnar <mingo@kernel.org> CommitDate: Sun, 11 Oct 2015 11:06:40 +0200 x86/entry/64/compat: Document sysenter_fix_flags's reason for existence The code under the label can normally be inline, without the jumping back and forth but the latter is an optimization. Document that. Signed-off-by: Borislav Petkov <bp@suse.de> Acked-by: Andy Lutomirski <luto@kernel.org> Cc: Andy Lutomirski <luto@amacapital.net> Cc: Borislav Petkov <bp@alien8.de> Cc: Brian Gerst <brgerst@gmail.com> Cc: Denys Vlasenko <dvlasenk@redhat.com> Cc: H. Peter Anvin <hpa@zytor.com> Cc: Linus Torvalds <torvalds@linux-foundation.org> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Thomas Gleixner <tglx@linutronix.de> Link: http://lkml.kernel.org/r/20151009170859.GA24266@pd.tnic Signed-off-by: Ingo Molnar <mingo@kernel.org> --- arch/x86/entry/entry_64_compat.S | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/arch/x86/entry/entry_64_compat.S b/arch/x86/entry/entry_64_compat.S index cf9641c..92b0b27 100644 --- a/arch/x86/entry/entry_64_compat.S +++ b/arch/x86/entry/entry_64_compat.S @@ -102,6 +102,12 @@ ENTRY(entry_SYSENTER_compat) * NT was set instead of doing an unconditional popfq. * This needs to happen before enabling interrupts so that * we don't get preempted with NT set. + * + * NB.: sysenter_fix_flags is a label with the code under it moved + * out-of-line as an optimization: NT is unlikely to be set in the + * majority of the cases and instead of polluting the I$ unnecessarily, + * we're keeping that code behind a branch which will predict as + * not-taken and therefore its instructions won't be fetched. */ testl $X86_EFLAGS_NT, EFLAGS(%rsp) jnz sysenter_fix_flags -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | tip-bot for Andy Lutomirski <tipbot@zytor.com> |
|---|---|
| Date | 2015-10-07 18:20 +0200 |
| Subject | [tip:x86/asm] x86/entry/64/compat: Fix SYSENTER' s NT flag before user memory access |
| Message-ID | <qgZBE-1xt-11@gated-at.bofh.it> |
| In reply to | #1240075 |
Commit-ID: dd27f998f0ed3c797032a82033fa191be7c61e4c Gitweb: http://git.kernel.org/tip/dd27f998f0ed3c797032a82033fa191be7c61e4c Author: Andy Lutomirski <luto@kernel.org> AuthorDate: Mon, 5 Oct 2015 17:47:53 -0700 Committer: Ingo Molnar <mingo@kernel.org> CommitDate: Wed, 7 Oct 2015 11:34:07 +0200 x86/entry/64/compat: Fix SYSENTER's NT flag before user memory access Clearing NT is part of the prologue, whereas loading up arg6 makes more sense to think about as part of syscall processing. Reorder them. Signed-off-by: Andy Lutomirski <luto@kernel.org> Cc: Andy Lutomirski <luto@amacapital.net> Cc: Borislav Petkov <bp@alien8.de> Cc: Brian Gerst <brgerst@gmail.com> Cc: Denys Vlasenko <dvlasenk@redhat.com> Cc: H. Peter Anvin <hpa@zytor.com> Cc: Linus Torvalds <torvalds@linux-foundation.org> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Thomas Gleixner <tglx@linutronix.de> Cc: linux-kernel@vger.kernel.org Link: http://lkml.kernel.org/r/19eb235828b2d2a52c53459e09f2974e15e65a35.1444091584.git.luto@kernel.org Signed-off-by: Ingo Molnar <mingo@kernel.org> --- arch/x86/entry/entry_64_compat.S | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/arch/x86/entry/entry_64_compat.S b/arch/x86/entry/entry_64_compat.S index a9360d4..e2cca89 100644 --- a/arch/x86/entry/entry_64_compat.S +++ b/arch/x86/entry/entry_64_compat.S @@ -89,15 +89,6 @@ ENTRY(entry_SYSENTER_compat) sub $(10*8), %rsp /* pt_regs->r8-11, bp, bx, r12-15 not saved */ /* - * no need to do an access_ok check here because rbp has been - * 32-bit zero extended - */ - ASM_STAC -1: movl (%rbp), %ebp - _ASM_EXTABLE(1b, ia32_badarg) - ASM_CLAC - - /* * Sysenter doesn't filter flags, so we need to clear NT * ourselves. To save a few cycles, we can check whether * NT was set instead of doing an unconditional popfq. @@ -106,6 +97,15 @@ ENTRY(entry_SYSENTER_compat) jnz sysenter_fix_flags sysenter_flags_fixed: + /* + * No need to do an access_ok() check here because RBP has been + * 32-bit zero extended: + */ + ASM_STAC +1: movl (%rbp), %ebp + _ASM_EXTABLE(1b, ia32_badarg) + ASM_CLAC + orl $TS_COMPAT, ASM_THREAD_INFO(TI_status, %rsp, SIZEOF_PTREGS) testl $_TIF_WORK_SYSCALL_ENTRY, ASM_THREAD_INFO(TI_flags, %rsp, SIZEOF_PTREGS) jnz sysenter_tracesys -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web