[v3,5/5] x86/entry: Make IA32 syscalls' availability depend on ia32_enabled()

Message ID 20230616125730.1164989-6-nik.borisov@suse.com
State New
Headers
Series Make IA32_EMULATION boot time overridable |

Commit Message

Nikolay Borisov June 16, 2023, 12:57 p.m. UTC
  Another major aspect of supporting running of 32bit processes is the
ability to access 32bit syscalls. Such syscalls are invoked either by
using the legacy int 0x80 call gate interface or via the newer sysenter
instruction.

Ensure that if ia32 emulation is disabled (either at compile time or
runtime) then those 2 syscall mechanisms are also disabled.

Signed-off-by: Nikolay Borisov <nik.borisov@suse.com>
---
 arch/x86/include/asm/proto.h |  3 +++
 arch/x86/kernel/cpu/common.c | 37 ++++++++++++++++++------------------
 arch/x86/kernel/idt.c        | 10 ++++++++++
 3 files changed, 32 insertions(+), 18 deletions(-)
  

Comments

Thomas Gleixner June 18, 2023, 9:17 p.m. UTC | #1
On Fri, Jun 16 2023 at 15:57, Nikolay Borisov wrote:
> Another major aspect of supporting running of 32bit processes is the
> ability to access 32bit syscalls. Such syscalls are invoked either by
> using the legacy int 0x80 call gate interface or via the newer sysenter
> instruction.
>
> Ensure that if ia32 emulation is disabled (either at compile time or
> runtime) then those 2 syscall mechanisms are also disabled.

AFAICT there are _three_ mechanisms for 32bit syscalls, no?

>  void __init idt_setup_traps(void)
>  {
>  	idt_setup_from_table(idt_table, def_idts, ARRAY_SIZE(def_idts), true);
> +
> +	if (ia32_enabled()) {
> +		idt_setup_from_table(idt_table, ia32_idt, ARRAY_SIZE(ia32_idt),
> +				     true);

Just let it stick out. The 80 character limit is history.

Thanks,

        tglx
  
Nikolay Borisov June 19, 2023, 6:28 a.m. UTC | #2
On 19.06.23 г. 0:17 ч., Thomas Gleixner wrote:
> On Fri, Jun 16 2023 at 15:57, Nikolay Borisov wrote:
>> Another major aspect of supporting running of 32bit processes is the
>> ability to access 32bit syscalls. Such syscalls are invoked either by
>> using the legacy int 0x80 call gate interface or via the newer sysenter
>> instruction.
>>
>> Ensure that if ia32 emulation is disabled (either at compile time or
>> runtime) then those 2 syscall mechanisms are also disabled.
> 
> AFAICT there are _three_ mechanisms for 32bit syscalls, no?

int 0x80 and sysenter make it 2? Which one is the 3rd one - the "native 
64bit syscall" used in for X32 ABI ? This patch specifically deals with 
the first 2?
> 
>>   void __init idt_setup_traps(void)
>>   {
>>   	idt_setup_from_table(idt_table, def_idts, ARRAY_SIZE(def_idts), true);
>> +
>> +	if (ia32_enabled()) {
>> +		idt_setup_from_table(idt_table, ia32_idt, ARRAY_SIZE(ia32_idt),
>> +				     true);
> 
> Just let it stick out. The 80 character limit is history.
> 
> Thanks,
> 
>          tglx
  
Thomas Gleixner June 19, 2023, 8:40 a.m. UTC | #3
On Mon, Jun 19 2023 at 09:28, Nikolay Borisov wrote:
> On 19.06.23 г. 0:17 ч., Thomas Gleixner wrote:
>> On Fri, Jun 16 2023 at 15:57, Nikolay Borisov wrote:
>>> Another major aspect of supporting running of 32bit processes is the
>>> ability to access 32bit syscalls. Such syscalls are invoked either by
>>> using the legacy int 0x80 call gate interface or via the newer sysenter
>>> instruction.
>>>
>>> Ensure that if ia32 emulation is disabled (either at compile time or
>>> runtime) then those 2 syscall mechanisms are also disabled.
>> 
>> AFAICT there are _three_ mechanisms for 32bit syscalls, no?
>
> int 0x80 and sysenter make it 2? Which one is the 3rd one - the "native 
> 64bit syscall" used in for X32 ABI ? This patch specifically deals with 
> the first 2?

int 80, sysenter, syscall = 3

They obviously depend on the vendor preference when the CPU has enabled
long mode:

                      AMD    Intel
compat_int 80         y      y
compat_sysenter       #UD    y
compat_syscall        y      #UD

On Intel SYSENTER is trivial to disable by setting MSR_IA32_SYSENTER_CS
to 0 which makes sysenter raise #GP.

The nasty one is SYSCALL on AMD. If MSR_EFER.SCE=1 then MSR_CSTAR must
contain a valid kernel text address because otherwise compat SYSCALL
faults with CPL0 and user GSBASE. That's the whole reason for the stub
function which just sets EAX to -ENOSYS and returns via SYSRET.

And your patch deals with all _three_:

                    compat=ON                    compat=OFF
compat_int 80:      Set system interrupt gate    ---

compat_sysenter:    Set up SYSENTER MSRs for     Invalidate SYSENTER
                    entry_SYSENTER_compat()      MSRs

compat_syscall:     Set MSR_CSTAR to             Set MSR_CSTAR to
                    entry_SYSCALL_compat()       stub function
                    (AMD only)                   (AMD only)

No?

Changelogs have to be precise. Otherwise they are useless and in the
worst case actively misleading.

Thanks,

        tglx
  

Patch

diff --git a/arch/x86/include/asm/proto.h b/arch/x86/include/asm/proto.h
index 12ef86b19910..1241d27fc4a6 100644
--- a/arch/x86/include/asm/proto.h
+++ b/arch/x86/include/asm/proto.h
@@ -36,6 +36,9 @@  void entry_INT80_compat(void);
 #ifdef CONFIG_XEN_PV
 void xen_entry_INT80_compat(void);
 #endif
+#else /* #CONFIG_IA32_EMULATION */
+#define entry_SYSCALL_compat NULL
+#define entry_SYSENTER_compat NULL
 #endif
 
 void x86_configure_nx(void);
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index b20774181e1a..aafb83d1b3a7 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -59,6 +59,7 @@ 
 #include <asm/intel-family.h>
 #include <asm/cpu_device_id.h>
 #include <asm/uv/uv.h>
+#include <asm/ia32.h>
 #include <asm/sigframe.h>
 #include <asm/traps.h>
 #include <asm/sev.h>
@@ -2053,24 +2054,24 @@  void syscall_init(void)
 	wrmsr(MSR_STAR, 0, (__USER32_CS << 16) | __KERNEL_CS);
 	wrmsrl(MSR_LSTAR, (unsigned long)entry_SYSCALL_64);
 
-#ifdef CONFIG_IA32_EMULATION
-	wrmsrl_cstar((unsigned long)entry_SYSCALL_compat);
-	/*
-	 * This only works on Intel CPUs.
-	 * On AMD CPUs these MSRs are 32-bit, CPU truncates MSR_IA32_SYSENTER_EIP.
-	 * This does not cause SYSENTER to jump to the wrong location, because
-	 * AMD doesn't allow SYSENTER in long mode (either 32- or 64-bit).
-	 */
-	wrmsrl_safe(MSR_IA32_SYSENTER_CS, (u64)__KERNEL_CS);
-	wrmsrl_safe(MSR_IA32_SYSENTER_ESP,
-		    (unsigned long)(cpu_entry_stack(smp_processor_id()) + 1));
-	wrmsrl_safe(MSR_IA32_SYSENTER_EIP, (u64)entry_SYSENTER_compat);
-#else
-	wrmsrl_cstar((unsigned long)entry_SYSCALL32_ignore);
-	wrmsrl_safe(MSR_IA32_SYSENTER_CS, (u64)GDT_ENTRY_INVALID_SEG);
-	wrmsrl_safe(MSR_IA32_SYSENTER_ESP, 0ULL);
-	wrmsrl_safe(MSR_IA32_SYSENTER_EIP, 0ULL);
-#endif
+	if (ia32_enabled()) {
+		wrmsrl_cstar((unsigned long)entry_SYSCALL_compat);
+		/*
+		 * This only works on Intel CPUs.
+		 * On AMD CPUs these MSRs are 32-bit, CPU truncates MSR_IA32_SYSENTER_EIP.
+		 * This does not cause SYSENTER to jump to the wrong location, because
+		 * AMD doesn't allow SYSENTER in long mode (either 32- or 64-bit).
+		 */
+		wrmsrl_safe(MSR_IA32_SYSENTER_CS, (u64)__KERNEL_CS);
+		wrmsrl_safe(MSR_IA32_SYSENTER_ESP,
+			    (unsigned long)(cpu_entry_stack(smp_processor_id()) + 1));
+		wrmsrl_safe(MSR_IA32_SYSENTER_EIP, (u64)entry_SYSENTER_compat);
+	} else {
+		wrmsrl_cstar((unsigned long)entry_SYSCALL32_ignore);
+		wrmsrl_safe(MSR_IA32_SYSENTER_CS, (u64)GDT_ENTRY_INVALID_SEG);
+		wrmsrl_safe(MSR_IA32_SYSENTER_ESP, 0ULL);
+		wrmsrl_safe(MSR_IA32_SYSENTER_EIP, 0ULL);
+	}
 
 	/*
 	 * Flags to clear on syscall; clear as much as possible
diff --git a/arch/x86/kernel/idt.c b/arch/x86/kernel/idt.c
index a58c6bc1cd68..c76953656212 100644
--- a/arch/x86/kernel/idt.c
+++ b/arch/x86/kernel/idt.c
@@ -10,6 +10,7 @@ 
 #include <asm/proto.h>
 #include <asm/desc.h>
 #include <asm/hw_irq.h>
+#include <asm/ia32.h>
 #include <asm/idtentry.h>
 
 #define DPL0		0x0
@@ -116,6 +117,9 @@  static const __initconst struct idt_data def_idts[] = {
 #endif
 
 	SYSG(X86_TRAP_OF,		asm_exc_overflow),
+};
+
+static const struct idt_data ia32_idt[] __initconst = {
 #if defined(CONFIG_IA32_EMULATION)
 	SYSG(IA32_SYSCALL_VECTOR,	entry_INT80_compat),
 #elif defined(CONFIG_X86_32)
@@ -226,6 +230,12 @@  void __init idt_setup_early_traps(void)
 void __init idt_setup_traps(void)
 {
 	idt_setup_from_table(idt_table, def_idts, ARRAY_SIZE(def_idts), true);
+
+	if (ia32_enabled()) {
+		idt_setup_from_table(idt_table, ia32_idt, ARRAY_SIZE(ia32_idt),
+				     true);
+	}
+
 }
 
 #ifdef CONFIG_X86_64