Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1455732 > unrolled thread

[PATCH] x86/entry: Clarify the RF saving/restoring situation with SYSCALL/SYSRET

Started byBorislav Petkov <bp@alien8.de>
First post2016-08-03 12:30 +0200
Last post2016-08-10 22:50 +0200
Articles 6 — 3 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH] x86/entry: Clarify the RF saving/restoring situation with SYSCALL/SYSRET Borislav Petkov <bp@alien8.de> - 2016-08-03 12:30 +0200
    Re: [PATCH] x86/entry: Clarify the RF saving/restoring situation with SYSCALL/SYSRET Andy Lutomirski <luto@amacapital.net> - 2016-08-03 18:50 +0200
      Re: [PATCH] x86/entry: Clarify the RF saving/restoring situation  with SYSCALL/SYSRET Borislav Petkov <bp@alien8.de> - 2016-08-03 19:20 +0200
        Re: [PATCH] x86/entry: Clarify the RF saving/restoring situation with SYSCALL/SYSRET Andy Lutomirski <luto@amacapital.net> - 2016-08-03 19:30 +0200
          Re: [PATCH] x86/entry: Clarify the RF saving/restoring situation  with SYSCALL/SYSRET Borislav Petkov <bp@alien8.de> - 2016-08-03 20:10 +0200
        [tip:x86/urgent] x86/entry: Clarify the RF saving/restoring  situation with SYSCALL/SYSRET tip-bot for Borislav Petkov <tipbot@zytor.com> - 2016-08-10 22:50 +0200

#1455732 — [PATCH] x86/entry: Clarify the RF saving/restoring situation with SYSCALL/SYSRET

FromBorislav Petkov <bp@alien8.de>
Date2016-08-03 12:30 +0200
Subject[PATCH] x86/entry: Clarify the RF saving/restoring situation with SYSCALL/SYSRET
Message-ID<s21B0-1hs-7@gated-at.bofh.it>
From: Borislav Petkov <bp@suse.de>

Clarify why exactly RF cannot be restored properly by SYSRET to avoid
confusion.

No functionality change.

Signed-off-by: Borislav Petkov <bp@suse.de>
Cc: Andy Lutomirski <luto@amacapital.net>
---
 arch/x86/entry/entry_64.S | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index 8956eae04c25..80ad6d0fe38b 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -288,11 +288,15 @@ return_from_SYSCALL_64:
 	jne	opportunistic_sysret_failed
 
 	/*
-	 * SYSRET can't restore RF.  SYSRET can restore TF, but unlike IRET,
-	 * restoring TF results in a trap from userspace immediately after
-	 * SYSRET.  This would cause an infinite loop whenever #DB happens
-	 * with register state that satisfies the opportunistic SYSRET
-	 * conditions.  For example, single-stepping this user code:
+	 * SYSCALL clears RF when it saves rFLAGS in R11 so SYSRET cannot
+	 * restore RF properly. If the slowpath sets it for whatever reason, we
+	 * need to restore it correctly.
+	 *
+	 * SYSRET can restore TF, but unlike IRET, restoring TF results in a
+	 * trap from userspace immediately after SYSRET.  This would cause an
+	 * infinite loop whenever #DB happens with register state that satisfies
+	 * the opportunistic SYSRET conditions.  For example, single-stepping
+	 * this user code:
 	 *
 	 *           movq	$stuck_here, %rcx
 	 *           pushfq
-- 
2.8.4

[toc] | [next] | [standalone]


#1455896

FromAndy Lutomirski <luto@amacapital.net>
Date2016-08-03 18:50 +0200
Message-ID<s27wJ-4Vz-15@gated-at.bofh.it>
In reply to#1455732
On Wed, Aug 3, 2016 at 3:19 AM, Borislav Petkov <bp@alien8.de> wrote:
> From: Borislav Petkov <bp@suse.de>
>
> Clarify why exactly RF cannot be restored properly by SYSRET to avoid
> confusion.
>
> No functionality change.
>
> Signed-off-by: Borislav Petkov <bp@suse.de>
> Cc: Andy Lutomirski <luto@amacapital.net>
> ---
>  arch/x86/entry/entry_64.S | 14 +++++++++-----
>  1 file changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
> index 8956eae04c25..80ad6d0fe38b 100644
> --- a/arch/x86/entry/entry_64.S
> +++ b/arch/x86/entry/entry_64.S
> @@ -288,11 +288,15 @@ return_from_SYSCALL_64:
>         jne     opportunistic_sysret_failed
>
>         /*
> -        * SYSRET can't restore RF.  SYSRET can restore TF, but unlike IRET,
> -        * restoring TF results in a trap from userspace immediately after
> -        * SYSRET.  This would cause an infinite loop whenever #DB happens
> -        * with register state that satisfies the opportunistic SYSRET
> -        * conditions.  For example, single-stepping this user code:
> +        * SYSCALL clears RF when it saves rFLAGS in R11 so SYSRET cannot

I would change "so" and "and" -- the CPU designers could have make
SYSRET restore RF, but they chose not to.

Other than that substitution:

Acked-by: Andy Lutomirski <luto@kernel.org>

[toc] | [prev] | [next] | [standalone]


#1455906 — Re: [PATCH] x86/entry: Clarify the RF saving/restoring situation with SYSCALL/SYSRET

FromBorislav Petkov <bp@alien8.de>
Date2016-08-03 19:20 +0200
SubjectRe: [PATCH] x86/entry: Clarify the RF saving/restoring situation with SYSCALL/SYSRET
Message-ID<s27ZM-5m7-3@gated-at.bofh.it>
In reply to#1455896
On Wed, Aug 03, 2016 at 09:42:29AM -0700, Andy Lutomirski wrote:
> I would change "so" and "and" -- the CPU designers could have make
> SYSRET restore RF, but they chose not to.

I'm assuming the reasoning behind it was that you should be able to
break after SYSRET but then again, the kernel could've been left in
control of that bit and set it or clear it however it likes before
SYSRETing.

Oh well.

> Other than that substitution:
> 
> Acked-by: Andy Lutomirski <luto@kernel.org>

Thanks.

Here's v1.1:

---
From: Borislav Petkov <bp@suse.de>
Date: Wed, 3 Aug 2016 12:17:10 +0200
Subject: [PATCH -v1.1] x86/entry: Clarify the RF saving/restoring situation with SYSCALL/SYSRET

Clarify why exactly RF cannot be restored properly by SYSRET to avoid
confusion.

No functionality change.

Signed-off-by: Borislav Petkov <bp@suse.de>
Acked-by: Andy Lutomirski <luto@amacapital.net>
---
 arch/x86/entry/entry_64.S | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index 8956eae04c25..b4b546bb23c3 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -288,11 +288,15 @@ return_from_SYSCALL_64:
 	jne	opportunistic_sysret_failed
 
 	/*
-	 * SYSRET can't restore RF.  SYSRET can restore TF, but unlike IRET,
-	 * restoring TF results in a trap from userspace immediately after
-	 * SYSRET.  This would cause an infinite loop whenever #DB happens
-	 * with register state that satisfies the opportunistic SYSRET
-	 * conditions.  For example, single-stepping this user code:
+	 * SYSCALL clears RF when it saves rFLAGS in R11 and SYSRET cannot
+	 * restore RF properly. If the slowpath sets it for whatever reason, we
+	 * need to restore it correctly.
+	 *
+	 * SYSRET can restore TF, but unlike IRET, restoring TF results in a
+	 * trap from userspace immediately after SYSRET.  This would cause an
+	 * infinite loop whenever #DB happens with register state that satisfies
+	 * the opportunistic SYSRET conditions.  For example, single-stepping
+	 * this user code:
 	 *
 	 *           movq	$stuck_here, %rcx
 	 *           pushfq
-- 
2.8.4


-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

[toc] | [prev] | [next] | [standalone]


#1455913

FromAndy Lutomirski <luto@amacapital.net>
Date2016-08-03 19:30 +0200
Message-ID<s289s-5sX-19@gated-at.bofh.it>
In reply to#1455906
On Wed, Aug 3, 2016 at 10:14 AM, Borislav Petkov <bp@alien8.de> wrote:
> On Wed, Aug 03, 2016 at 09:42:29AM -0700, Andy Lutomirski wrote:
>> I would change "so" and "and" -- the CPU designers could have make
>> SYSRET restore RF, but they chose not to.
>
> I'm assuming the reasoning behind it was that you should be able to
> break after SYSRET but then again, the kernel could've been left in
> control of that bit and set it or clear it however it likes before
> SYSRETing.
>
> Oh well.

AFAICT the AMD people didn't think of any use cases involving doing
anything interesting between SYSCALL and SYSRET.  Witness the
sysret_ss_attrs goof, for example: apparently SYSCALL; context switch;
IRET; interrupt; context switch; SYSRET didn't occur to AMD as a valid
use case.

--Andy

[toc] | [prev] | [next] | [standalone]


#1455936 — Re: [PATCH] x86/entry: Clarify the RF saving/restoring situation with SYSCALL/SYSRET

FromBorislav Petkov <bp@alien8.de>
Date2016-08-03 20:10 +0200
SubjectRe: [PATCH] x86/entry: Clarify the RF saving/restoring situation with SYSCALL/SYSRET
Message-ID<s28Mb-62m-41@gated-at.bofh.it>
In reply to#1455913
On Wed, Aug 03, 2016 at 10:24:18AM -0700, Andy Lutomirski wrote:
> AFAICT the AMD people didn't think of any use cases involving doing
> anything interesting between SYSCALL and SYSRET.  Witness the
> sysret_ss_attrs goof, for example: apparently SYSCALL; context switch;
> IRET; interrupt; context switch; SYSRET didn't occur to AMD as a valid
> use case.

/me hands Andy a time machine to go fix this properly, before so much
silicon ships.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--

[toc] | [prev] | [next] | [standalone]


#1459881 — [tip:x86/urgent] x86/entry: Clarify the RF saving/restoring situation with SYSCALL/SYSRET

Fromtip-bot for Borislav Petkov <tipbot@zytor.com>
Date2016-08-10 22:50 +0200
Subject[tip:x86/urgent] x86/entry: Clarify the RF saving/restoring situation with SYSCALL/SYSRET
Message-ID<s4IBP-1DK-3@gated-at.bofh.it>
In reply to#1455906
Commit-ID:  3e035305875cfa8a58c1ca573d0cfa6a7f201f27
Gitweb:     http://git.kernel.org/tip/3e035305875cfa8a58c1ca573d0cfa6a7f201f27
Author:     Borislav Petkov <bp@alien8.de>
AuthorDate: Wed, 3 Aug 2016 19:14:29 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Wed, 10 Aug 2016 15:53:43 +0200

x86/entry: Clarify the RF saving/restoring situation with SYSCALL/SYSRET

Clarify why exactly RF cannot be restored properly by SYSRET to avoid
confusion.

No functionality change.

Signed-off-by: Borislav Petkov <bp@suse.de>
Acked-by: Andy Lutomirski <luto@amacapital.net>
Cc: Andy Lutomirski <luto@kernel.org>
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: Josh Poimboeuf <jpoimboe@redhat.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/20160803171429.GA2590@nazgul.tnic
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/entry/entry_64.S | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index 9f85827..d172c61 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -288,11 +288,15 @@ return_from_SYSCALL_64:
 	jne	opportunistic_sysret_failed
 
 	/*
-	 * SYSRET can't restore RF.  SYSRET can restore TF, but unlike IRET,
-	 * restoring TF results in a trap from userspace immediately after
-	 * SYSRET.  This would cause an infinite loop whenever #DB happens
-	 * with register state that satisfies the opportunistic SYSRET
-	 * conditions.  For example, single-stepping this user code:
+	 * SYSCALL clears RF when it saves RFLAGS in R11 and SYSRET cannot
+	 * restore RF properly. If the slowpath sets it for whatever reason, we
+	 * need to restore it correctly.
+	 *
+	 * SYSRET can restore TF, but unlike IRET, restoring TF results in a
+	 * trap from userspace immediately after SYSRET.  This would cause an
+	 * infinite loop whenever #DB happens with register state that satisfies
+	 * the opportunistic SYSRET conditions.  For example, single-stepping
+	 * this user code:
 	 *
 	 *           movq	$stuck_here, %rcx
 	 *           pushfq

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web