[PATCH 0/3] powerpc: Use proper printk format specifiers

[PATCH 0/3] powerpc: Use proper printk format specifiers

[Naveen N. Rao]

This series changes printk format specifiers from bare %p to %px/%pK in 
ftrace, kprobes and hw bkpts code. In addition, use of %lx is also 
changed over to conform to the recommended practice.

This series applies on top of the below patch series:
https://lore.kernel.org/r/cover.1587654213.git.naveen.n.rao@linux.vnet.ibm.com

- Naveen

Balamuruhan S (1):
  powerpc/kprobes: Use appropriate format specifier for printing kernel
    address

Naveen N. Rao (2):
  powerpc/ftrace: Use appropriate format specifier for printing kernel
    addresses
  powerpc/hw_bkpt: Update printk format specifiers for kernel addresses

 arch/powerpc/kernel/hw_breakpoint.c |  5 +-
 arch/powerpc/kernel/optprobes.c     |  2 +-
 arch/powerpc/kernel/trace/ftrace.c  | 74 ++++++++++++++++-------------
 3 files changed, 45 insertions(+), 36 deletions(-)


base-commit: 8299da600ad05b8aa0f15ec0f5f03bd40e37d6f0
-- 2.25.1

[PATCH 1/3] powerpc/kprobes: Use appropriate format specifier for printing kernel address

[Naveen N. Rao]

From: Balamuruhan S <bala24@linux.ibm.com>

Change use of %p to %pK when printing address of the instruction slot so
that the actual kernel address is visible for privileged users.

Signed-off-by: Balamuruhan S <bala24@linux.ibm.com>
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/optprobes.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/optprobes.c b/arch/powerpc/kernel/optprobes.c
index ef0924b0809d..d5f8c25b7cac 100644
--- a/arch/powerpc/kernel/optprobes.c
+++ b/arch/powerpc/kernel/optprobes.c
@@ -247,7 +247,7 @@ int arch_prepare_optimized_kprobe(struct optimized_kprobe *op, struct kprobe *p)
 	/* Setup template */
 	/* We can optimize this via patch_instruction_window later */
 	size = (TMPL_END_IDX * sizeof(kprobe_opcode_t)) / sizeof(int);
-	pr_devel("Copying template to %p, size %lu\n", buff, size);
+	pr_devel("Copying template to %pK, size %lu\n", (void *)buff, size);
 	for (i = 0; i < size; i++) {
 		rc = patch_instruction(buff + i, *(optprobe_template_entry + i));
 		if (rc) {
-- 
2.25.1

[PATCH 2/3] powerpc/ftrace: Use appropriate format specifier for printing kernel addresses

[Naveen N. Rao]

Update use of printk format specifiers in ftrace code, so that addresses
are made visible for privileged users, or always for pr_devel() code:
- change %lx to use %px or %pK
- change %p to %px or %pK
- add %pS in certain places to show the symbol as well

Signed-off-by: Balamuruhan S <bala24@linux.ibm.com>
Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/trace/ftrace.c | 74 +++++++++++++++++-------------
 1 file changed, 41 insertions(+), 33 deletions(-)

diff --git a/arch/powerpc/kernel/trace/ftrace.c b/arch/powerpc/kernel/trace/ftrace.c
index 679d5249b002..29b77204f46d 100644
--- a/arch/powerpc/kernel/trace/ftrace.c
+++ b/arch/powerpc/kernel/trace/ftrace.c
@@ -83,8 +83,8 @@ ftrace_modify_code(unsigned long ip, unsigned int old, unsigned int new)
 
 	/* Make sure it is what we expect it to be */
 	if (replaced != old) {
-		pr_err("%p: replaced (%#x) != old (%#x)",
-		(void *)ip, replaced, old);
+		pr_err("%pK (%pS): replaced (%#x) != old (%#x)",
+		       (void *)ip, (void *)ip, replaced, old);
 		return -EINVAL;
 	}
 
@@ -152,19 +152,20 @@ __ftrace_make_nop(struct module *mod,
 	/* lets find where the pointer goes */
 	tramp = find_bl_target(ip, op);
 
-	pr_devel("ip:%lx jumps to %lx", ip, tramp);
+	pr_devel("ip:0x%px jumps to 0x%px", (void *)ip, (void *)tramp);
 
 	if (module_trampoline_target(mod, tramp, &ptr)) {
 		pr_err("Failed to get trampoline target\n");
 		return -EFAULT;
 	}
 
-	pr_devel("trampoline target %lx", ptr);
+	pr_devel("trampoline target 0x%px", (void *)ptr);
 
 	entry = ppc_global_function_entry((void *)addr);
 	/* This should match what was called */
 	if (ptr != entry) {
-		pr_err("addr %lx does not match expected %lx\n", ptr, entry);
+		pr_err("addr 0x%pK does not match expected 0x%pK\n",
+				(void *)ptr, (void *)entry);
 		return -EINVAL;
 	}
 
@@ -173,7 +174,8 @@ __ftrace_make_nop(struct module *mod,
 	pop = PPC_INST_NOP;
 
 	if (probe_kernel_read(&op, (void *)(ip - 4), 4)) {
-		pr_err("Fetching instruction at %lx failed.\n", ip - 4);
+		pr_err("Fetching instruction at 0x%pK (%pS) failed.\n",
+				(void *)(ip - 4), (void *)(ip - 4));
 		return -EFAULT;
 	}
 
@@ -249,11 +251,11 @@ __ftrace_make_nop(struct module *mod,
 	 *  0x4e, 0x80, 0x04, 0x20  bctr
 	 */
 
-	pr_devel("ip:%lx jumps to %lx", ip, tramp);
+	pr_devel("ip:0x%px jumps to 0x%px", (void *)ip, (void *)tramp);
 
 	/* Find where the trampoline jumps to */
 	if (probe_kernel_read(jmp, (void *)tramp, sizeof(jmp))) {
-		pr_err("Failed to read %lx\n", tramp);
+		pr_err("Failed to read 0x%pK\n", (void *)tramp);
 		return -EFAULT;
 	}
 
@@ -273,11 +275,11 @@ __ftrace_make_nop(struct module *mod,
 	if (tramp & 0x8000)
 		tramp -= 0x10000;
 
-	pr_devel(" %lx ", tramp);
+	pr_devel(" 0x%px ", (void *)tramp);
 
 	if (tramp != addr) {
-		pr_err("Trampoline location %08lx does not match addr\n",
-		       tramp);
+		pr_err("Trampoline location 0x%pK does not match addr\n",
+		       (void *)tramp);
 		return -EINVAL;
 	}
 
@@ -362,7 +364,8 @@ static int setup_mcount_compiler_tramp(unsigned long tramp)
 	ptr = find_bl_target(tramp, op);
 
 	if (ptr != ppc_global_function_entry((void *)_mcount)) {
-		pr_debug("Trampoline target %p is not _mcount\n", (void *)ptr);
+		pr_debug("Trampoline target 0x%px (%pS) is not _mcount\n",
+				(void *)ptr, (void *)ptr);
 		return -1;
 	}
 
@@ -374,8 +377,8 @@ static int setup_mcount_compiler_tramp(unsigned long tramp)
 #endif
 	op = create_branch((void *)tramp, ptr, 0);
 	if (!op) {
-		pr_debug("%ps is not reachable from existing mcount tramp\n",
-				(void *)ptr);
+		pr_debug("0x%px (%ps) is not reachable from existing mcount tramp\n",
+				(void *)ptr, (void *)ptr);
 		return -1;
 	}
 
@@ -409,13 +412,13 @@ static int __ftrace_make_nop_kernel(struct dyn_ftrace *rec, unsigned long addr)
 	/* Let's find where the pointer goes */
 	tramp = find_bl_target(ip, op);
 
-	pr_devel("ip:%lx jumps to %lx", ip, tramp);
+	pr_devel("ip:0x%px jumps to 0x%px", (void *)ip, (void *)tramp);
 
 	if (setup_mcount_compiler_tramp(tramp)) {
 		/* Are other trampolines reachable? */
 		if (!find_ftrace_tramp(ip)) {
-			pr_err("No ftrace trampolines reachable from %ps\n",
-					(void *)ip);
+			pr_err("No ftrace trampolines reachable from 0x%pK (%pS)\n",
+					(void *)ip, (void *)ip);
 			return -EINVAL;
 		}
 	}
@@ -452,13 +455,13 @@ int ftrace_make_nop(struct module *mod,
 	 */
 	if (!rec->arch.mod) {
 		if (!mod) {
-			pr_err("No module loaded addr=%lx\n", addr);
+			pr_err("No module loaded addr=0x%pK\n", (void *)addr);
 			return -EFAULT;
 		}
 		rec->arch.mod = mod;
 	} else if (mod) {
 		if (mod != rec->arch.mod) {
-			pr_err("Record mod %p not equal to passed in mod %p\n",
+			pr_err("Record mod %pK not equal to passed in mod %pK\n",
 			       rec->arch.mod, mod);
 			return -EINVAL;
 		}
@@ -521,8 +524,8 @@ __ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
 		return -EFAULT;
 
 	if (!expected_nop_sequence(ip, op[0], op[1])) {
-		pr_err("Unexpected call sequence at %p: %x %x\n",
-		ip, op[0], op[1]);
+		pr_err("Unexpected call sequence at %pK: %x %x\n",
+			ip, op[0], op[1]);
 		return -EINVAL;
 	}
 
@@ -548,12 +551,13 @@ __ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
 		return -EFAULT;
 	}
 
-	pr_devel("trampoline target %lx", ptr);
+	pr_devel("trampoline target 0x%px", (void *)ptr);
 
 	entry = ppc_global_function_entry((void *)addr);
 	/* This should match what was called */
 	if (ptr != entry) {
-		pr_err("addr %lx does not match expected %lx\n", ptr, entry);
+		pr_err("addr 0x%pK does not match expected 0x%pK\n",
+				(void *)ptr, (void *)entry);
 		return -EINVAL;
 	}
 
@@ -600,7 +604,7 @@ __ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
 		return -EINVAL;
 	}
 
-	pr_devel("write to %lx\n", rec->ip);
+	pr_devel("write to 0x%px\n", (void *)rec->ip);
 
 	PATCH_INSN(ip, op);
 
@@ -624,7 +628,8 @@ static int __ftrace_make_call_kernel(struct dyn_ftrace *rec, unsigned long addr)
 		entry = ppc_global_function_entry((void *)ftrace_regs_caller);
 		if (ptr != entry) {
 #endif
-			pr_err("Unknown ftrace addr to patch: %ps\n", (void *)ptr);
+			pr_err("Unknown ftrace addr to patch: 0x%pK (%pS)\n",
+					(void *)ptr, (void *)ptr);
 			return -EINVAL;
 #ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
 		}
@@ -633,18 +638,19 @@ static int __ftrace_make_call_kernel(struct dyn_ftrace *rec, unsigned long addr)
 
 	/* Make sure we have a nop */
 	if (probe_kernel_read(&op, ip, sizeof(op))) {
-		pr_err("Unable to read ftrace location %p\n", ip);
+		pr_err("Unable to read ftrace location %pK\n", ip);
 		return -EFAULT;
 	}
 
 	if (op != PPC_INST_NOP) {
-		pr_err("Unexpected call sequence at %p: %x\n", ip, op);
+		pr_err("Unexpected call sequence at %pK: %x\n", ip, op);
 		return -EINVAL;
 	}
 
 	tramp = find_ftrace_tramp((unsigned long)ip);
 	if (!tramp) {
-		pr_err("No ftrace trampolines reachable from %ps\n", ip);
+		pr_err("No ftrace trampolines reachable from 0x%pK (%pS)\n",
+				ip, ip);
 		return -EINVAL;
 	}
 
@@ -728,7 +734,7 @@ __ftrace_modify_call(struct dyn_ftrace *rec, unsigned long old_addr,
 	tramp = find_bl_target(ip, op);
 	entry = ppc_global_function_entry((void *)old_addr);
 
-	pr_devel("ip:%lx jumps to %lx", ip, tramp);
+	pr_devel("ip:0x%px jumps to 0x%px", (void *)ip, (void *)tramp);
 
 	if (tramp != entry) {
 		/* old_addr is not within range, so we must have used a trampoline */
@@ -737,11 +743,12 @@ __ftrace_modify_call(struct dyn_ftrace *rec, unsigned long old_addr,
 			return -EFAULT;
 		}
 
-		pr_devel("trampoline target %lx", ptr);
+		pr_devel("trampoline target 0x%px", (void *)ptr);
 
 		/* This should match what was called */
 		if (ptr != entry) {
-			pr_err("addr %lx does not match expected %lx\n", ptr, entry);
+			pr_err("addr 0x%pK does not match expected 0x%pK\n",
+					(void *)ptr, (void *)entry);
 			return -EINVAL;
 		}
 	}
@@ -765,12 +772,13 @@ __ftrace_modify_call(struct dyn_ftrace *rec, unsigned long old_addr,
 		return -EFAULT;
 	}
 
-	pr_devel("trampoline target %lx", ptr);
+	pr_devel("trampoline target 0x%px", (void *)ptr);
 
 	entry = ppc_global_function_entry((void *)addr);
 	/* This should match what was called */
 	if (ptr != entry) {
-		pr_err("addr %lx does not match expected %lx\n", ptr, entry);
+		pr_err("addr 0x%pK does not match expected 0x%pK\n",
+				(void *)ptr, (void *)entry);
 		return -EINVAL;
 	}
 
-- 
2.25.1

[PATCH 3/3] powerpc/hw_bkpt: Update printk format specifiers for kernel addresses

[Naveen N. Rao]

Change prinkt format specifier from %lx to %pK to indicate kernel
pointer, and to hide the addresses from unprivileged users.

Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/hw_breakpoint.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/kernel/hw_breakpoint.c b/arch/powerpc/kernel/hw_breakpoint.c
index 72f461bd70fb..93a303cf0c67 100644
--- a/arch/powerpc/kernel/hw_breakpoint.c
+++ b/arch/powerpc/kernel/hw_breakpoint.c
@@ -257,7 +257,8 @@ static bool stepping_handler(struct pt_regs *regs, struct perf_event *bp,
 
 	if (!ret && (type == LARX || type == STCX)) {
 		printk_ratelimited("Breakpoint hit on instruction that can't be emulated."
-				   " Breakpoint at 0x%lx will be disabled.\n", addr);
+				   " Breakpoint at 0x%pK will be disabled.\n",
+				   (void *)addr);
 		goto disable;
 	}
 
@@ -286,7 +287,7 @@ static bool stepping_handler(struct pt_regs *regs, struct perf_event *bp,
 	 * it and throw a warning message to let the user know about it.
 	 */
 	WARN(1, "Unable to handle hardware breakpoint. Breakpoint at "
-		"0x%lx will be disabled.", addr);
+		"0x%pK will be disabled.", (void *)addr);
 
 disable:
 	perf_event_disable_inatomic(bp);
-- 
2.25.1

Re: [PATCH 1/3] powerpc/kprobes: Use appropriate format specifier for printing kernel address

[Christophe Leroy]

Le 23/04/2020 à 17:17, Naveen N. Rao a écrit :
> From: Balamuruhan S <bala24@linux.ibm.com>
> 
> Change use of %p to %pK when printing address of the instruction slot so
> that the actual kernel address is visible for privileged users.
> 
> Signed-off-by: Balamuruhan S <bala24@linux.ibm.com>
> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>

This series doesn't apply.

Linking it to https://github.com/linuxppc/issues/issues/390

> ---
>   arch/powerpc/kernel/optprobes.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/kernel/optprobes.c b/arch/powerpc/kernel/optprobes.c
> index ef0924b0809d..d5f8c25b7cac 100644
> --- a/arch/powerpc/kernel/optprobes.c
> +++ b/arch/powerpc/kernel/optprobes.c
> @@ -247,7 +247,7 @@ int arch_prepare_optimized_kprobe(struct optimized_kprobe *op, struct kprobe *p)
>   	/* Setup template */
>   	/* We can optimize this via patch_instruction_window later */
>   	size = (TMPL_END_IDX * sizeof(kprobe_opcode_t)) / sizeof(int);
> -	pr_devel("Copying template to %p, size %lu\n", buff, size);
> +	pr_devel("Copying template to %pK, size %lu\n", (void *)buff, size);
>   	for (i = 0; i < size; i++) {
>   		rc = patch_instruction(buff + i, *(optprobe_template_entry + i));
>   		if (rc) {