* [PATCH] x86/entry: Fix AC assertion
@ 2020-08-24 10:14 peterz
2020-08-24 14:22 ` Andrew Cooper
0 siblings, 1 reply; 5+ messages in thread
From: peterz @ 2020-08-24 10:14 UTC (permalink / raw)
To: x86; +Cc: Andrew Cooper, Juergen Gross, LKML, Andy Lutomirski
The WARN added in commit 3c73b81a9164 ("x86/entry, selftests: Further
improve user entry sanity checks") unconditionally triggers on my IVB
machine because it does not support SMAP.
For !SMAP hardware we patch out CLAC/STAC instructions and thus if
userspace sets AC, we'll still have it set after entry.
Fixes: 3c73b81a9164 ("x86/entry, selftests: Further improve user entry sanity checks")
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Acked-by: Andy Lutomirski <luto@kernel.org>
---
arch/x86/include/asm/entry-common.h | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
--- a/arch/x86/include/asm/entry-common.h
+++ b/arch/x86/include/asm/entry-common.h
@@ -18,8 +18,15 @@ static __always_inline void arch_check_u
* state, not the interrupt state as imagined by Xen.
*/
unsigned long flags = native_save_fl();
- WARN_ON_ONCE(flags & (X86_EFLAGS_AC | X86_EFLAGS_DF |
- X86_EFLAGS_NT));
+ unsigned long mask = X86_EFLAGS_DF | X86_EFLAGS_NT;
+
+ /*
+ * For !SMAP hardware we patch out CLAC on entry.
+ */
+ if (boot_cpu_has(X86_FEATURE_SMAP))
+ mask |= X86_EFLAGS_AC;
+
+ WARN_ON_ONCE(flags & mask);
/* We think we came from user mode. Make sure pt_regs agrees. */
WARN_ON_ONCE(!user_mode(regs));
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] x86/entry: Fix AC assertion
2020-08-24 10:14 [PATCH] x86/entry: Fix AC assertion peterz
@ 2020-08-24 14:22 ` Andrew Cooper
2020-08-24 15:21 ` peterz
0 siblings, 1 reply; 5+ messages in thread
From: Andrew Cooper @ 2020-08-24 14:22 UTC (permalink / raw)
To: peterz, x86; +Cc: Juergen Gross, LKML, Andy Lutomirski
On 24/08/2020 11:14, peterz@infradead.org wrote:
> The WARN added in commit 3c73b81a9164 ("x86/entry, selftests: Further
> improve user entry sanity checks") unconditionally triggers on my IVB
> machine because it does not support SMAP.
>
> For !SMAP hardware we patch out CLAC/STAC instructions and thus if
> userspace sets AC, we'll still have it set after entry.
Technically, you don't patch in, rather than patch out.
>
> Fixes: 3c73b81a9164 ("x86/entry, selftests: Further improve user entry sanity checks")
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> Acked-by: Andy Lutomirski <luto@kernel.org>
> ---
> arch/x86/include/asm/entry-common.h | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> --- a/arch/x86/include/asm/entry-common.h
> +++ b/arch/x86/include/asm/entry-common.h
> @@ -18,8 +18,15 @@ static __always_inline void arch_check_u
> * state, not the interrupt state as imagined by Xen.
> */
> unsigned long flags = native_save_fl();
> - WARN_ON_ONCE(flags & (X86_EFLAGS_AC | X86_EFLAGS_DF |
> - X86_EFLAGS_NT));
> + unsigned long mask = X86_EFLAGS_DF | X86_EFLAGS_NT;
> +
> + /*
> + * For !SMAP hardware we patch out CLAC on entry.
> + */
> + if (boot_cpu_has(X86_FEATURE_SMAP))
> + mask |= X86_EFLAGS_AC;
The Xen PV ABI clears AC on entry for 64bit guests, because Linux is
actually running in Ring 3, and therefore susceptible to #AC's which
wouldn't occur natively.
~Andrew
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] x86/entry: Fix AC assertion
2020-08-24 14:22 ` Andrew Cooper
@ 2020-08-24 15:21 ` peterz
2020-08-24 15:58 ` Andrew Cooper
0 siblings, 1 reply; 5+ messages in thread
From: peterz @ 2020-08-24 15:21 UTC (permalink / raw)
To: Andrew Cooper; +Cc: x86, Juergen Gross, LKML, Andy Lutomirski
On Mon, Aug 24, 2020 at 03:22:06PM +0100, Andrew Cooper wrote:
> On 24/08/2020 11:14, peterz@infradead.org wrote:
> > The WARN added in commit 3c73b81a9164 ("x86/entry, selftests: Further
> > improve user entry sanity checks") unconditionally triggers on my IVB
> > machine because it does not support SMAP.
> >
> > For !SMAP hardware we patch out CLAC/STAC instructions and thus if
> > userspace sets AC, we'll still have it set after entry.
>
> Technically, you don't patch in, rather than patch out.
True.
> > Fixes: 3c73b81a9164 ("x86/entry, selftests: Further improve user entry sanity checks")
> > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> > Acked-by: Andy Lutomirski <luto@kernel.org>
> > ---
> > arch/x86/include/asm/entry-common.h | 11 +++++++++--
> > 1 file changed, 9 insertions(+), 2 deletions(-)
> >
> > --- a/arch/x86/include/asm/entry-common.h
> > +++ b/arch/x86/include/asm/entry-common.h
> > @@ -18,8 +18,15 @@ static __always_inline void arch_check_u
> > * state, not the interrupt state as imagined by Xen.
> > */
> > unsigned long flags = native_save_fl();
> > - WARN_ON_ONCE(flags & (X86_EFLAGS_AC | X86_EFLAGS_DF |
> > - X86_EFLAGS_NT));
> > + unsigned long mask = X86_EFLAGS_DF | X86_EFLAGS_NT;
> > +
> > + /*
> > + * For !SMAP hardware we patch out CLAC on entry.
> > + */
> > + if (boot_cpu_has(X86_FEATURE_SMAP))
> > + mask |= X86_EFLAGS_AC;
>
> The Xen PV ABI clears AC on entry for 64bit guests, because Linux is
> actually running in Ring 3, and therefore susceptible to #AC's which
> wouldn't occur natively.
So do you then want it to be something like:
if (boot_cpu_has(X86_FEATURE_SMAP) ||
(IS_ENABLED(CONFIG_64_BIT) && boot_cpu_has(X86_FEATURE_XENPV)))
? Or are you fine with the proposed?
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] x86/entry: Fix AC assertion
2020-08-24 15:21 ` peterz
@ 2020-08-24 15:58 ` Andrew Cooper
2020-08-24 16:27 ` Jürgen Groß
0 siblings, 1 reply; 5+ messages in thread
From: Andrew Cooper @ 2020-08-24 15:58 UTC (permalink / raw)
To: peterz; +Cc: x86, Juergen Gross, LKML, Andy Lutomirski
On 24/08/2020 16:21, peterz@infradead.org wrote:
> On Mon, Aug 24, 2020 at 03:22:06PM +0100, Andrew Cooper wrote:
>> On 24/08/2020 11:14, peterz@infradead.org wrote:
>>> The WARN added in commit 3c73b81a9164 ("x86/entry, selftests: Further
>>> improve user entry sanity checks") unconditionally triggers on my IVB
>>> machine because it does not support SMAP.
>>>
>>> For !SMAP hardware we patch out CLAC/STAC instructions and thus if
>>> userspace sets AC, we'll still have it set after entry.
>> Technically, you don't patch in, rather than patch out.
> True.
>
>>> Fixes: 3c73b81a9164 ("x86/entry, selftests: Further improve user entry sanity checks")
>>> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
>>> Acked-by: Andy Lutomirski <luto@kernel.org>
>>> ---
>>> arch/x86/include/asm/entry-common.h | 11 +++++++++--
>>> 1 file changed, 9 insertions(+), 2 deletions(-)
>>>
>>> --- a/arch/x86/include/asm/entry-common.h
>>> +++ b/arch/x86/include/asm/entry-common.h
>>> @@ -18,8 +18,15 @@ static __always_inline void arch_check_u
>>> * state, not the interrupt state as imagined by Xen.
>>> */
>>> unsigned long flags = native_save_fl();
>>> - WARN_ON_ONCE(flags & (X86_EFLAGS_AC | X86_EFLAGS_DF |
>>> - X86_EFLAGS_NT));
>>> + unsigned long mask = X86_EFLAGS_DF | X86_EFLAGS_NT;
>>> +
>>> + /*
>>> + * For !SMAP hardware we patch out CLAC on entry.
>>> + */
>>> + if (boot_cpu_has(X86_FEATURE_SMAP))
>>> + mask |= X86_EFLAGS_AC;
>> The Xen PV ABI clears AC on entry for 64bit guests, because Linux is
>> actually running in Ring 3, and therefore susceptible to #AC's which
>> wouldn't occur natively.
> So do you then want it to be something like:
>
> if (boot_cpu_has(X86_FEATURE_SMAP) ||
> (IS_ENABLED(CONFIG_64_BIT) && boot_cpu_has(X86_FEATURE_XENPV)))
>
> ? Or are you fine with the proposed?
Dealers choice, but this option would be slightly better overall.
(Are there any other cases where Linux will be running in Ring 3? I
haven't been paying attention to recent changes in PVOps.)
~Andrew
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] x86/entry: Fix AC assertion
2020-08-24 15:58 ` Andrew Cooper
@ 2020-08-24 16:27 ` Jürgen Groß
0 siblings, 0 replies; 5+ messages in thread
From: Jürgen Groß @ 2020-08-24 16:27 UTC (permalink / raw)
To: Andrew Cooper, peterz; +Cc: x86, LKML, Andy Lutomirski
On 24.08.20 17:58, Andrew Cooper wrote:
> On 24/08/2020 16:21, peterz@infradead.org wrote:
>> On Mon, Aug 24, 2020 at 03:22:06PM +0100, Andrew Cooper wrote:
>>> On 24/08/2020 11:14, peterz@infradead.org wrote:
>>>> The WARN added in commit 3c73b81a9164 ("x86/entry, selftests: Further
>>>> improve user entry sanity checks") unconditionally triggers on my IVB
>>>> machine because it does not support SMAP.
>>>>
>>>> For !SMAP hardware we patch out CLAC/STAC instructions and thus if
>>>> userspace sets AC, we'll still have it set after entry.
>>> Technically, you don't patch in, rather than patch out.
>> True.
>>
>>>> Fixes: 3c73b81a9164 ("x86/entry, selftests: Further improve user entry sanity checks")
>>>> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
>>>> Acked-by: Andy Lutomirski <luto@kernel.org>
>>>> ---
>>>> arch/x86/include/asm/entry-common.h | 11 +++++++++--
>>>> 1 file changed, 9 insertions(+), 2 deletions(-)
>>>>
>>>> --- a/arch/x86/include/asm/entry-common.h
>>>> +++ b/arch/x86/include/asm/entry-common.h
>>>> @@ -18,8 +18,15 @@ static __always_inline void arch_check_u
>>>> * state, not the interrupt state as imagined by Xen.
>>>> */
>>>> unsigned long flags = native_save_fl();
>>>> - WARN_ON_ONCE(flags & (X86_EFLAGS_AC | X86_EFLAGS_DF |
>>>> - X86_EFLAGS_NT));
>>>> + unsigned long mask = X86_EFLAGS_DF | X86_EFLAGS_NT;
>>>> +
>>>> + /*
>>>> + * For !SMAP hardware we patch out CLAC on entry.
>>>> + */
>>>> + if (boot_cpu_has(X86_FEATURE_SMAP))
>>>> + mask |= X86_EFLAGS_AC;
>>> The Xen PV ABI clears AC on entry for 64bit guests, because Linux is
>>> actually running in Ring 3, and therefore susceptible to #AC's which
>>> wouldn't occur natively.
>> So do you then want it to be something like:
>>
>> if (boot_cpu_has(X86_FEATURE_SMAP) ||
>> (IS_ENABLED(CONFIG_64_BIT) && boot_cpu_has(X86_FEATURE_XENPV)))
>>
>> ? Or are you fine with the proposed?
>
> Dealers choice, but this option would be slightly better overall.
>
> (Are there any other cases where Linux will be running in Ring 3? I
> haven't been paying attention to recent changes in PVOps.)
I'm not aware of any other case running kernel code in ring 3.
Juergen
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-08-24 16:28 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-24 10:14 [PATCH] x86/entry: Fix AC assertion peterz
2020-08-24 14:22 ` Andrew Cooper
2020-08-24 15:21 ` peterz
2020-08-24 15:58 ` Andrew Cooper
2020-08-24 16:27 ` Jürgen Groß
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®