[PATCH v3 1/1] x86/fred: Fix INT80 emulation for FRED

[PATCH v3 1/1] x86/fred: Fix INT80 emulation for FRED

[Xin Li (Intel)]

Add a FRED-specific INT80 handler fred_int80_emulation():

1) As INT instructions and hardware interrupts are separate event
   types, FRED does not preclude the use of vector 0x80 for external
   interrupts. As a result the FRED setup code does *NOT* reserve
   vector 0x80 and calling int80_is_external() is not merely
   suboptimal but actively incorrect: it could cause a system call
   to be incorrectly ignored.

2) fred_int80_emulation(), only called for handling vector 0x80 of
   event type EVENT_TYPE_SWINT, will NEVER be called to handle any
   external interrupt (event type EVENT_TYPE_EXTINT).

3) The FRED kernel entry handler does *NOT* dispatch INT instructions,
   which is of event type EVENT_TYPE_SWINT, so compared with
   do_int80_emulation(), there is no need to do any user mode check.

4) int80_emulation() does a CLEAR_BRANCH_HISTORY, which is likely
   overkill for new x86 CPU implementations that support FRED.

5) int $0x80 is the FAST path for 32-bit system calls under FRED.

A dedicated FRED INT80 handler duplicates quite a bit of the code in
do_int80_emulation(), but it avoids sprinkling more tests and seems
more readable. Just remember that we can always unify common stuff
later if it turns out that it won't diverge anymore, i.e., after the
FRED code settles.

Fixes: 55617fb991df ("x86/entry: Do not allow external 0x80 interrupts")

Suggested-by: H. Peter Anvin (Intel) <hpa@zytor.com>
Signed-off-by: Xin Li (Intel) <xin@zytor.com>
---

Changes since v2:
* Add comments explaining the reasons why a FRED-specific INT80 handler
  is required to the head comment of fred_int80_emulation(), not just
  the change log (H. Peter Anvin).
* Incorporate extra clarifications from H. Peter Anvin.
* Fix a few typos and wordings (H. Peter Anvin).
* Add a maintainer tip to the change log and head comment: unify common
  stuff later, i.e., after the code settles (Borislav Petkov).

Change since v1:
* Prefer a FRED-specific INT80 handler instead of sprinkling more tests
  around (Borislav Petkov).
---
 arch/x86/entry/common.c     | 64 +++++++++++++++++++++++++++++++++++++
 arch/x86/entry/entry_fred.c |  2 +-
 2 files changed, 65 insertions(+), 1 deletion(-)

diff --git a/arch/x86/entry/common.c b/arch/x86/entry/common.c
index 6de50b80702e..213d9b33a63c 100644
--- a/arch/x86/entry/common.c
+++ b/arch/x86/entry/common.c
@@ -255,6 +255,70 @@ __visible noinstr void do_int80_emulation(struct pt_regs *regs)
 	instrumentation_end();
 	syscall_exit_to_user_mode(regs);
 }
+
+#ifdef CONFIG_X86_FRED
+/*
+ * A FRED-specific INT80 handler fred_int80_emulation() is required:
+ *
+ * 1) As INT instructions and hardware interrupts are separate event
+ *    types, FRED does not preclude the use of vector 0x80 for external
+ *    interrupts. As a result the FRED setup code does *NOT* reserve
+ *    vector 0x80 and calling int80_is_external() is not merely
+ *    suboptimal but actively incorrect: it could cause a system call
+ *    to be incorrectly ignored.
+ *
+ * 2) fred_int80_emulation(), only called for handling vector 0x80 of
+ *    event type EVENT_TYPE_SWINT, will NEVER be called to handle any
+ *    external interrupt (event type EVENT_TYPE_EXTINT).
+ *
+ * 3) The FRED kernel entry handler does *NOT* dispatch INT instructions,
+ *    which is of event type EVENT_TYPE_SWINT, so compared with
+ *    do_int80_emulation(), there is no need to do any user mode check.
+ *
+ * 4) int80_emulation() does a CLEAR_BRANCH_HISTORY, which is likely
+ *    overkill for new x86 CPU implementations that support FRED.
+ *
+ * 5) int $0x80 is the FAST path for 32-bit system calls under FRED.
+ *
+ * A dedicated FRED INT80 handler duplicates quite a bit of the code in
+ * do_int80_emulation(), but it avoids sprinkling more tests and seems
+ * more readable. Just remember that we can always unify common stuff
+ * later if it turns out that it won't diverge anymore, i.e., after the
+ * FRED code settles.
+ */
+DEFINE_FREDENTRY_RAW(int80_emulation)
+{
+	int nr;
+
+	enter_from_user_mode(regs);
+
+	instrumentation_begin();
+	add_random_kstack_offset();
+
+	/*
+	 * FRED pushed 0 into regs::orig_ax and regs::ax contains the
+	 * syscall number.
+	 *
+	 * User tracing code (ptrace or signal handlers) might assume
+	 * that the regs::orig_ax contains a 32-bit number on invoking
+	 * a 32-bit syscall.
+	 *
+	 * Establish the syscall convention by saving the 32bit truncated
+	 * syscall number in regs::orig_ax and by invalidating regs::ax.
+	 */
+	regs->orig_ax = regs->ax & GENMASK(31, 0);
+	regs->ax = -ENOSYS;
+
+	nr = syscall_32_enter(regs);
+
+	local_irq_enable();
+	nr = syscall_enter_from_user_mode_work(regs, nr);
+	do_syscall_32_irqs_on(regs, nr);
+
+	instrumentation_end();
+	syscall_exit_to_user_mode(regs);
+}
+#endif
 #else /* CONFIG_IA32_EMULATION */
 
 /* Handles int $0x80 on a 32bit kernel */
diff --git a/arch/x86/entry/entry_fred.c b/arch/x86/entry/entry_fred.c
index ac120cbdaaf2..9fa18b8c7f26 100644
--- a/arch/x86/entry/entry_fred.c
+++ b/arch/x86/entry/entry_fred.c
@@ -66,7 +66,7 @@ static noinstr void fred_intx(struct pt_regs *regs)
 	/* INT80 */
 	case IA32_SYSCALL_VECTOR:
 		if (ia32_enabled())
-			return int80_emulation(regs);
+			return fred_int80_emulation(regs);
 		fallthrough;
 #endif
 

base-commit: 367dc2b68007e8ca00a0d8dc9afb69bff5451ae7
-- 
2.44.0

Re: [PATCH v3 1/1] x86/fred: Fix INT80 emulation for FRED

[Borislav Petkov]

On Tue, Apr 16, 2024 at 11:30:01PM -0700, Xin Li (Intel) wrote:
> 3) The FRED kernel entry handler does *NOT* dispatch INT instructions,
>    which is of event type EVENT_TYPE_SWINT, so compared with
>    do_int80_emulation(), there is no need to do any user mode check.

What does that mean?

An event handler doesn't dispatch INT insns?

/me is confused.

> +#ifdef CONFIG_X86_FRED
> +/*
> + * A FRED-specific INT80 handler fred_int80_emulation() is required:
> + *
> + * 1) As INT instructions and hardware interrupts are separate event
> + *    types, FRED does not preclude the use of vector 0x80 for external
> + *    interrupts. As a result the FRED setup code does *NOT* reserve
> + *    vector 0x80 and calling int80_is_external() is not merely
> + *    suboptimal but actively incorrect: it could cause a system call
> + *    to be incorrectly ignored.
> + *
> + * 2) fred_int80_emulation(), only called for handling vector 0x80 of
> + *    event type EVENT_TYPE_SWINT, will NEVER be called to handle any
> + *    external interrupt (event type EVENT_TYPE_EXTINT).
> + *
> + * 3) The FRED kernel entry handler does *NOT* dispatch INT instructions,
> + *    which is of event type EVENT_TYPE_SWINT, so compared with
> + *    do_int80_emulation(), there is no need to do any user mode check.
> + *
> + * 4) int80_emulation() does a CLEAR_BRANCH_HISTORY, which is likely
> + *    overkill for new x86 CPU implementations that support FRED.
> + *
> + * 5) int $0x80 is the FAST path for 32-bit system calls under FRED.
> + *
> + * A dedicated FRED INT80 handler duplicates quite a bit of the code in
> + * do_int80_emulation(), but it avoids sprinkling more tests and seems
> + * more readable. Just remember that we can always unify common stuff
> + * later if it turns out that it won't diverge anymore, i.e., after the
> + * FRED code settles.
> + */

And this is talking about duplication above and that text is duplicated
from the commit message. :)

I'll zap it when applying.

Thx.

-- 
Regards/Gruss,
    Boris.

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

Re: [PATCH v3 1/1] x86/fred: Fix INT80 emulation for FRED

[Nikolay Borisov]

On 17.04.24 г. 9:30 ч., Xin Li (Intel) wrote:
> Add a FRED-specific INT80 handler fred_int80_emulation():
> 
> 1) As INT instructions and hardware interrupts are separate event
>     types, FRED does not preclude the use of vector 0x80 for external
>     interrupts. As a result the FRED setup code does *NOT* reserve
>     vector 0x80 and calling int80_is_external() is not merely
>     suboptimal but actively incorrect: it could cause a system call
>     to be incorrectly ignored.
> 
> 2) fred_int80_emulation(), only called for handling vector 0x80 of
>     event type EVENT_TYPE_SWINT, will NEVER be called to handle any
>     external interrupt (event type EVENT_TYPE_EXTINT).
> 
> 3) The FRED kernel entry handler does *NOT* dispatch INT instructions,
>     which is of event type EVENT_TYPE_SWINT, so compared with
>     do_int80_emulation(), there is no need to do any user mode check.
> 
> 4) int80_emulation() does a CLEAR_BRANCH_HISTORY, which is likely >     overkill for new x86 CPU implementations that support FRED.

Well, that's a bit of an overstatement/speculation, because 
clear_branch_history will only be effective if the machine is 
susceptible to the given bug and there isn't a better options (i.e using 
a hardware bit controlling the respective aspect of the CPU).
> 
> 5) int $0x80 is the FAST path for 32-bit system calls under FRED.
> 
> A dedicated FRED INT80 handler duplicates quite a bit of the code in
> do_int80_emulation(), but it avoids sprinkling more tests and seems
> more readable. Just remember that we can always unify common stuff
> later if it turns out that it won't diverge anymore, i.e., after the
> FRED code settles.
> 
> Fixes: 55617fb991df ("x86/entry: Do not allow external 0x80 interrupts")
> 
> Suggested-by: H. Peter Anvin (Intel) <hpa@zytor.com>
> Signed-off-by: Xin Li (Intel) <xin@zytor.com>
> ---
> 
> Changes since v2:
> * Add comments explaining the reasons why a FRED-specific INT80 handler
>    is required to the head comment of fred_int80_emulation(), not just
>    the change log (H. Peter Anvin).
> * Incorporate extra clarifications from H. Peter Anvin.
> * Fix a few typos and wordings (H. Peter Anvin).
> * Add a maintainer tip to the change log and head comment: unify common
>    stuff later, i.e., after the code settles (Borislav Petkov).
> 
> Change since v1:
> * Prefer a FRED-specific INT80 handler instead of sprinkling more tests
>    around (Borislav Petkov).
> ---
>   arch/x86/entry/common.c     | 64 +++++++++++++++++++++++++++++++++++++
>   arch/x86/entry/entry_fred.c |  2 +-
>   2 files changed, 65 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/entry/common.c b/arch/x86/entry/common.c
> index 6de50b80702e..213d9b33a63c 100644
> --- a/arch/x86/entry/common.c
> +++ b/arch/x86/entry/common.c
> @@ -255,6 +255,70 @@ __visible noinstr void do_int80_emulation(struct pt_regs *regs)
>   	instrumentation_end();
>   	syscall_exit_to_user_mode(regs);
>   }
> +
> +#ifdef CONFIG_X86_FRED
> +/*
> + * A FRED-specific INT80 handler fred_int80_emulation() is required:
> + *
> + * 1) As INT instructions and hardware interrupts are separate event
> + *    types, FRED does not preclude the use of vector 0x80 for external
> + *    interrupts. As a result the FRED setup code does *NOT* reserve
> + *    vector 0x80 and calling int80_is_external() is not merely
> + *    suboptimal but actively incorrect: it could cause a system call
> + *    to be incorrectly ignored.
> + *
> + * 2) fred_int80_emulation(), only called for handling vector 0x80 of
> + *    event type EVENT_TYPE_SWINT, will NEVER be called to handle any
> + *    external interrupt (event type EVENT_TYPE_EXTINT).
> + *
> + * 3) The FRED kernel entry handler does *NOT* dispatch INT instructions,
> + *    which is of event type EVENT_TYPE_SWINT, so compared with
> + *    do_int80_emulation(), there is no need to do any user mode check.
> + *
> + * 4) int80_emulation() does a CLEAR_BRANCH_HISTORY, which is likely
> + *    overkill for new x86 CPU implementations that support FRED.
> + *
> + * 5) int $0x80 is the FAST path for 32-bit system calls under FRED.
> + *
> + * A dedicated FRED INT80 handler duplicates quite a bit of the code in
> + * do_int80_emulation(), but it avoids sprinkling more tests and seems
> + * more readable. Just remember that we can always unify common stuff
> + * later if it turns out that it won't diverge anymore, i.e., after the
> + * FRED code settles.
> + */
> +DEFINE_FREDENTRY_RAW(int80_emulation)
> +{
> +	int nr;
> +
> +	enter_from_user_mode(regs);
> +
> +	instrumentation_begin();
> +	add_random_kstack_offset();
> +
> +	/*
> +	 * FRED pushed 0 into regs::orig_ax and regs::ax contains the
> +	 * syscall number.
> +	 *
> +	 * User tracing code (ptrace or signal handlers) might assume
> +	 * that the regs::orig_ax contains a 32-bit number on invoking
> +	 * a 32-bit syscall.
> +	 *
> +	 * Establish the syscall convention by saving the 32bit truncated
> +	 * syscall number in regs::orig_ax and by invalidating regs::ax.
> +	 */
> +	regs->orig_ax = regs->ax & GENMASK(31, 0);
> +	regs->ax = -ENOSYS;
> +
> +	nr = syscall_32_enter(regs);
> +
> +	local_irq_enable();
> +	nr = syscall_enter_from_user_mode_work(regs, nr);
> +	do_syscall_32_irqs_on(regs, nr);
> +
> +	instrumentation_end();
> +	syscall_exit_to_user_mode(regs);
> +}
> +#endif
>   #else /* CONFIG_IA32_EMULATION */
>   
>   /* Handles int $0x80 on a 32bit kernel */
> diff --git a/arch/x86/entry/entry_fred.c b/arch/x86/entry/entry_fred.c
> index ac120cbdaaf2..9fa18b8c7f26 100644
> --- a/arch/x86/entry/entry_fred.c
> +++ b/arch/x86/entry/entry_fred.c
> @@ -66,7 +66,7 @@ static noinstr void fred_intx(struct pt_regs *regs)
>   	/* INT80 */
>   	case IA32_SYSCALL_VECTOR:
>   		if (ia32_enabled())
> -			return int80_emulation(regs);
> +			return fred_int80_emulation(regs);
>   		fallthrough;
>   #endif
>   
> 
> base-commit: 367dc2b68007e8ca00a0d8dc9afb69bff5451ae7

Re: [PATCH v3 1/1] x86/fred: Fix INT80 emulation for FRED

[H. Peter Anvin]

On 4/17/24 02:38, Borislav Petkov wrote:
> On Tue, Apr 16, 2024 at 11:30:01PM -0700, Xin Li (Intel) wrote:
>> 3) The FRED kernel entry handler does *NOT* dispatch INT instructions,
>>     which is of event type EVENT_TYPE_SWINT, so compared with
>>     do_int80_emulation(), there is no need to do any user mode check.
> 
> What does that mean?
> 
> An event handler doesn't dispatch INT insns?
> 
> /me is confused.

FRED has separate entry flows depending on if the event came from user 
space or kernel space:

asm_fred_entrypoint_user -> fred_entry_from_user

asm_fred_entrypoint_kernel -> fred_entry_from_kernel

fred_entry_from_kernel does not invoke fred_intx() if the event type is 
EVENT_TYPE_SWINT, instead it falls through to fred_bad_type(). Perhaps 
fred_intx() should be renamed fred_intx_user() for additional clarity.

(It might also we worth noting in that function that the reason int 
$0x03 and int $0x04 are dispatched as INT3 and INTO is to be fully user 
space compatible with legacy IDT, which behaves similarly.)

Thus, the int $0x80 code is simply not reachable from kernel space; if 
kernel code were to invoke int $0x80 or any other INT instruction it 
will error out before getting to this code.

>> +#ifdef CONFIG_X86_FRED
>> +/*
>> + * A FRED-specific INT80 handler fred_int80_emulation() is required:
>> + *
>> + * 1) As INT instructions and hardware interrupts are separate event
>> + *    types, FRED does not preclude the use of vector 0x80 for external
>> + *    interrupts. As a result the FRED setup code does *NOT* reserve
>> + *    vector 0x80 and calling int80_is_external() is not merely
>> + *    suboptimal but actively incorrect: it could cause a system call
>> + *    to be incorrectly ignored.
>> + *
>> + * 2) fred_int80_emulation(), only called for handling vector 0x80 of
>> + *    event type EVENT_TYPE_SWINT, will NEVER be called to handle any
>> + *    external interrupt (event type EVENT_TYPE_EXTINT).
>> + *
>> + * 3) The FRED kernel entry handler does *NOT* dispatch INT instructions,
>> + *    which is of event type EVENT_TYPE_SWINT, so compared with
>> + *    do_int80_emulation(), there is no need to do any user mode check.
>> + *
>> + * 4) int80_emulation() does a CLEAR_BRANCH_HISTORY, which is likely
>> + *    overkill for new x86 CPU implementations that support FRED.
>> + *
>> + * 5) int $0x80 is the FAST path for 32-bit system calls under FRED.
>> + *
>> + * A dedicated FRED INT80 handler duplicates quite a bit of the code in
>> + * do_int80_emulation(), but it avoids sprinkling more tests and seems
>> + * more readable. Just remember that we can always unify common stuff
>> + * later if it turns out that it won't diverge anymore, i.e., after the
>> + * FRED code settles.
>> + */
> 
> And this is talking about duplication above and that text is duplicated
> from the commit message. :)
> 
> I'll zap it when applying.
> 

I suggested putting it into a comment for future reference. Obviously no 
need to duplicate it in the commit message :)

	-hpa

Re: [PATCH v3 1/1] x86/fred: Fix INT80 emulation for FRED

[Xin Li]

On 4/17/2024 2:38 AM, Borislav Petkov wrote:
> On Tue, Apr 16, 2024 at 11:30:01PM -0700, Xin Li (Intel) wrote:
>> 3) The FRED kernel entry handler does *NOT* dispatch INT instructions,
>>     which is of event type EVENT_TYPE_SWINT, so compared with
>>     do_int80_emulation(), there is no need to do any user mode check.
> 
> What does that mean?
> 
> An event handler doesn't dispatch INT insns?
> 
> /me is confused.
> 

Maybe better to say?

The FRED kernel entry handler fred_entry_from_kernel() calls 
fred_bad_type() if an event is of type EVENT_TYPE_SWINT because Kernel 
does not use INT insns! So if the kernel is handling an INT insn, it can 
only be from a user level application.


>> + * A dedicated FRED INT80 handler duplicates quite a bit of the code in
>> + * do_int80_emulation(), but it avoids sprinkling more tests and seems
>> + * more readable. Just remember that we can always unify common stuff
>> + * later if it turns out that it won't diverge anymore, i.e., after the
>> + * FRED code settles.
>> + */
> 
> And this is talking about duplication above and that text is duplicated
> from the commit message. :)

I wanted to keep it as a TODO.  I can remove it in the next iteration.

> 
> I'll zap it when applying.
> 
> Thx.
> 

Re: [PATCH v3 1/1] x86/fred: Fix INT80 emulation for FRED

[H. Peter Anvin]

On 4/17/24 04:02, Nikolay Borisov wrote:
> 
> On 17.04.24 г. 9:30 ч., Xin Li (Intel) wrote:
>> Add a FRED-specific INT80 handler fred_int80_emulation():
>>
>> 1) As INT instructions and hardware interrupts are separate event
>>     types, FRED does not preclude the use of vector 0x80 for external
>>     interrupts. As a result the FRED setup code does *NOT* reserve
>>     vector 0x80 and calling int80_is_external() is not merely
>>     suboptimal but actively incorrect: it could cause a system call
>>     to be incorrectly ignored.
>>
>> 2) fred_int80_emulation(), only called for handling vector 0x80 of
>>     event type EVENT_TYPE_SWINT, will NEVER be called to handle any
>>     external interrupt (event type EVENT_TYPE_EXTINT).
>>
>> 3) The FRED kernel entry handler does *NOT* dispatch INT instructions,
>>     which is of event type EVENT_TYPE_SWINT, so compared with
>>     do_int80_emulation(), there is no need to do any user mode check.
>>
>> 4) int80_emulation() does a CLEAR_BRANCH_HISTORY, which is likely 
>> >     overkill for new x86 CPU implementations that support FRED.
> 
> Well, that's a bit of an overstatement/speculation, because 
> clear_branch_history will only be effective if the machine is 
> susceptible to the given bug and there isn't a better options (i.e using 
> a hardware bit controlling the respective aspect of the CPU).
>>

It would seem like a huge stretch to expect that a FRED-capable CPU 
would not have such a facility. This is a matter of establishing a 
baseline for FRED-capable hardware.

It would make more sense to me to add it if we turn out to need it; note 
that FRED code is currently only enabled on demand, in order to defend 
against bit rot until we have physical hardware.

Now, if this is still desired, it *probably* belongs better in either 
fred_intx()/fred_other() or asm_fred_entrypoint_user, depending on if 
this ought to be done for all entries from userspace or only system calls.

	-hpa

Re: [PATCH v3 1/1] x86/fred: Fix INT80 emulation for FRED

[H. Peter Anvin]

On 4/17/24 08:06, Xin Li wrote:
> On 4/17/2024 2:38 AM, Borislav Petkov wrote:
>> On Tue, Apr 16, 2024 at 11:30:01PM -0700, Xin Li (Intel) wrote:
>>> 3) The FRED kernel entry handler does *NOT* dispatch INT instructions,
>>>     which is of event type EVENT_TYPE_SWINT, so compared with
>>>     do_int80_emulation(), there is no need to do any user mode check.
>>
>> What does that mean?
>>
>> An event handler doesn't dispatch INT insns?
>>
>> /me is confused.
>>
> 
> Maybe better to say?
> 
> The FRED kernel entry handler fred_entry_from_kernel() calls 
> fred_bad_type() if an event is of type EVENT_TYPE_SWINT because Kernel 
> does not use INT insns! So if the kernel is handling an INT insn, it can 
> only be from a user level application.
> 

(Note: if we at some point in the future we may end up adding INT calls 
from the kernel, at least on FRED it would *still* not share the same 
code paths and so int $0x80 would not get dispatched.)

	-hpa

Re: [PATCH v3 1/1] x86/fred: Fix INT80 emulation for FRED

[Xin Li]

On 4/17/2024 7:59 AM, H. Peter Anvin wrote:
> On 4/17/24 02:38, Borislav Petkov wrote:
>> On Tue, Apr 16, 2024 at 11:30:01PM -0700, Xin Li (Intel) wrote:
>>> 3) The FRED kernel entry handler does *NOT* dispatch INT instructions,
>>>     which is of event type EVENT_TYPE_SWINT, so compared with
>>>     do_int80_emulation(), there is no need to do any user mode check.
>>
>> What does that mean?
>>
>> An event handler doesn't dispatch INT insns?
>>
>> /me is confused.
> 
> FRED has separate entry flows depending on if the event came from user 
> space or kernel space:
> 
> asm_fred_entrypoint_user -> fred_entry_from_user
> 
> asm_fred_entrypoint_kernel -> fred_entry_from_kernel
> 
> fred_entry_from_kernel does not invoke fred_intx() if the event type is 
> EVENT_TYPE_SWINT, instead it falls through to fred_bad_type(). Perhaps 
> fred_intx() should be renamed fred_intx_user() for additional clarity.

This is a good idea, and again naming is so important.

> 
> (It might also we worth noting in that function that the reason int 
> $0x03 and int $0x04 are dispatched as INT3 and INTO is to be fully user 
> space compatible with legacy IDT, which behaves similarly.)

Yeah, this is subtle, and we'd better make it explicit with comments.

FRED distinguishes int $0x03/$0x04 from INT3/INTO with event type 
EVENT_TYPE_SWINT and EVENT_TYPE_SWEXC, and the Linux kernel itself can 
still use INT3/INTO, however int $0x03/$0x04 is NOT allowed from kernel 
context.

> 
> Thus, the int $0x80 code is simply not reachable from kernel space; if 
> kernel code were to invoke int $0x80 or any other INT instruction it 
> will error out before getting to this code.
> 
>>> +#ifdef CONFIG_X86_FRED
>>> +/*
>>> + * A FRED-specific INT80 handler fred_int80_emulation() is required:
>>> + *
>>> + * 1) As INT instructions and hardware interrupts are separate event
>>> + *    types, FRED does not preclude the use of vector 0x80 for external
>>> + *    interrupts. As a result the FRED setup code does *NOT* reserve
>>> + *    vector 0x80 and calling int80_is_external() is not merely
>>> + *    suboptimal but actively incorrect: it could cause a system call
>>> + *    to be incorrectly ignored.
>>> + *
>>> + * 2) fred_int80_emulation(), only called for handling vector 0x80 of
>>> + *    event type EVENT_TYPE_SWINT, will NEVER be called to handle any
>>> + *    external interrupt (event type EVENT_TYPE_EXTINT).
>>> + *
>>> + * 3) The FRED kernel entry handler does *NOT* dispatch INT 
>>> instructions,
>>> + *    which is of event type EVENT_TYPE_SWINT, so compared with
>>> + *    do_int80_emulation(), there is no need to do any user mode check.
>>> + *
>>> + * 4) int80_emulation() does a CLEAR_BRANCH_HISTORY, which is likely
>>> + *    overkill for new x86 CPU implementations that support FRED.
>>> + *
>>> + * 5) int $0x80 is the FAST path for 32-bit system calls under FRED.
>>> + *
>>> + * A dedicated FRED INT80 handler duplicates quite a bit of the code in
>>> + * do_int80_emulation(), but it avoids sprinkling more tests and seems
>>> + * more readable. Just remember that we can always unify common stuff
>>> + * later if it turns out that it won't diverge anymore, i.e., after the
>>> + * FRED code settles.
>>> + */
>>
>> And this is talking about duplication above and that text is duplicated
>> from the commit message. :)
>>
>> I'll zap it when applying.
>>
> 
> I suggested putting it into a comment for future reference. Obviously no 
> need to duplicate it in the commit message :)
> 

yes!

Re: [PATCH v3 1/1] x86/fred: Fix INT80 emulation for FRED

[Xin Li]

On 4/17/2024 8:07 AM, H. Peter Anvin wrote:
> On 4/17/24 04:02, Nikolay Borisov wrote:
>>
>> On 17.04.24 г. 9:30 ч., Xin Li (Intel) wrote:
>>> 4) int80_emulation() does a CLEAR_BRANCH_HISTORY, which is likely 
>>> >     overkill for new x86 CPU implementations that support FRED.
>>
>> Well, that's a bit of an overstatement/speculation, because 
>> clear_branch_history will only be effective if the machine is 
>> susceptible to the given bug and there isn't a better options (i.e 
>> using a hardware bit controlling the respective aspect of the CPU).
>>>
> 
> It would seem like a huge stretch to expect that a FRED-capable CPU 
> would not have such a facility. This is a matter of establishing a 
> baseline for FRED-capable hardware.
> 
> It would make more sense to me to add it if we turn out to need it; note 
> that FRED code is currently only enabled on demand, in order to defend 
> against bit rot until we have physical hardware.
> 
> Now, if this is still desired, it *probably* belongs better in either 
> fred_intx()/fred_other() or asm_fred_entrypoint_user, depending on if 
> this ought to be done for all entries from userspace or only system calls.

My bad that I didn't make this a good comment, even neglected that 
clear_branch_history is just a nop on machines w/o such security issues.


So how about?

int80_emulation() does a CLEAR_BRANCH_HISTORY, which is IDT-specific.
While FRED will likely take a different approach when it is needed:
it *probably* belongs in either fred_intx()/fred_other() or
asm_fred_entrypoint_user(), depending on if this ought to be done for 
all entries from userspace or only system calls.

Re: [PATCH v3 1/1] x86/fred: Fix INT80 emulation for FRED

[H. Peter Anvin]

On 4/17/24 08:55, Xin Li wrote:
> On 4/17/2024 8:07 AM, H. Peter Anvin wrote:
>> On 4/17/24 04:02, Nikolay Borisov wrote:
>>>
>>> On 17.04.24 г. 9:30 ч., Xin Li (Intel) wrote:
>>>> 4) int80_emulation() does a CLEAR_BRANCH_HISTORY, which is likely 
>>>> >     overkill for new x86 CPU implementations that support FRED.
>>>
>>> Well, that's a bit of an overstatement/speculation, because 
>>> clear_branch_history will only be effective if the machine is 
>>> susceptible to the given bug and there isn't a better options (i.e 
>>> using a hardware bit controlling the respective aspect of the CPU).
>>>>
>>
>> It would seem like a huge stretch to expect that a FRED-capable CPU 
>> would not have such a facility. This is a matter of establishing a 
>> baseline for FRED-capable hardware.
>>
>> It would make more sense to me to add it if we turn out to need it; 
>> note that FRED code is currently only enabled on demand, in order to 
>> defend against bit rot until we have physical hardware.
>>
>> Now, if this is still desired, it *probably* belongs better in either 
>> fred_intx()/fred_other() or asm_fred_entrypoint_user, depending on if 
>> this ought to be done for all entries from userspace or only system 
>> calls.
> 
> My bad that I didn't make this a good comment, even neglected that 
> clear_branch_history is just a nop on machines w/o such security issues.
> 
> 
> So how about?
> 
> int80_emulation() does a CLEAR_BRANCH_HISTORY, which is IDT-specific.
> While FRED will likely take a different approach when it is needed:

s/when/if it is ever needed/

> it *probably* belongs in either fred_intx()/fred_other() or
> asm_fred_entrypoint_user(), depending on if this ought to be done for 
> all entries from userspace or only system calls.

Re: [PATCH v3 1/1] x86/fred: Fix INT80 emulation for FRED

[Borislav Petkov]

On Wed, Apr 17, 2024 at 07:59:25AM -0700, H. Peter Anvin wrote:
> FRED has separate entry flows depending on if the event came from user space
> or kernel space:
> 
> asm_fred_entrypoint_user -> fred_entry_from_user
> 
> asm_fred_entrypoint_kernel -> fred_entry_from_kernel
> 
> fred_entry_from_kernel does not invoke fred_intx() if the event type is
> EVENT_TYPE_SWINT, instead it falls through to fred_bad_type(). Perhaps
> fred_intx() should be renamed fred_intx_user() for additional clarity.

Sure.

> (It might also we worth noting in that function that the reason int $0x03
> and int $0x04 are dispatched as INT3 and INTO is to be fully user space
> compatible with legacy IDT, which behaves similarly.)

Also yes.

> Thus, the int $0x80 code is simply not reachable from kernel space; if
> kernel code were to invoke int $0x80 or any other INT instruction it will
> error out before getting to this code.

Yah, that explanation is a lot better, thanks. I'll use it. :)

> I suggested putting it into a comment for future reference. Obviously
> no need to duplicate it in the commit message :)

Yap, that makes more sense.

Thx.

-- 
Regards/Gruss,
    Boris.

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