powerpc: Add ppc_inst_next()

In a few places we want to calculate the address of the next
instruction. Previously that was simple, we just added 4 bytes, or if
using a u32 * we incremented that pointer by 1.

But prefixed instructions make it more complicated, we need to advance
by either 4 or 8 bytes depending on the actual instruction. We also
can't do pointer arithmetic using struct ppc_inst, because it is
always 8 bytes in size on 64-bit, even though we might only need to
advance by 4 bytes.

So add a ppc_inst_next() helper which calculates the location of the
next instruction, if the given instruction was located at the given
address. Note the instruction doesn't need to actually be at the
address in memory.

Although it would seem natural for the value to be passed by value,
that makes it too easy to write a loop that will read off the end of a
page, eg:

	for (; src < end; src = ppc_inst_next(src, *src),
			  dest = ppc_inst_next(dest, *dest))

As noticed by Christophe and Jordan, if end is the exact end of a
page, and the next page is not mapped, this will fault, because *dest
will read 8 bytes, 4 bytes into the next page.

So value is passed by reference, so the helper can be careful to use
ppc_inst_read() on it.

Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Reviewed-by: Jordan Niethe <jniethe5@gmail.com>
Link: https://lore.kernel.org/r/20200522133318.1681406-1-mpe@ellerman.id.au

diff --git a/arch/powerpc/include/asm/inst.h b/arch/powerpc/include/asm/inst.h
index d82e0c99cfa10ef1202fe6e19ff978303cd40fef..5b756ba77ed2ef12f5a0b9bb0c785c92444fbfed 100644 (file)
--- a/arch/powerpc/include/asm/inst.h
+++ b/arch/powerpc/include/asm/inst.h
@@ -100,6 +100,19 @@ static inline int ppc_inst_len(struct ppc_inst x)
        return ppc_inst_prefixed(x) ? 8 : 4;
 }
 
+/*
+ * Return the address of the next instruction, if the instruction @value was
+ * located at @location.
+ */
+static inline struct ppc_inst *ppc_inst_next(void *location, struct ppc_inst *value)
+{
+       struct ppc_inst tmp;
+
+       tmp = ppc_inst_read(value);
+
+       return location + ppc_inst_len(tmp);
+}
+
 int probe_user_read_inst(struct ppc_inst *inst,
                         struct ppc_inst __user *nip);
 

diff --git a/arch/powerpc/kernel/uprobes.c b/arch/powerpc/kernel/uprobes.c
index 83e883e1a42db2b0059983a0baeaf46875f610a6..d200e7df71678926e3b5eb4df33e23cac61ac5da 100644 (file)
--- a/arch/powerpc/kernel/uprobes.c
+++ b/arch/powerpc/kernel/uprobes.c
@@ -112,7 +112,7 @@ int arch_uprobe_post_xol(struct arch_uprobe *auprobe, struct pt_regs *regs)
         * support doesn't exist and have to fix-up the next instruction
         * to be executed.
         */
-       regs->nip = utask->vaddr + ppc_inst_len(ppc_inst_read(&auprobe->insn));
+       regs->nip = (unsigned long)ppc_inst_next((void *)utask->vaddr, &auprobe->insn);
 
        user_disable_single_step(current);
        return 0;

diff --git a/arch/powerpc/lib/feature-fixups.c b/arch/powerpc/lib/feature-fixups.c
index 80f320c2e189224645818caab8e6495ef6bec840..4c0a7ee9fa000c0be78b629633eece27abc8ebbc 100644 (file)
--- a/arch/powerpc/lib/feature-fixups.c
+++ b/arch/powerpc/lib/feature-fixups.c
@@ -68,7 +68,7 @@ static int patch_alt_instruction(struct ppc_inst *src, struct ppc_inst *dest,
 
 static int patch_feature_section(unsigned long value, struct fixup_entry *fcur)
 {
-       struct ppc_inst *start, *end, *alt_start, *alt_end, *src, *dest;
+       struct ppc_inst *start, *end, *alt_start, *alt_end, *src, *dest, nop;
 
        start = calc_addr(fcur, fcur->start_off);
        end = calc_addr(fcur, fcur->end_off);
@@ -84,14 +84,15 @@ static int patch_feature_section(unsigned long value, struct fixup_entry *fcur)
        src = alt_start;
        dest = start;
 
-       for (; src < alt_end; src = (void *)src + ppc_inst_len(ppc_inst_read(src)),
-            (dest = (void *)dest + ppc_inst_len(ppc_inst_read(dest)))) {
+       for (; src < alt_end; src = ppc_inst_next(src, src),
+                             dest = ppc_inst_next(dest, dest)) {
                if (patch_alt_instruction(src, dest, alt_start, alt_end))
                        return 1;
        }
 
-       for (; dest < end; dest = (void *)dest + ppc_inst_len(ppc_inst(PPC_INST_NOP)))
-               raw_patch_instruction(dest, ppc_inst(PPC_INST_NOP));
+       nop = ppc_inst(PPC_INST_NOP);
+       for (; dest < end; dest = ppc_inst_next(dest, &nop))
+               raw_patch_instruction(dest, nop);
 
        return 0;
 }
@@ -405,8 +406,8 @@ static void do_final_fixups(void)
        while (src < end) {
                inst = ppc_inst_read(src);
                raw_patch_instruction(dest, inst);
-               src = (void *)src + ppc_inst_len(inst);
-               dest = (void *)dest + ppc_inst_len(inst);
+               src = ppc_inst_next(src, src);
+               dest = ppc_inst_next(dest, dest);
        }
 #endif
 }

diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index de585204d1d2632307524ef21155e6eee44b293e..16ee6639a60cb018fb59c2c866725d16bb0858d7 100644 (file)
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -939,7 +939,7 @@ static void insert_bpts(void)
                }
 
                patch_instruction(bp->instr, instr);
-               patch_instruction((void *)bp->instr + ppc_inst_len(instr),
+               patch_instruction(ppc_inst_next(bp->instr, &instr),
                                  ppc_inst(bpinstr));
                if (bp->enabled & BP_CIABR)
                        continue;