Re: [PATCH v5 10/21] powerpc: Use a function for reading instructions
On Mon, 2020-04-06 at 18:09 +1000, Jordan Niethe wrote:
> Prefixed instructions will mean there are instructions of different
> length. As a result dereferencing a pointer to an instruction will not
> necessarily give the desired result. Introduce a function for reading
> instructions from memory into the instruction data type.
>
> Signed-off-by: Jordan Niethe
> ---
> v4: New to series
> v5: - Rename read_inst() -> probe_kernel_read_inst()
> - No longer modify uprobe probe type in this patch
> ---
> arch/powerpc/include/asm/inst.h| 5 +
> arch/powerpc/kernel/kprobes.c | 11 --
> arch/powerpc/kernel/mce_power.c| 2 +-
> arch/powerpc/kernel/optprobes.c| 4 ++--
> arch/powerpc/kernel/trace/ftrace.c | 33 +++---
> arch/powerpc/lib/code-patching.c | 23 ++---
> arch/powerpc/lib/feature-fixups.c | 2 +-
> arch/powerpc/xmon/xmon.c | 6 +++---
> 8 files changed, 48 insertions(+), 38 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/inst.h
> b/arch/powerpc/include/asm/inst.h
> index a71decf5f871..369b35ce964c 100644
> --- a/arch/powerpc/include/asm/inst.h
> +++ b/arch/powerpc/include/asm/inst.h
> @@ -27,6 +27,11 @@ static inline struct ppc_inst ppc_inst_swab(struct
> ppc_inst x)
> return ppc_inst(swab32(ppc_inst_val(x)));
> }
>
> +static inline struct ppc_inst ppc_inst_read(const struct ppc_inst *ptr)
> +{
> + return *ptr;
> +}
> +
> static inline bool ppc_inst_equal(struct ppc_inst x, struct ppc_inst y)
> {
> return !memcmp(&x, &y, sizeof(struct ppc_inst));
> diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c
> index 9ed996cb0589..ff53e5ef7e40 100644
> --- a/arch/powerpc/kernel/kprobes.c
> +++ b/arch/powerpc/kernel/kprobes.c
> @@ -106,7 +106,7 @@ kprobe_opcode_t *kprobe_lookup_name(const char *name,
> unsigned int offset)
> int arch_prepare_kprobe(struct kprobe *p)
> {
> int ret = 0;
> - struct ppc_inst insn = *(struct ppc_inst *)p->addr;
> + struct ppc_inst insn = ppc_inst_read((struct ppc_inst *)p->addr);
>
> if ((unsigned long)p->addr & 0x03) {
> printk("Attempt to register kprobe at an unaligned address\n");
> @@ -125,11 +125,8 @@ int arch_prepare_kprobe(struct kprobe *p)
> }
>
> if (!ret) {
> - memcpy(p->ainsn.insn, p->addr,
> - MAX_INSN_SIZE * sizeof(kprobe_opcode_t));
> - p->opcode = *p->addr;
> - flush_icache_range((unsigned long)p->ainsn.insn,
> - (unsigned long)p->ainsn.insn +
> sizeof(kprobe_opcode_t));
> + patch_instruction((struct ppc_inst *)p->ainsn.insn, insn);
> + p->opcode = ppc_inst_val(insn);
This is a different change from this commit
-- Bala
> }
>
> p->ainsn.boostable = 0;
> @@ -217,7 +214,7 @@ NOKPROBE_SYMBOL(arch_prepare_kretprobe);
> static int try_to_emulate(struct kprobe *p, struct pt_regs *regs)
> {
> int ret;
> - struct ppc_inst insn = *(struct ppc_inst *)p->ainsn.insn;
> + struct ppc_inst insn = ppc_inst_read((struct ppc_inst *)p->ainsn.insn);
>
> /* regs->nip is also adjusted if emulate_step returns 1 */
> ret = emulate_step(regs, insn);
> diff --git a/arch/powerpc/kernel/mce_power.c
> b/arch/powerpc/kernel/mce_power.c
> index 7118b46a6543..859b602fa270 100644
> --- a/arch/powerpc/kernel/mce_power.c
> +++ b/arch/powerpc/kernel/mce_power.c
> @@ -374,7 +374,7 @@ static int mce_find_instr_ea_and_phys(struct pt_regs
> *regs, uint64_t *addr,
> pfn = addr_to_pfn(regs, regs->nip);
> if (pfn != ULONG_MAX) {
> instr_addr = (pfn << PAGE_SHIFT) + (regs->nip & ~PAGE_MASK);
> - instr = *(struct ppc_inst *)(instr_addr);
> + instr = ppc_inst_read((struct ppc_inst *)instr_addr);
> if (!analyse_instr(&op, &tmp, instr)) {
> pfn = addr_to_pfn(regs, op.ea);
> *addr = op.ea;
> diff --git a/arch/powerpc/kernel/optprobes.c
> b/arch/powerpc/kernel/optprobes.c
> index b61bbcee84f4..684640b8fa2e 100644
> --- a/arch/powerpc/kernel/optprobes.c
> +++ b/arch/powerpc/kernel/optprobes.c
> @@ -100,8 +100,8 @@ static unsigned long can_optimize(struct kprobe *p)
>* Ensure that the instruction is not a conditional branch,
>* and that can be emulated.
>*/
> - if (!is_conditional_branch(*(struct ppc_inst *)p->ainsn.insn) &&
> - analyse_instr(&op, ®s, *(struct ppc_inst *)p-
> >ainsn.insn) == 1) {
> + if (!is_conditional_branch(ppc_inst_read((struct ppc_inst *)p-
> >ainsn.insn)) &&
> + analyse_instr(&op, ®s, ppc_inst_read((struct
> ppc_inst *)p->ainsn.insn)) == 1) {
> emulate_update_regs(®s, &op);
> nip = regs.nip;
> }
> diff --git a/arch/powerpc/kernel/trace/ftrace.c
> b/arch/powerpc/kernel/trace/ftrace.c
> index 442c62fb68ff..e78742613b36 100644
> --- a/arch/powerpc/
[PATCH v5 10/21] powerpc: Use a function for reading instructions
Prefixed instructions will mean there are instructions of different
length. As a result dereferencing a pointer to an instruction will not
necessarily give the desired result. Introduce a function for reading
instructions from memory into the instruction data type.
Signed-off-by: Jordan Niethe
---
v4: New to series
v5: - Rename read_inst() -> probe_kernel_read_inst()
- No longer modify uprobe probe type in this patch
---
arch/powerpc/include/asm/inst.h| 5 +
arch/powerpc/kernel/kprobes.c | 11 --
arch/powerpc/kernel/mce_power.c| 2 +-
arch/powerpc/kernel/optprobes.c| 4 ++--
arch/powerpc/kernel/trace/ftrace.c | 33 +++---
arch/powerpc/lib/code-patching.c | 23 ++---
arch/powerpc/lib/feature-fixups.c | 2 +-
arch/powerpc/xmon/xmon.c | 6 +++---
8 files changed, 48 insertions(+), 38 deletions(-)
diff --git a/arch/powerpc/include/asm/inst.h b/arch/powerpc/include/asm/inst.h
index a71decf5f871..369b35ce964c 100644
--- a/arch/powerpc/include/asm/inst.h
+++ b/arch/powerpc/include/asm/inst.h
@@ -27,6 +27,11 @@ static inline struct ppc_inst ppc_inst_swab(struct ppc_inst
x)
return ppc_inst(swab32(ppc_inst_val(x)));
}
+static inline struct ppc_inst ppc_inst_read(const struct ppc_inst *ptr)
+{
+ return *ptr;
+}
+
static inline bool ppc_inst_equal(struct ppc_inst x, struct ppc_inst y)
{
return !memcmp(&x, &y, sizeof(struct ppc_inst));
diff --git a/arch/powerpc/kernel/kprobes.c b/arch/powerpc/kernel/kprobes.c
index 9ed996cb0589..ff53e5ef7e40 100644
--- a/arch/powerpc/kernel/kprobes.c
+++ b/arch/powerpc/kernel/kprobes.c
@@ -106,7 +106,7 @@ kprobe_opcode_t *kprobe_lookup_name(const char *name,
unsigned int offset)
int arch_prepare_kprobe(struct kprobe *p)
{
int ret = 0;
- struct ppc_inst insn = *(struct ppc_inst *)p->addr;
+ struct ppc_inst insn = ppc_inst_read((struct ppc_inst *)p->addr);
if ((unsigned long)p->addr & 0x03) {
printk("Attempt to register kprobe at an unaligned address\n");
@@ -125,11 +125,8 @@ int arch_prepare_kprobe(struct kprobe *p)
}
if (!ret) {
- memcpy(p->ainsn.insn, p->addr,
- MAX_INSN_SIZE * sizeof(kprobe_opcode_t));
- p->opcode = *p->addr;
- flush_icache_range((unsigned long)p->ainsn.insn,
- (unsigned long)p->ainsn.insn + sizeof(kprobe_opcode_t));
+ patch_instruction((struct ppc_inst *)p->ainsn.insn, insn);
+ p->opcode = ppc_inst_val(insn);
}
p->ainsn.boostable = 0;
@@ -217,7 +214,7 @@ NOKPROBE_SYMBOL(arch_prepare_kretprobe);
static int try_to_emulate(struct kprobe *p, struct pt_regs *regs)
{
int ret;
- struct ppc_inst insn = *(struct ppc_inst *)p->ainsn.insn;
+ struct ppc_inst insn = ppc_inst_read((struct ppc_inst *)p->ainsn.insn);
/* regs->nip is also adjusted if emulate_step returns 1 */
ret = emulate_step(regs, insn);
diff --git a/arch/powerpc/kernel/mce_power.c b/arch/powerpc/kernel/mce_power.c
index 7118b46a6543..859b602fa270 100644
--- a/arch/powerpc/kernel/mce_power.c
+++ b/arch/powerpc/kernel/mce_power.c
@@ -374,7 +374,7 @@ static int mce_find_instr_ea_and_phys(struct pt_regs *regs,
uint64_t *addr,
pfn = addr_to_pfn(regs, regs->nip);
if (pfn != ULONG_MAX) {
instr_addr = (pfn << PAGE_SHIFT) + (regs->nip & ~PAGE_MASK);
- instr = *(struct ppc_inst *)(instr_addr);
+ instr = ppc_inst_read((struct ppc_inst *)instr_addr);
if (!analyse_instr(&op, &tmp, instr)) {
pfn = addr_to_pfn(regs, op.ea);
*addr = op.ea;
diff --git a/arch/powerpc/kernel/optprobes.c b/arch/powerpc/kernel/optprobes.c
index b61bbcee84f4..684640b8fa2e 100644
--- a/arch/powerpc/kernel/optprobes.c
+++ b/arch/powerpc/kernel/optprobes.c
@@ -100,8 +100,8 @@ static unsigned long can_optimize(struct kprobe *p)
* Ensure that the instruction is not a conditional branch,
* and that can be emulated.
*/
- if (!is_conditional_branch(*(struct ppc_inst *)p->ainsn.insn) &&
- analyse_instr(&op, ®s, *(struct ppc_inst
*)p->ainsn.insn) == 1) {
+ if (!is_conditional_branch(ppc_inst_read((struct ppc_inst
*)p->ainsn.insn)) &&
+ analyse_instr(&op, ®s, ppc_inst_read((struct
ppc_inst *)p->ainsn.insn)) == 1) {
emulate_update_regs(®s, &op);
nip = regs.nip;
}
diff --git a/arch/powerpc/kernel/trace/ftrace.c
b/arch/powerpc/kernel/trace/ftrace.c
index 442c62fb68ff..e78742613b36 100644
--- a/arch/powerpc/kernel/trace/ftrace.c
+++ b/arch/powerpc/kernel/trace/ftrace.c
@@ -41,6 +41,12 @@
#defineNUM_FTRACE_TRAMPS 8
static unsigned long ftrace_tramps[NUM_FTRACE_TRAMPS];
+static long
+probe_kernel_read_inst(struct ppc_inst *inst, const void
