* [PATCH] x86/entry/64: Add an LFENCE after SAVE_AND_SET_GSBASE
@ 2020-08-20 9:15 Borislav Petkov
2020-08-20 18:04 ` Bae, Chang Seok
0 siblings, 1 reply; 4+ messages in thread
From: Borislav Petkov @ 2020-08-20 9:15 UTC (permalink / raw)
To: X86 ML; +Cc: Chang S . Bae, LKML
From: Borislav Petkov <bp@suse.de>
The FSGSBASE macro to swap current GSBASE with the kernel GSBASE
probably had a speculation-stopping MSR write at some point but not
anymore.
Therefore, a fence is needed to stop speculation. Add it.
Fixes: c82965f9e530 ("x86/entry/64: Handle FSGSBASE enabled paranoid entry/exit")
Signed-off-by: Borislav Petkov <bp@suse.de>
Cc: <stable@vger.kernel.org>
---
arch/x86/entry/entry_64.S | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index 26fc9b42fadc..3931d47cdd83 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -839,11 +839,9 @@ SYM_CODE_START_LOCAL(paranoid_entry)
* Read the current GSBASE and store it in %rbx unconditionally,
* retrieve and set the current CPUs kernel GSBASE. The stored value
* has to be restored in paranoid_exit unconditionally.
- *
- * The MSR write ensures that no subsequent load is based on a
- * mispredicted GSBASE. No extra FENCE required.
*/
SAVE_AND_SET_GSBASE scratch_reg=%rax save_reg=%rbx
+ FENCE_SWAPGS_KERNEL_ENTRY
ret
.Lparanoid_entry_checkgs:
--
2.21.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] x86/entry/64: Add an LFENCE after SAVE_AND_SET_GSBASE
2020-08-20 9:15 [PATCH] x86/entry/64: Add an LFENCE after SAVE_AND_SET_GSBASE Borislav Petkov
@ 2020-08-20 18:04 ` Bae, Chang Seok
2020-08-21 9:07 ` [PATCH -v2] x86/entry/64: Correct the comment over SAVE_AND_SET_GSBASE Borislav Petkov
0 siblings, 1 reply; 4+ messages in thread
From: Bae, Chang Seok @ 2020-08-20 18:04 UTC (permalink / raw)
To: Borislav Petkov; +Cc: X86 ML, LKML
> On Aug 20, 2020, at 02:15, Borislav Petkov <bp@alien8.de> wrote:
>
> From: Borislav Petkov <bp@suse.de>
>
> The FSGSBASE macro to swap current GSBASE with the kernel GSBASE
> probably had a speculation-stopping MSR write at some point but not
> anymore.
No, the macro has not any MSR write ever before.
> diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
> index 26fc9b42fadc..3931d47cdd83 100644
> --- a/arch/x86/entry/entry_64.S
> +++ b/arch/x86/entry/entry_64.S
> @@ -839,11 +839,9 @@ SYM_CODE_START_LOCAL(paranoid_entry)
> * Read the current GSBASE and store it in %rbx unconditionally,
> * retrieve and set the current CPUs kernel GSBASE. The stored value
> * has to be restored in paranoid_exit unconditionally.
> - *
> - * The MSR write ensures that no subsequent load is based on a
> - * mispredicted GSBASE. No extra FENCE required.
Thomas looks to add this text when he picked v7 [1].
FWIW, we added our text from v8, after reviewed the speculation impact
on this series:
This unconditional write of GS base ensures no subsequent load
based on a mispredicted GS base.
I think somehow the “MSR write” made confusion. Our conclusion was the
same as Thomas' that no FENCE is needed here.
Thanks,
Chang
[1] https://lore.kernel.org/lkml/1557309753-24073-5-git-send-email-chang.seok.bae@intel.com/t/#m45f380a23231b87498b40b708f0401147f3b0278
> */
> SAVE_AND_SET_GSBASE scratch_reg=%rax save_reg=%rbx
> + FENCE_SWAPGS_KERNEL_ENTRY
> ret
>
> .Lparanoid_entry_checkgs:
> --
> 2.21.0
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH -v2] x86/entry/64: Correct the comment over SAVE_AND_SET_GSBASE
2020-08-20 18:04 ` Bae, Chang Seok
@ 2020-08-21 9:07 ` Borislav Petkov
2020-08-24 8:40 ` [tip: x86/fsgsbase] " tip-bot2 for Borislav Petkov
0 siblings, 1 reply; 4+ messages in thread
From: Borislav Petkov @ 2020-08-21 9:07 UTC (permalink / raw)
To: Bae, Chang Seok; +Cc: X86 ML, LKML
On Thu, Aug 20, 2020 at 06:04:50PM +0000, Bae, Chang Seok wrote:
> I think somehow the “MSR write” made confusion. Our conclusion was the
> same as Thomas' that no FENCE is needed here.
Ok, here's v2 with a corrected comment:
---
From: Borislav Petkov <bp@suse.de>
Add the proper explanation why an LFENCE is not needed in the FSGSBASE
case.
Fixes: c82965f9e530 ("x86/entry/64: Handle FSGSBASE enabled paranoid entry/exit")
Signed-off-by: Borislav Petkov <bp@suse.de>
---
arch/x86/entry/entry_64.S | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index 26fc9b42fadc..5c5d234d968d 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -840,8 +840,9 @@ SYM_CODE_START_LOCAL(paranoid_entry)
* retrieve and set the current CPUs kernel GSBASE. The stored value
* has to be restored in paranoid_exit unconditionally.
*
- * The MSR write ensures that no subsequent load is based on a
- * mispredicted GSBASE. No extra FENCE required.
+ * The unconditional write to GS base below ensures that no subsequent
+ * loads based on a mispredicted GS base can happen, therefore no LFENCE
+ * is needed here.
*/
SAVE_AND_SET_GSBASE scratch_reg=%rax save_reg=%rbx
ret
--
2.21.0
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [tip: x86/fsgsbase] x86/entry/64: Correct the comment over SAVE_AND_SET_GSBASE
2020-08-21 9:07 ` [PATCH -v2] x86/entry/64: Correct the comment over SAVE_AND_SET_GSBASE Borislav Petkov
@ 2020-08-24 8:40 ` tip-bot2 for Borislav Petkov
0 siblings, 0 replies; 4+ messages in thread
From: tip-bot2 for Borislav Petkov @ 2020-08-24 8:40 UTC (permalink / raw)
To: linux-tip-commits; +Cc: Borislav Petkov, x86, LKML
The following commit has been merged into the x86/fsgsbase branch of tip:
Commit-ID: 0b2c605fa4ee3117c00b97b7af67791576b28f88
Gitweb: https://git.kernel.org/tip/0b2c605fa4ee3117c00b97b7af67791576b28f88
Author: Borislav Petkov <bp@suse.de>
AuthorDate: Thu, 20 Aug 2020 11:10:15 +02:00
Committer: Borislav Petkov <bp@suse.de>
CommitterDate: Mon, 24 Aug 2020 10:23:40 +02:00
x86/entry/64: Correct the comment over SAVE_AND_SET_GSBASE
Add the proper explanation why an LFENCE is not needed in the FSGSBASE
case.
Fixes: c82965f9e530 ("x86/entry/64: Handle FSGSBASE enabled paranoid entry/exit")
Signed-off-by: Borislav Petkov <bp@suse.de>
Link: https://lkml.kernel.org/r/20200821090710.GE12181@zn.tnic
---
arch/x86/entry/entry_64.S | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index 70dea93..bf78de4 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -840,8 +840,9 @@ SYM_CODE_START_LOCAL(paranoid_entry)
* retrieve and set the current CPUs kernel GSBASE. The stored value
* has to be restored in paranoid_exit unconditionally.
*
- * The MSR write ensures that no subsequent load is based on a
- * mispredicted GSBASE. No extra FENCE required.
+ * The unconditional write to GS base below ensures that no subsequent
+ * loads based on a mispredicted GS base can happen, therefore no LFENCE
+ * is needed here.
*/
SAVE_AND_SET_GSBASE scratch_reg=%rax save_reg=%rbx
ret
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-08-24 9:48 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-20 9:15 [PATCH] x86/entry/64: Add an LFENCE after SAVE_AND_SET_GSBASE Borislav Petkov
2020-08-20 18:04 ` Bae, Chang Seok
2020-08-21 9:07 ` [PATCH -v2] x86/entry/64: Correct the comment over SAVE_AND_SET_GSBASE Borislav Petkov
2020-08-24 8:40 ` [tip: x86/fsgsbase] " tip-bot2 for Borislav Petkov
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).