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

[Jordan Niethe]

On Wed, May 19, 2021 at 6:11 PM Naveen N. Rao
<naveen.n.rao at linux.vnet.ibm.com> wrote:
>
> Christophe Leroy wrote:
> >
> >
> > 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 ?
>
> Good catch!
> I think we can just skip validation if the instruction is at the
> beginning of a page. I have a few cleanups in this area - I will post a
> patchset soon.
Yeah thanks Christophe for noticing that. And thanks Naveen that
sounds like it should fix it.
>
> >
> >>
> >>      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 ?
>
> Yes. Per the ISA:
>   Bits 0:5 of all prefixes are assigned the primary opcode
>   value 0b000001. 0b000001 is not available for use as a
>   primary opcode for either word instructions or suffixes
>   of prefixed instructions.
Yep, a prefix will never be a valid word instruction or suffix.
>
>
> - Naveen
>