[PATCH v3 0/5] Make IA32_EMULATION boot time overridable

[PATCH v3 0/5] Make IA32_EMULATION boot time overridable

[Nikolay Borisov]

[Sending you to to gather a round of internals reviews before sending upstream]

Here's v3 of the patchset making IA32_EMULATION a boot time overridable switch.

Changes since v2:

* Re-worded the commit message of the first patch (tglx)
* Added help description for the newly introduces IA32_EMULATION_DEFAULT_DISABLED (rdunlap)
* Change the order of the last 2 patches (brgerst)
* Reworked the way ia32_enabled state is being checked - introduced a function
and eliminated code duplication (tglx)
* Reworked the way the idt table is being initialized (tglx)
* Split the rename and unconditional compile of of ignore_sysret (tglx)

Nikolay Borisov (5):
  x86: Make IA32_EMULATION boot time configurable
  x86/entry: Rename ignore_sysret
  x86/entry: Compile entry_SYSCALL32_ignore unconditionally
  x86/elf: Predicate loading of 32bit processes on ia32_enabled()
  x86/entry: Make IA32 syscalls availability depend on ia32_enabled()

 .../admin-guide/kernel-parameters.txt         |  5 +++
 arch/x86/Kconfig                              |  9 +++++
 arch/x86/entry/common.c                       | 16 ++++++++
 arch/x86/entry/entry_64.S                     |  6 +--
 arch/x86/include/asm/elf.h                    |  3 +-
 arch/x86/include/asm/ia32.h                   | 17 ++++++++-
 arch/x86/include/asm/processor.h              |  2 +-
 arch/x86/include/asm/proto.h                  |  3 ++
 arch/x86/kernel/cpu/common.c                  | 37 ++++++++++---------
 arch/x86/kernel/idt.c                         | 10 +++++
 10 files changed, 82 insertions(+), 26 deletions(-)

--
2.34.1

[PATCH v3 1/5] x86: Make IA32_EMULATION boot time configurable

[Nikolay Borisov]

Distributions would like to reduce their attack surface as much as
possible but at the same time they'd want to retain flexibility to cater
to a variety of legacy software. One such avenue where a balance has to
be struck is in supporting 32bit syscalls/processes on 64bit kernels. Ideally
it should be possible for the distribution to set their own policy and
give users the ability to override those policies as appropriate.

In order to support this usecase, introduce
CONFIG_IA32_EMULATION_DEFAULT_DISABLED compile time option, which
controls whether 32bit processes/syscalls should be allowed or not. This
allows distributions to set their preferred default behavior in their
kernel configs.

On the other hand, in order to allow users to override the distro's
policy, introduce the 'ia32_mode' parameter which allows overriding
CONFIG_IA32_EMULATION_DEFAULT_DISABLED state at boot time.

Signed-off-by: Nikolay Borisov <nik.borisov@suse.com>
---
 Documentation/admin-guide/kernel-parameters.txt |  5 +++++
 arch/x86/Kconfig                                |  9 +++++++++
 arch/x86/entry/common.c                         | 16 ++++++++++++++++
 arch/x86/include/asm/ia32.h                     | 16 +++++++++++++++-
 4 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 9e5bab29685f..59b1e86ecd9d 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -1865,6 +1865,11 @@
 			 0 -- machine default
 			 1 -- force brightness inversion
 
+	ia32_mode=		[X86-64]
+			Format: ia32_mode=disabled, ia32_mode=enabled
+			Allows overriding the compile-time state of
+			IA32_EMULATION_DEFAULT_DISABLED at boot time
+
 	icn=		[HW,ISDN]
 			Format: <io>[,<membase>[,<icn_id>[,<icn_id2>]]]
 
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 53bab123a8ee..8bec431c97ce 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -3038,6 +3038,15 @@ config IA32_EMULATION
 	  64-bit kernel. You should likely turn this on, unless you're
 	  100% sure that you don't have any 32-bit programs left.
 
+config IA32_EMULATION_DEFAULT_DISABLED
+	bool "IA32 emulation disabled by default"
+	default n
+	depends on IA32_EMULATION
+	help
+	  Make IA32 emulation disabled by default. This prevents loading 32bit
+	  processes and access to 32bit syscalls. If unsure, leave it to its
+	  default value.
+
 config X86_X32_ABI
 	bool "x32 ABI for 64-bit mode"
 	depends on X86_64
diff --git a/arch/x86/entry/common.c b/arch/x86/entry/common.c
index 6c2826417b33..35bacf2f8be5 100644
--- a/arch/x86/entry/common.c
+++ b/arch/x86/entry/common.c
@@ -19,6 +19,7 @@
 #include <linux/nospec.h>
 #include <linux/syscalls.h>
 #include <linux/uaccess.h>
+#include <linux/init.h>
 
 #ifdef CONFIG_XEN_PV
 #include <xen/xen-ops.h>
@@ -96,6 +97,21 @@ static __always_inline int syscall_32_enter(struct pt_regs *regs)
 	return (int)regs->orig_ax;
 }
 
+#ifdef CONFIG_IA32_EMULATION
+bool __ia32_enabled = !IS_ENABLED(CONFIG_IA32_EMULATION_DEFAULT_DISABLED);
+
+static int ia32_mode_override_cmdline(char *arg)
+{
+	if (!strcmp(arg, "disabled"))
+		__ia32_enabled = false;
+	else if (!strcmp(arg, "enabled"))
+		__ia32_enabled = true;
+
+	return 1;
+}
+__setup("ia32_mode=", ia32_mode_override_cmdline);
+#endif
+
 /*
  * Invoke a 32-bit syscall.  Called with IRQs on in CONTEXT_KERNEL.
  */
diff --git a/arch/x86/include/asm/ia32.h b/arch/x86/include/asm/ia32.h
index fada857f0a1e..c35e4a6d3317 100644
--- a/arch/x86/include/asm/ia32.h
+++ b/arch/x86/include/asm/ia32.h
@@ -68,6 +68,20 @@ extern void ia32_pick_mmap_layout(struct mm_struct *mm);
 
 #endif
 
-#endif /* CONFIG_IA32_EMULATION */
+extern bool __ia32_enabled;
+
+static inline bool ia32_enabled(void)
+{
+	return __ia32_enabled;
+}
+
+#else /* CONFIG_IA32_EMULATION */
+
+static inline bool ia32_enabled(void)
+{
+	return IS_ENABLED(CONFIG_X86_32);
+}
+
+#endif
 
 #endif /* _ASM_X86_IA32_H */
-- 
2.34.1

Re: [PATCH v3 1/5] x86: Make IA32_EMULATION boot time configurable

[Randy Dunlap]

On 6/16/23 05:57, Nikolay Borisov wrote:
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 53bab123a8ee..8bec431c97ce 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -3038,6 +3038,15 @@ config IA32_EMULATION
>  	  64-bit kernel. You should likely turn this on, unless you're
>  	  100% sure that you don't have any 32-bit programs left.
>  
> +config IA32_EMULATION_DEFAULT_DISABLED
> +	bool "IA32 emulation disabled by default"
> +	default n
> +	depends on IA32_EMULATION
> +	help
> +	  Make IA32 emulation disabled by default. This prevents loading 32bit

	                                                                 32-bit

> +	  processes and access to 32bit syscalls. If unsure, leave it to its

	                          32-bit

> +	  default value.

-- 
~Randy

Re: [PATCH v3 1/5] x86: Make IA32_EMULATION boot time configurable

[Borislav Petkov]

On Fri, Jun 16, 2023 at 03:57:26PM +0300, Nikolay Borisov wrote:
> Distributions would like to reduce their attack surface as much as
> possible but at the same time they'd want to retain flexibility to cater
> to a variety of legacy software. One such avenue where a balance has to
> be struck is in supporting 32bit syscalls/processes on 64bit kernels. Ideally
> it should be possible for the distribution to set their own policy and
> give users the ability to override those policies as appropriate.
> 
> In order to support this usecase, introduce
> CONFIG_IA32_EMULATION_DEFAULT_DISABLED compile time option, which
> controls whether 32bit processes/syscalls should be allowed or not. This
> allows distributions to set their preferred default behavior in their
> kernel configs.
> 
> On the other hand, in order to allow users to override the distro's
> policy, introduce the 'ia32_mode' parameter which allows overriding
> CONFIG_IA32_EMULATION_DEFAULT_DISABLED state at boot time.
> 
> Signed-off-by: Nikolay Borisov <nik.borisov@suse.com>
> ---
>  Documentation/admin-guide/kernel-parameters.txt |  5 +++++
>  arch/x86/Kconfig                                |  9 +++++++++
>  arch/x86/entry/common.c                         | 16 ++++++++++++++++
>  arch/x86/include/asm/ia32.h                     | 16 +++++++++++++++-
>  4 files changed, 45 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 9e5bab29685f..59b1e86ecd9d 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -1865,6 +1865,11 @@
>  			 0 -- machine default
>  			 1 -- force brightness inversion
>  
> +	ia32_mode=		[X86-64]
> +			Format: ia32_mode=disabled, ia32_mode=enabled

ia32_mode=(on|off)

is less typing. Especially if you're standing somewhere in a server room
and trying to type on some weird keyboard which always has the wrong
layout.

:-)

> +			Allows overriding the compile-time state of
> +			IA32_EMULATION_DEFAULT_DISABLED at boot time

Just say what "=on" and "=off" does here - loading of 32-bit programs
and 32-bit syscalls is enabled/disabled.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH v3 1/5] x86: Make IA32_EMULATION boot time configurable

[Nikolay Borisov]

On 21.06.23 г. 21:27 ч., Borislav Petkov wrote:
> On Fri, Jun 16, 2023 at 03:57:26PM +0300, Nikolay Borisov wrote:
>> Distributions would like to reduce their attack surface as much as
>> possible but at the same time they'd want to retain flexibility to cater
>> to a variety of legacy software. One such avenue where a balance has to
>> be struck is in supporting 32bit syscalls/processes on 64bit kernels. Ideally
>> it should be possible for the distribution to set their own policy and
>> give users the ability to override those policies as appropriate.
>>
>> In order to support this usecase, introduce
>> CONFIG_IA32_EMULATION_DEFAULT_DISABLED compile time option, which
>> controls whether 32bit processes/syscalls should be allowed or not. This
>> allows distributions to set their preferred default behavior in their
>> kernel configs.
>>
>> On the other hand, in order to allow users to override the distro's
>> policy, introduce the 'ia32_mode' parameter which allows overriding
>> CONFIG_IA32_EMULATION_DEFAULT_DISABLED state at boot time.
>>
>> Signed-off-by: Nikolay Borisov <nik.borisov@suse.com>
>> ---
>>   Documentation/admin-guide/kernel-parameters.txt |  5 +++++
>>   arch/x86/Kconfig                                |  9 +++++++++
>>   arch/x86/entry/common.c                         | 16 ++++++++++++++++
>>   arch/x86/include/asm/ia32.h                     | 16 +++++++++++++++-
>>   4 files changed, 45 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
>> index 9e5bab29685f..59b1e86ecd9d 100644
>> --- a/Documentation/admin-guide/kernel-parameters.txt
>> +++ b/Documentation/admin-guide/kernel-parameters.txt
>> @@ -1865,6 +1865,11 @@
>>   			 0 -- machine default
>>   			 1 -- force brightness inversion
>>   
>> +	ia32_mode=		[X86-64]
>> +			Format: ia32_mode=disabled, ia32_mode=enabled
> 
> ia32_mode=(on|off)

In the next version I called this ia32_emulation=on|off seems more 
descriptive.
> 
> is less typing. Especially if you're standing somewhere in a server room
> and trying to type on some weird keyboard which always has the wrong
> layout.
> 
> :-)
> 
>> +			Allows overriding the compile-time state of
>> +			IA32_EMULATION_DEFAULT_DISABLED at boot time
> 
> Just say what "=on" and "=off" does here - loading of 32-bit programs
> and 32-bit syscalls is enabled/disabled.

ack
> 

[PATCH v3 2/5] x86/entry: Rename ignore_sysret

[Nikolay Borisov]

Give ignore_sysret() a more descriptive name as it's actually used to make
32bit syscalls return ENOSYS, rather than doing anything specific with
regards to sysret.

Signed-off-by: Nikolay Borisov <nik.borisov@suse.com>
---
 arch/x86/entry/entry_64.S        | 4 ++--
 arch/x86/include/asm/processor.h | 2 +-
 arch/x86/kernel/cpu/common.c     | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index f31e286c2977..ccce0ccd8589 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -1519,12 +1519,12 @@ SYM_CODE_END(asm_exc_nmi)
  * This handles SYSCALL from 32-bit code.  There is no way to program
  * MSRs to fully disable 32-bit SYSCALL.
  */
-SYM_CODE_START(ignore_sysret)
+SYM_CODE_START(entry_SYSCALL32_ignore)
 	UNWIND_HINT_END_OF_STACK
 	ENDBR
 	mov	$-ENOSYS, %eax
 	sysretl
-SYM_CODE_END(ignore_sysret)
+SYM_CODE_END(entry_SYSCALL32_ignore)
 #endif
 
 .pushsection .text, "ax"
diff --git a/arch/x86/include/asm/processor.h b/arch/x86/include/asm/processor.h
index a1e4fa58b357..61c10b4e3e35 100644
--- a/arch/x86/include/asm/processor.h
+++ b/arch/x86/include/asm/processor.h
@@ -399,7 +399,7 @@ static inline unsigned long cpu_kernelmode_gs_base(int cpu)
 	return (unsigned long)per_cpu(fixed_percpu_data.gs_base, cpu);
 }
 
-extern asmlinkage void ignore_sysret(void);
+extern asmlinkage void entry_SYSCALL32_ignore(void);
 
 /* Save actual FS/GS selectors and bases to current->thread */
 void current_save_fsgs(void);
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 80710a68ef7d..b20774181e1a 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -2066,7 +2066,7 @@ void syscall_init(void)
 		    (unsigned long)(cpu_entry_stack(smp_processor_id()) + 1));
 	wrmsrl_safe(MSR_IA32_SYSENTER_EIP, (u64)entry_SYSENTER_compat);
 #else
-	wrmsrl_cstar((unsigned long)ignore_sysret);
+	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);
-- 
2.34.1

Re: [PATCH v3 2/5] x86/entry: Rename ignore_sysret

[Thomas Gleixner]

On Fri, Jun 16 2023 at 15:57, Nikolay Borisov wrote:

> Give ignore_sysret() a more descriptive name as it's actually used to make
> 32bit syscalls return ENOSYS, rather than doing anything specific with

That's not really correct. This is not about 32bit syscalls in general.

It's specifically about the SYSCALL entry point on 32bit, right?

The reason why this is required is because 32bit SYSCALL cannot be
disabled in hardware.

Thanks,

        tglx

Re: [PATCH v3 2/5] x86/entry: Rename ignore_sysret

[Nikolay Borisov]

On 18.06.23 г. 23:51 ч., Thomas Gleixner wrote:
> On Fri, Jun 16 2023 at 15:57, Nikolay Borisov wrote:
> 
>> Give ignore_sysret() a more descriptive name as it's actually used to make
>> 32bit syscalls return ENOSYS, rather than doing anything specific with
> 
> That's not really correct. This is not about 32bit syscalls in general.
> 
> It's specifically about the SYSCALL entry point on 32bit, right?
> 
> The reason why this is required is because 32bit SYSCALL cannot be
> disabled in hardware.

How about:

"SYSCALL instruction cannot really be disabled in compatibility mode. 
The best that can be done is to configure the CSTAR msr with a minimal 
handler that returns directly some error value.

ignore_sysret is this minimal handler in the Linux kernel. Rename it by 
giving it a more descriptive name."

> 
> Thanks,
> 
>          tglx
> 
> 

[PATCH v3 3/5] x86/entry: Compile entry_SYSCALL32_ignore unconditionally

[Nikolay Borisov]

In upcomiing patches entry_SYSCALL32_ignore() could be used even if
CONFIG_IA32_EMULATION is selected but IA32 support is disabled either
via CONFIG_IA32_EMULATION_DEFAULT_DISABLED or the runtime override.i

Just compile it unconditionally.

Signed-off-by: Nikolay Borisov <nik.borisov@suse.com>
---
 arch/x86/entry/entry_64.S | 2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index ccce0ccd8589..7068af44008a 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -1514,7 +1514,6 @@ SYM_CODE_START(asm_exc_nmi)
 	iretq
 SYM_CODE_END(asm_exc_nmi)
 
-#ifndef CONFIG_IA32_EMULATION
 /*
  * This handles SYSCALL from 32-bit code.  There is no way to program
  * MSRs to fully disable 32-bit SYSCALL.
@@ -1525,7 +1524,6 @@ SYM_CODE_START(entry_SYSCALL32_ignore)
 	mov	$-ENOSYS, %eax
 	sysretl
 SYM_CODE_END(entry_SYSCALL32_ignore)
-#endif
 
 .pushsection .text, "ax"
 	__FUNC_ALIGN
-- 
2.34.1

Re: [PATCH v3 3/5] x86/entry: Compile entry_SYSCALL32_ignore unconditionally

[Thomas Gleixner]

On Fri, Jun 16 2023 at 15:57, Nikolay Borisov wrote:
> In upcomiing patches entry_SYSCALL32_ignore() could be used even if

comiing?

> CONFIG_IA32_EMULATION is selected but IA32 support is disabled either
> via CONFIG_IA32_EMULATION_DEFAULT_DISABLED or the runtime override.i

override.i ?

Aside of the lack of a spell checker, this sentence is not really
well structured and helpful.

You already introduced CONFIG_IA32_EMULATION_DEFAULT_DISABLED in the
first patch, which is questionable to start with because the config
switch is only fully functional when everything is in place.

So up to that point the command line option and the config switch can
create inconsistent state, no?

So the right thing to do is to introduce the global variable which
controls that first. As it is 'false' it won't do anything, but allows
you to build up the code. The last step adds the config option and the
command line parsing.

Then the changelog might read like this:

 "To limit the IA32 exposure on 64bit kernels while keeping the
  flexibility for the user to enable it when required, the compile time
  enable/disable via CONFIG_IA32_EMULATION is not good enough and will
  be complemented with a kernel command line option.

  Right now entry_SYSCALL32_ignore() is only compiled when
  CONFIG_IA32_EMULATION=n, but boot-time enable- / disablement obviously
  requires it to be unconditionally available.

  Remove the #ifndef CONFIG_IA32_EMULATION guard"

The point is that changelogs need to convey enough information on their
own that they make sense without having access to the full context of the
patch series.

Thanks,

        tglx

[PATCH v3 4/5] x86/elf: Make loading of 32bit processes depend on ia32_enabled()

[Nikolay Borisov]

Major aspect of ia32 emulation is the ability to load 32bit processes.
That's currently decided (among others) by compat_elf_check_arch().

Make this macro also consider the runtime state of ia32 emulation before
allowing to load a 32bit process.

Signed-off-by: Nikolay Borisov <nik.borisov@suse.com>
---
 arch/x86/include/asm/elf.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/elf.h b/arch/x86/include/asm/elf.h
index 18fd06f7936a..a0234dfd1031 100644
--- a/arch/x86/include/asm/elf.h
+++ b/arch/x86/include/asm/elf.h
@@ -7,6 +7,7 @@
  */
 #include <linux/thread_info.h>
 
+#include <asm/ia32.h>
 #include <asm/ptrace.h>
 #include <asm/user.h>
 #include <asm/auxvec.h>
@@ -149,7 +150,7 @@ do {						\
 	((x)->e_machine == EM_X86_64)
 
 #define compat_elf_check_arch(x)					\
-	(elf_check_arch_ia32(x) ||					\
+	((elf_check_arch_ia32(x) && ia32_enabled()) ||			\
 	 (IS_ENABLED(CONFIG_X86_X32_ABI) && (x)->e_machine == EM_X86_64))
 
 static inline void elf_common_init(struct thread_struct *t,
-- 
2.34.1

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

[Nikolay Borisov]

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(-)

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
-- 
2.34.1

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

[Thomas Gleixner]

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

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

[Nikolay Borisov]

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

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

[Thomas Gleixner]

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