Messages in this thread | | | Date | Wed, 17 Apr 2024 14:02:33 +0300 | Subject | Re: [PATCH v3 1/1] x86/fred: Fix INT80 emulation for FRED | From | 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
| |