2020-02-19 09:58:53

by Thomas Gleixner

Subject: [PATCH] x86/entry/32: Add missing ASM_CLAC in general_protection entry

All exception entry points must have ASM_CLAC right at the
beginning. The general_protection entry is missing one.

Fixes: e59d1b0a2419 ("x86-32, smap: Add STAC/CLAC instructions to 32-bit kernel entry")
Signed-off-by: Thomas Gleixner <[email protected]>
Cc: [email protected]
---
arch/x86/entry/entry_32.S | 1 +
1 file changed, 1 insertion(+)

--- a/arch/x86/entry/entry_32.S
+++ b/arch/x86/entry/entry_32.S
@@ -1681,6 +1681,7 @@ SYM_CODE_START(int3)
SYM_CODE_END(int3)

SYM_CODE_START(general_protection)
+ ASM_CLAC
pushl $do_general_protection
jmp common_exception
SYM_CODE_END(general_protection)

2020-02-19 20:11:54

by Brian Gerst

Subject: Re: [PATCH] x86/entry/32: Add missing ASM_CLAC in general_protection entry

On Wed, Feb 19, 2020 at 4:58 AM Thomas Gleixner <[email protected]> wrote:
>
> All exception entry points must have ASM_CLAC right at the
> beginning. The general_protection entry is missing one.
>
> Fixes: e59d1b0a2419 ("x86-32, smap: Add STAC/CLAC instructions to 32-bit kernel entry")
> Signed-off-by: Thomas Gleixner <[email protected]>
> Cc: [email protected]
> ---
> arch/x86/entry/entry_32.S | 1 +
> 1 file changed, 1 insertion(+)
>
> --- a/arch/x86/entry/entry_32.S
> +++ b/arch/x86/entry/entry_32.S
> @@ -1681,6 +1681,7 @@ SYM_CODE_START(int3)
> SYM_CODE_END(int3)
>
> SYM_CODE_START(general_protection)
> + ASM_CLAC
> pushl $do_general_protection
> jmp common_exception
> SYM_CODE_END(general_protection)

How about moving ASM_CLAC to common_exception instead? That would
save a few bytes (kernel text + alternatives), and the AC bit has no
effect on kernel stack pushes.

--
Brian Gerst

2020-02-20 23:05:46

by Thomas Gleixner

Subject: Re: [PATCH] x86/entry/32: Add missing ASM_CLAC in general_protection entry

Brian Gerst <[email protected]> writes:

> On Wed, Feb 19, 2020 at 4:58 AM Thomas Gleixner <[email protected]> wrote:
>>
>> All exception entry points must have ASM_CLAC right at the
>> beginning. The general_protection entry is missing one.
>>
>> Fixes: e59d1b0a2419 ("x86-32, smap: Add STAC/CLAC instructions to 32-bit kernel entry")
>> Signed-off-by: Thomas Gleixner <[email protected]>
>> Cc: [email protected]
>> ---
>> arch/x86/entry/entry_32.S | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> --- a/arch/x86/entry/entry_32.S
>> +++ b/arch/x86/entry/entry_32.S
>> @@ -1681,6 +1681,7 @@ SYM_CODE_START(int3)
>> SYM_CODE_END(int3)
>>
>> SYM_CODE_START(general_protection)
>> + ASM_CLAC
>> pushl $do_general_protection
>> jmp common_exception
>> SYM_CODE_END(general_protection)
>
> How about moving ASM_CLAC to common_exception instead? That would
> save a few bytes (kernel text + alternatives), and the AC bit has no
> effect on kernel stack pushes.

Agreed, but that's a seperate cleanup. The fix is the right thing also
for backports.

Aisde of that this mindlessly copied code will be gone in the
foreseeable future. Just lacks some testing and changelog writing :)

Thanks,

tglx