| 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: <1612833796.dl9doe6njg.astroid@bobo.none>
Date: Tue, 09 Feb 2021 11:27:13 +1000
From: Nicholas Piggin <npiggin@...il.com>
To: Benjamin Herrenschmidt <benh@...nel.crashing.org>,
Christophe Leroy <christophe.leroy@...roup.eu>,
Michael Ellerman <mpe@...erman.id.au>, msuchanek@...e.de,
Paul Mackerras <paulus@...ba.org>
Cc: linux-kernel@...r.kernel.org, linuxppc-dev@...ts.ozlabs.org
Subject: Re: [PATCH v5 09/22] powerpc/syscall: Make interrupt.c buildable on
PPC32
Excerpts from Christophe Leroy's message of February 9, 2021 1:10 am:
> To allow building interrupt.c on PPC32, ifdef out specific PPC64
> code or use helpers which are available on both PP32 and PPC64
>
> Modify Makefile to always build interrupt.o
>
> Signed-off-by: Christophe Leroy <christophe.leroy@...roup.eu>
> ---
> v5:
> - Also for interrupt exit preparation
> - Opted out kuap related code, ppc32 keeps it in ASM for the time being
> ---
> arch/powerpc/kernel/Makefile | 4 ++--
> arch/powerpc/kernel/interrupt.c | 31 ++++++++++++++++++++++++-------
> 2 files changed, 26 insertions(+), 9 deletions(-)
>
> diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
> index 26ff8c6e06b7..163755b1cef4 100644
> --- a/arch/powerpc/kernel/Makefile
> +++ b/arch/powerpc/kernel/Makefile
> @@ -57,10 +57,10 @@ obj-y := cputable.o syscalls.o \
> prom.o traps.o setup-common.o \
> udbg.o misc.o io.o misc_$(BITS).o \
> of_platform.o prom_parse.o firmware.o \
> - hw_breakpoint_constraints.o
> + hw_breakpoint_constraints.o interrupt.o
> obj-y += ptrace/
> obj-$(CONFIG_PPC64) += setup_64.o \
> - paca.o nvram_64.o note.o interrupt.o
> + paca.o nvram_64.o note.o
> obj-$(CONFIG_COMPAT) += sys_ppc32.o signal_32.o
> obj-$(CONFIG_VDSO32) += vdso32_wrapper.o
> obj-$(CONFIG_PPC_WATCHDOG) += watchdog.o
> diff --git a/arch/powerpc/kernel/interrupt.c b/arch/powerpc/kernel/interrupt.c
> index d6be4f9a67e5..2dac4d2bb1cf 100644
> --- a/arch/powerpc/kernel/interrupt.c
> +++ b/arch/powerpc/kernel/interrupt.c
> @@ -39,7 +39,7 @@ notrace long system_call_exception(long r3, long r4, long r5,
> BUG_ON(!(regs->msr & MSR_RI));
> BUG_ON(!(regs->msr & MSR_PR));
> BUG_ON(!FULL_REGS(regs));
> - BUG_ON(regs->softe != IRQS_ENABLED);
> + BUG_ON(arch_irq_disabled_regs(regs));
>
> #ifdef CONFIG_PPC_PKEY
> if (mmu_has_feature(MMU_FTR_PKEY)) {
> @@ -65,7 +65,9 @@ notrace long system_call_exception(long r3, long r4, long r5,
> isync();
> } else
> #endif
> +#ifdef CONFIG_PPC64
> kuap_check_amr();
> +#endif
Wouldn't mind trying to get rid of these ifdefs at some point, but
there's some kuap / keys changes going on recently so I'm happy enough
to let this settle then look at whether we can refactor.
>
> account_cpu_user_entry();
>
> @@ -77,7 +79,7 @@ notrace long system_call_exception(long r3, long r4, long r5,
> * frame, or if the unwinder was taught the first stack frame always
> * returns to user with IRQS_ENABLED, this store could be avoided!
> */
> - regs->softe = IRQS_ENABLED;
> + irq_soft_mask_regs_set_state(regs, IRQS_ENABLED);
>
> local_irq_enable();
>
> @@ -151,6 +153,7 @@ static notrace inline bool __prep_irq_for_enabled_exit(bool clear_ri)
> __hard_EE_RI_disable();
> else
> __hard_irq_disable();
> +#ifdef CONFIG_PPC64
> if (unlikely(lazy_irq_pending_nocheck())) {
> /* Took an interrupt, may have more exit work to do. */
> if (clear_ri)
> @@ -162,7 +165,7 @@ static notrace inline bool __prep_irq_for_enabled_exit(bool clear_ri)
> }
> local_paca->irq_happened = 0;
> irq_soft_mask_set(IRQS_ENABLED);
> -
> +#endif
> return true;
> }
>
Do we prefer space before return except in trivial cases?
> @@ -216,7 +219,9 @@ notrace unsigned long syscall_exit_prepare(unsigned long r3,
>
> CT_WARN_ON(ct_state() == CONTEXT_USER);
>
> +#ifdef CONFIG_PPC64
> kuap_check_amr();
> +#endif
>
> regs->result = r3;
>
> @@ -309,7 +314,7 @@ notrace unsigned long syscall_exit_prepare(unsigned long r3,
>
> account_cpu_user_exit();
>
> -#ifdef CONFIG_PPC_BOOK3S /* BOOK3E not yet using this */
> +#ifdef CONFIG_PPC_BOOK3S_64 /* BOOK3E and ppc32 not using this */
> /*
> * We do this at the end so that we do context switch with KERNEL AMR
> */
> @@ -318,7 +323,7 @@ notrace unsigned long syscall_exit_prepare(unsigned long r3,
> return ret;
> }
>
> -#ifdef CONFIG_PPC_BOOK3S /* BOOK3E not yet using this */
> +#ifndef CONFIG_PPC_BOOK3E_64 /* BOOK3E not yet using this */
> notrace unsigned long interrupt_exit_user_prepare(struct pt_regs *regs, unsigned long msr)
> {
> #ifdef CONFIG_PPC_BOOK3E
Why are you building this for 32? I don't mind if it's just to keep
things similar and make it build for now, but you're not using it yet,
right?
Reviewed-by: Nicholas Piggin <npiggin@...il.com>
> @@ -333,14 +338,16 @@ notrace unsigned long interrupt_exit_user_prepare(struct pt_regs *regs, unsigned
> BUG_ON(!(regs->msr & MSR_RI));
> BUG_ON(!(regs->msr & MSR_PR));
> BUG_ON(!FULL_REGS(regs));
> - BUG_ON(regs->softe != IRQS_ENABLED);
> + BUG_ON(arch_irq_disabled_regs(regs));
> CT_WARN_ON(ct_state() == CONTEXT_USER);
>
> /*
> * We don't need to restore AMR on the way back to userspace for KUAP.
> * AMR can only have been unlocked if we interrupted the kernel.
> */
> +#ifdef CONFIG_PPC64
> kuap_check_amr();
> +#endif
>
> local_irq_save(flags);
>
> @@ -407,7 +414,9 @@ notrace unsigned long interrupt_exit_user_prepare(struct pt_regs *regs, unsigned
> /*
> * We do this at the end so that we do context switch with KERNEL AMR
> */
> +#ifdef CONFIG_PPC64
> kuap_user_restore(regs);
> +#endif
> return ret;
> }
>
> @@ -419,7 +428,9 @@ notrace unsigned long interrupt_exit_kernel_prepare(struct pt_regs *regs, unsign
> unsigned long *ti_flagsp = ¤t_thread_info()->flags;
> unsigned long flags;
> unsigned long ret = 0;
> +#ifdef CONFIG_PPC64
> unsigned long amr;
> +#endif
>
> if (IS_ENABLED(CONFIG_PPC_BOOK3S) && unlikely(!(regs->msr & MSR_RI)))
> unrecoverable_exception(regs);
> @@ -432,7 +443,9 @@ notrace unsigned long interrupt_exit_kernel_prepare(struct pt_regs *regs, unsign
> if (TRAP(regs) != 0x700)
> CT_WARN_ON(ct_state() == CONTEXT_USER);
>
> +#ifdef CONFIG_PPC64
> amr = kuap_get_and_check_amr();
> +#endif
>
> if (unlikely(*ti_flagsp & _TIF_EMULATE_STACK_STORE)) {
> clear_bits(_TIF_EMULATE_STACK_STORE, ti_flagsp);
> @@ -441,7 +454,7 @@ notrace unsigned long interrupt_exit_kernel_prepare(struct pt_regs *regs, unsign
>
> local_irq_save(flags);
>
> - if (regs->softe == IRQS_ENABLED) {
> + if (!arch_irq_disabled_regs(regs)) {
> /* Returning to a kernel context with local irqs enabled. */
> WARN_ON_ONCE(!(regs->msr & MSR_EE));
> again:
> @@ -458,8 +471,10 @@ notrace unsigned long interrupt_exit_kernel_prepare(struct pt_regs *regs, unsign
> } else {
> /* Returning to a kernel context with local irqs disabled. */
> __hard_EE_RI_disable();
> +#ifdef CONFIG_PPC64
> if (regs->msr & MSR_EE)
> local_paca->irq_happened &= ~PACA_IRQ_HARD_DIS;
> +#endif
> }
>
>
> @@ -472,7 +487,9 @@ notrace unsigned long interrupt_exit_kernel_prepare(struct pt_regs *regs, unsign
> * which would cause Read-After-Write stalls. Hence, we take the AMR
> * value from the check above.
> */
> +#ifdef CONFIG_PPC64
> kuap_kernel_restore(regs, amr);
> +#endif
>
> return ret;
> }
> --
> 2.25.0
>
>
Powered by blists - more mailing lists