| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <d0979bd9-fd12-4672-b451-23f23fc2353c@suse.com>
Date: Wed, 17 Apr 2024 14:02:33 +0300
From: Nikolay Borisov <nik.borisov@...e.com>
To: "Xin Li (Intel)" <xin@...or.com>, linux-kernel@...r.kernel.org
Cc: luto@...nel.org, tglx@...utronix.de, mingo@...hat.com, bp@...en8.de,
dave.hansen@...ux.intel.com, x86@...nel.org, hpa@...or.com
Subject: Re: [PATCH v3 1/1] x86/fred: Fix INT80 emulation for FRED
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@...or.com>
> Signed-off-by: Xin Li (Intel) <xin@...or.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
Powered by blists - more mailing lists