[PATCH v8 27/30] powerpc/kprobes: Don't allow breakpoints on suffixes

[Christophe Leroy]

Le 06/05/2020 à 05:40, Jordan Niethe a écrit :
> Do not allow inserting breakpoints on the suffix of a prefix instruction
> in kprobes.
> 
> Signed-off-by: Jordan Niethe <jniethe5 at gmail.com>
> ---
> v8: Add this back from v3
> ---
>   arch/powerpc/kernel/kprobes.c | 13 +++++++++++++
>   1 file changed, 13 insertions(+)
> 
> diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c
> index 33d54b091c70..227510df8c55 100644
> --- a/arch/powerpc/kernel/kprobes.c
> +++ b/arch/powerpc/kernel/kprobes.c
> @@ -106,7 +106,9 @@ kprobe_opcode_t *kprobe_lookup_name(const char *name, unsigned int offset)
>   int arch_prepare_kprobe(struct kprobe *p)
>   {
>   	int ret = 0;
> +	struct kprobe *prev;
>   	struct ppc_inst insn = ppc_inst_read((struct ppc_inst *)p->addr);
> +	struct ppc_inst prefix = ppc_inst_read((struct ppc_inst *)(p->addr - 1));

What if p->addr is the first word of a page and the previous page is not mapped ?

>   
>   	if ((unsigned long)p->addr & 0x03) {
>   		printk("Attempt to register kprobe at an unaligned address\n");
> @@ -114,6 +116,17 @@ int arch_prepare_kprobe(struct kprobe *p)
>   	} else if (IS_MTMSRD(insn) || IS_RFID(insn) || IS_RFI(insn)) {
>   		printk("Cannot register a kprobe on rfi/rfid or mtmsr[d]\n");
>   		ret = -EINVAL;
> +	} else if (ppc_inst_prefixed(prefix)) {

If p->addr - 2 contains a valid prefixed instruction, then p->addr - 1 contains the suffix of that 
prefixed instruction. Are we sure a suffix can never ever be misinterpreted as the prefix of a 
prefixed instruction ?


> +		printk("Cannot register a kprobe on the second word of prefixed instruction\n");
> +		ret = -EINVAL;
> +	}
> +	preempt_disable();
> +	prev = get_kprobe(p->addr - 1);
> +	preempt_enable_no_resched();
> +	if (prev &&
> +	    ppc_inst_prefixed(ppc_inst_read((struct ppc_inst *)prev->ainsn.insn))) {
> +		printk("Cannot register a kprobe on the second word of prefixed instruction\n");
> +		ret = -EINVAL;
>   	}
>   
>   	/* insn must be on a special executable page on ppc64.  This is
>