| 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: <5684e581-ea19-979e-579b-7259dfe3c1f3@linux.ibm.com>
Date: Thu, 17 Sep 2020 10:25:47 -0300
From: Rogerio Alves <rcardoso@...ux.ibm.com>
To: Ravi Bangoria <ravi.bangoria@...ux.ibm.com>, mpe@...erman.id.au,
christophe.leroy@....fr
Cc: mikey@...ling.org, jniethe5@...il.com, pedromfc@...ux.ibm.com,
linux-kernel@...r.kernel.org, paulus@...ba.org,
rogealve@...ux.ibm.com, naveen.n.rao@...ux.vnet.ibm.com,
linuxppc-dev@...ts.ozlabs.org
Subject: Re: [PATCH v6 4/8] powerpc/watchpoint: Move DAWR detection logic
outside of hw_breakpoint.c
On 9/2/20 1:29 AM, Ravi Bangoria wrote:
> Power10 hw has multiple DAWRs but hw doesn't tell which DAWR caused
> the exception. So we have a sw logic to detect that in hw_breakpoint.c.
> But hw_breakpoint.c gets compiled only with CONFIG_HAVE_HW_BREAKPOINT=Y.
> Move DAWR detection logic outside of hw_breakpoint.c so that it can be
> reused when CONFIG_HAVE_HW_BREAKPOINT is not set.
>
> Signed-off-by: Ravi Bangoria <ravi.bangoria@...ux.ibm.com>
Tested-by: Rogerio Alves <rcardoso@...ux.ibm.com>
> ---
> arch/powerpc/include/asm/hw_breakpoint.h | 8 +
> arch/powerpc/kernel/Makefile | 3 +-
> arch/powerpc/kernel/hw_breakpoint.c | 159 +----------------
> .../kernel/hw_breakpoint_constraints.c | 162 ++++++++++++++++++
> 4 files changed, 174 insertions(+), 158 deletions(-)
> create mode 100644 arch/powerpc/kernel/hw_breakpoint_constraints.c
>
> diff --git a/arch/powerpc/include/asm/hw_breakpoint.h b/arch/powerpc/include/asm/hw_breakpoint.h
> index 9b68eafebf43..81872c420476 100644
> --- a/arch/powerpc/include/asm/hw_breakpoint.h
> +++ b/arch/powerpc/include/asm/hw_breakpoint.h
> @@ -10,6 +10,7 @@
> #define _PPC_BOOK3S_64_HW_BREAKPOINT_H
>
> #include <asm/cpu_has_feature.h>
> +#include <asm/inst.h>
>
> #ifdef __KERNEL__
> struct arch_hw_breakpoint {
> @@ -52,6 +53,13 @@ static inline int nr_wp_slots(void)
> return cpu_has_feature(CPU_FTR_DAWR1) ? 2 : 1;
> }
>
> +bool wp_check_constraints(struct pt_regs *regs, struct ppc_inst instr,
> + unsigned long ea, int type, int size,
> + struct arch_hw_breakpoint *info);
> +
> +void wp_get_instr_detail(struct pt_regs *regs, struct ppc_inst *instr,
> + int *type, int *size, unsigned long *ea);
> +
> #ifdef CONFIG_HAVE_HW_BREAKPOINT
> #include <linux/kdebug.h>
> #include <asm/reg.h>
> diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
> index cbf41fb4ee89..a5550c2b24c4 100644
> --- a/arch/powerpc/kernel/Makefile
> +++ b/arch/powerpc/kernel/Makefile
> @@ -45,7 +45,8 @@ obj-y := cputable.o syscalls.o \
> signal.o sysfs.o cacheinfo.o time.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
> + of_platform.o prom_parse.o firmware.o \
> + hw_breakpoint_constraints.o
> obj-y += ptrace/
> obj-$(CONFIG_PPC64) += setup_64.o \
> paca.o nvram_64.o note.o syscall_64.o
> diff --git a/arch/powerpc/kernel/hw_breakpoint.c b/arch/powerpc/kernel/hw_breakpoint.c
> index f6b24838ca3c..f4e8f21046f5 100644
> --- a/arch/powerpc/kernel/hw_breakpoint.c
> +++ b/arch/powerpc/kernel/hw_breakpoint.c
> @@ -494,161 +494,6 @@ void thread_change_pc(struct task_struct *tsk, struct pt_regs *regs)
> }
> }
>
> -static bool dar_in_user_range(unsigned long dar, struct arch_hw_breakpoint *info)
> -{
> - return ((info->address <= dar) && (dar - info->address < info->len));
> -}
> -
> -static bool ea_user_range_overlaps(unsigned long ea, int size,
> - struct arch_hw_breakpoint *info)
> -{
> - return ((ea < info->address + info->len) &&
> - (ea + size > info->address));
> -}
> -
> -static bool dar_in_hw_range(unsigned long dar, struct arch_hw_breakpoint *info)
> -{
> - unsigned long hw_start_addr, hw_end_addr;
> -
> - hw_start_addr = ALIGN_DOWN(info->address, HW_BREAKPOINT_SIZE);
> - hw_end_addr = ALIGN(info->address + info->len, HW_BREAKPOINT_SIZE);
> -
> - return ((hw_start_addr <= dar) && (hw_end_addr > dar));
> -}
> -
> -static bool ea_hw_range_overlaps(unsigned long ea, int size,
> - struct arch_hw_breakpoint *info)
> -{
> - unsigned long hw_start_addr, hw_end_addr;
> - unsigned long align_size = HW_BREAKPOINT_SIZE;
> -
> - /*
> - * On p10 predecessors, quadword is handle differently then
> - * other instructions.
> - */
> - if (!cpu_has_feature(CPU_FTR_ARCH_31) && size == 16)
> - align_size = HW_BREAKPOINT_SIZE_QUADWORD;
> -
> - hw_start_addr = ALIGN_DOWN(info->address, align_size);
> - hw_end_addr = ALIGN(info->address + info->len, align_size);
> -
> - return ((ea < hw_end_addr) && (ea + size > hw_start_addr));
> -}
> -
> -/*
> - * If hw has multiple DAWR registers, we also need to check all
> - * dawrx constraint bits to confirm this is _really_ a valid event.
> - * If type is UNKNOWN, but privilege level matches, consider it as
> - * a positive match.
> - */
> -static bool check_dawrx_constraints(struct pt_regs *regs, int type,
> - struct arch_hw_breakpoint *info)
> -{
> - if (OP_IS_LOAD(type) && !(info->type & HW_BRK_TYPE_READ))
> - return false;
> -
> - /*
> - * The Cache Management instructions other than dcbz never
> - * cause a match. i.e. if type is CACHEOP, the instruction
> - * is dcbz, and dcbz is treated as Store.
> - */
> - if ((OP_IS_STORE(type) || type == CACHEOP) && !(info->type & HW_BRK_TYPE_WRITE))
> - return false;
> -
> - if (is_kernel_addr(regs->nip) && !(info->type & HW_BRK_TYPE_KERNEL))
> - return false;
> -
> - if (user_mode(regs) && !(info->type & HW_BRK_TYPE_USER))
> - return false;
> -
> - return true;
> -}
> -
> -/*
> - * Return true if the event is valid wrt dawr configuration,
> - * including extraneous exception. Otherwise return false.
> - */
> -static bool check_constraints(struct pt_regs *regs, struct ppc_inst instr,
> - unsigned long ea, int type, int size,
> - struct arch_hw_breakpoint *info)
> -{
> - bool in_user_range = dar_in_user_range(regs->dar, info);
> - bool dawrx_constraints;
> -
> - /*
> - * 8xx supports only one breakpoint and thus we can
> - * unconditionally return true.
> - */
> - if (IS_ENABLED(CONFIG_PPC_8xx)) {
> - if (!in_user_range)
> - info->type |= HW_BRK_TYPE_EXTRANEOUS_IRQ;
> - return true;
> - }
> -
> - if (unlikely(ppc_inst_equal(instr, ppc_inst(0)))) {
> - if (cpu_has_feature(CPU_FTR_ARCH_31) &&
> - !dar_in_hw_range(regs->dar, info))
> - return false;
> -
> - return true;
> - }
> -
> - dawrx_constraints = check_dawrx_constraints(regs, type, info);
> -
> - if (type == UNKNOWN) {
> - if (cpu_has_feature(CPU_FTR_ARCH_31) &&
> - !dar_in_hw_range(regs->dar, info))
> - return false;
> -
> - return dawrx_constraints;
> - }
> -
> - if (ea_user_range_overlaps(ea, size, info))
> - return dawrx_constraints;
> -
> - if (ea_hw_range_overlaps(ea, size, info)) {
> - if (dawrx_constraints) {
> - info->type |= HW_BRK_TYPE_EXTRANEOUS_IRQ;
> - return true;
> - }
> - }
> - return false;
> -}
> -
> -static int cache_op_size(void)
> -{
> -#ifdef __powerpc64__
> - return ppc64_caches.l1d.block_size;
> -#else
> - return L1_CACHE_BYTES;
> -#endif
> -}
> -
> -static void get_instr_detail(struct pt_regs *regs, struct ppc_inst *instr,
> - int *type, int *size, unsigned long *ea)
> -{
> - struct instruction_op op;
> -
> - if (__get_user_instr_inatomic(*instr, (void __user *)regs->nip))
> - return;
> -
> - analyse_instr(&op, regs, *instr);
> - *type = GETTYPE(op.type);
> - *ea = op.ea;
> -#ifdef __powerpc64__
> - if (!(regs->msr & MSR_64BIT))
> - *ea &= 0xffffffffUL;
> -#endif
> -
> - *size = GETSIZE(op.type);
> - if (*type == CACHEOP) {
> - *size = cache_op_size();
> - *ea &= ~(*size - 1);
> - } else if (*type == LOAD_VMX || *type == STORE_VMX) {
> - *ea &= ~(*size - 1);
> - }
> -}
> -
> static bool is_larx_stcx_instr(int type)
> {
> return type == LARX || type == STCX;
> @@ -732,7 +577,7 @@ int hw_breakpoint_handler(struct die_args *args)
> rcu_read_lock();
>
> if (!IS_ENABLED(CONFIG_PPC_8xx))
> - get_instr_detail(regs, &instr, &type, &size, &ea);
> + wp_get_instr_detail(regs, &instr, &type, &size, &ea);
>
> for (i = 0; i < nr_wp_slots(); i++) {
> bp[i] = __this_cpu_read(bp_per_reg[i]);
> @@ -742,7 +587,7 @@ int hw_breakpoint_handler(struct die_args *args)
> info[i] = counter_arch_bp(bp[i]);
> info[i]->type &= ~HW_BRK_TYPE_EXTRANEOUS_IRQ;
>
> - if (check_constraints(regs, instr, ea, type, size, info[i])) {
> + if (wp_check_constraints(regs, instr, ea, type, size, info[i])) {
> if (!IS_ENABLED(CONFIG_PPC_8xx) &&
> ppc_inst_equal(instr, ppc_inst(0))) {
> handler_error(bp[i], info[i]);
> diff --git a/arch/powerpc/kernel/hw_breakpoint_constraints.c b/arch/powerpc/kernel/hw_breakpoint_constraints.c
> new file mode 100644
> index 000000000000..867ee4aa026a
> --- /dev/null
> +++ b/arch/powerpc/kernel/hw_breakpoint_constraints.c
> @@ -0,0 +1,162 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +#include <linux/kernel.h>
> +#include <linux/uaccess.h>
> +#include <linux/sched.h>
> +#include <asm/hw_breakpoint.h>
> +#include <asm/sstep.h>
> +#include <asm/cache.h>
> +
> +static bool dar_in_user_range(unsigned long dar, struct arch_hw_breakpoint *info)
> +{
> + return ((info->address <= dar) && (dar - info->address < info->len));
> +}
> +
> +static bool ea_user_range_overlaps(unsigned long ea, int size,
> + struct arch_hw_breakpoint *info)
> +{
> + return ((ea < info->address + info->len) &&
> + (ea + size > info->address));
> +}
> +
> +static bool dar_in_hw_range(unsigned long dar, struct arch_hw_breakpoint *info)
> +{
> + unsigned long hw_start_addr, hw_end_addr;
> +
> + hw_start_addr = ALIGN_DOWN(info->address, HW_BREAKPOINT_SIZE);
> + hw_end_addr = ALIGN(info->address + info->len, HW_BREAKPOINT_SIZE);
> +
> + return ((hw_start_addr <= dar) && (hw_end_addr > dar));
> +}
> +
> +static bool ea_hw_range_overlaps(unsigned long ea, int size,
> + struct arch_hw_breakpoint *info)
> +{
> + unsigned long hw_start_addr, hw_end_addr;
> + unsigned long align_size = HW_BREAKPOINT_SIZE;
> +
> + /*
> + * On p10 predecessors, quadword is handle differently then
> + * other instructions.
> + */
> + if (!cpu_has_feature(CPU_FTR_ARCH_31) && size == 16)
> + align_size = HW_BREAKPOINT_SIZE_QUADWORD;
> +
> + hw_start_addr = ALIGN_DOWN(info->address, align_size);
> + hw_end_addr = ALIGN(info->address + info->len, align_size);
> +
> + return ((ea < hw_end_addr) && (ea + size > hw_start_addr));
> +}
> +
> +/*
> + * If hw has multiple DAWR registers, we also need to check all
> + * dawrx constraint bits to confirm this is _really_ a valid event.
> + * If type is UNKNOWN, but privilege level matches, consider it as
> + * a positive match.
> + */
> +static bool check_dawrx_constraints(struct pt_regs *regs, int type,
> + struct arch_hw_breakpoint *info)
> +{
> + if (OP_IS_LOAD(type) && !(info->type & HW_BRK_TYPE_READ))
> + return false;
> +
> + /*
> + * The Cache Management instructions other than dcbz never
> + * cause a match. i.e. if type is CACHEOP, the instruction
> + * is dcbz, and dcbz is treated as Store.
> + */
> + if ((OP_IS_STORE(type) || type == CACHEOP) && !(info->type & HW_BRK_TYPE_WRITE))
> + return false;
> +
> + if (is_kernel_addr(regs->nip) && !(info->type & HW_BRK_TYPE_KERNEL))
> + return false;
> +
> + if (user_mode(regs) && !(info->type & HW_BRK_TYPE_USER))
> + return false;
> +
> + return true;
> +}
> +
> +/*
> + * Return true if the event is valid wrt dawr configuration,
> + * including extraneous exception. Otherwise return false.
> + */
> +bool wp_check_constraints(struct pt_regs *regs, struct ppc_inst instr,
> + unsigned long ea, int type, int size,
> + struct arch_hw_breakpoint *info)
> +{
> + bool in_user_range = dar_in_user_range(regs->dar, info);
> + bool dawrx_constraints;
> +
> + /*
> + * 8xx supports only one breakpoint and thus we can
> + * unconditionally return true.
> + */
> + if (IS_ENABLED(CONFIG_PPC_8xx)) {
> + if (!in_user_range)
> + info->type |= HW_BRK_TYPE_EXTRANEOUS_IRQ;
> + return true;
> + }
> +
> + if (unlikely(ppc_inst_equal(instr, ppc_inst(0)))) {
> + if (cpu_has_feature(CPU_FTR_ARCH_31) &&
> + !dar_in_hw_range(regs->dar, info))
> + return false;
> +
> + return true;
> + }
> +
> + dawrx_constraints = check_dawrx_constraints(regs, type, info);
> +
> + if (type == UNKNOWN) {
> + if (cpu_has_feature(CPU_FTR_ARCH_31) &&
> + !dar_in_hw_range(regs->dar, info))
> + return false;
> +
> + return dawrx_constraints;
> + }
> +
> + if (ea_user_range_overlaps(ea, size, info))
> + return dawrx_constraints;
> +
> + if (ea_hw_range_overlaps(ea, size, info)) {
> + if (dawrx_constraints) {
> + info->type |= HW_BRK_TYPE_EXTRANEOUS_IRQ;
> + return true;
> + }
> + }
> + return false;
> +}
> +
> +static int cache_op_size(void)
> +{
> +#ifdef __powerpc64__
> + return ppc64_caches.l1d.block_size;
> +#else
> + return L1_CACHE_BYTES;
> +#endif
> +}
> +
> +void wp_get_instr_detail(struct pt_regs *regs, struct ppc_inst *instr,
> + int *type, int *size, unsigned long *ea)
> +{
> + struct instruction_op op;
> +
> + if (__get_user_instr_inatomic(*instr, (void __user *)regs->nip))
> + return;
> +
> + analyse_instr(&op, regs, *instr);
> + *type = GETTYPE(op.type);
> + *ea = op.ea;
> +#ifdef __powerpc64__
> + if (!(regs->msr & MSR_64BIT))
> + *ea &= 0xffffffffUL;
> +#endif
> +
> + *size = GETSIZE(op.type);
> + if (*type == CACHEOP) {
> + *size = cache_op_size();
> + *ea &= ~(*size - 1);
> + } else if (*type == LOAD_VMX || *type == STORE_VMX) {
> + *ea &= ~(*size - 1);
> + }
> +}
>
Powered by blists - more mailing lists