On Mon, Feb 24, 2025 at 06:54:47PM +0000, Maciej W. Rozycki wrote:

On Mon, 24 Feb 2025, Christophe Leroy wrote:

quoted

quoted

Restricted pointers ("%pK") are not meant to be used through printk().
It can unintentionally expose security sensitive, raw pointer values.

Use regular pointer formatting instead.

Link:
https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Flkml%2F20250113171731-dc10e3c1-da64-4af0-b767-7c7070468023%40linutronix.de%2F&data=05%7C02%7Cchristophe.leroy%40csgroup.eu%7C75a852a0fef54fa43a3608dd4f263f45%7C8b87af7d86474dc78df45f69a2011bb5%7C0%7C0%7C638753747883689862%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=aUgq6pXb1ySaQ6e%2FdyM09jfc4MNLE71Njw0%2FnCg%2F6VU%3D&reserved=0
Signed-off-by: Thomas Weißschuh <redacted>

Reviewed-by: Christophe Leroy <redacted>

quoted

---
  arch/powerpc/kernel/eeh_driver.c | 2 +-
  arch/powerpc/perf/hv-24x7.c      | 8 ++++----
  2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/kernel/eeh_driver.c

b/arch/powerpc/kernel/eeh_driver.c
index
7efe04c68f0fe3fb1c3c13d97d58e79e47cf103b..10ce6b3bd3b7c54f91544ae7f7fd3f32a51ee09a
100644

--- a/arch/powerpc/kernel/eeh_driver.c
+++ b/arch/powerpc/kernel/eeh_driver.c

@@ -907,7 +907,7 @@ void eeh_handle_normal_event(struct eeh_pe *pe)
  		/* FIXME: Use the same format as dump_stack() */
  		pr_err("EEH: Call Trace:\n");
  		for (i = 0; i < pe->trace_entries; i++)
-			pr_err("EEH: [%pK] %pS\n", ptrs[i], ptrs[i]);
+			pr_err("EEH: [%p] %pS\n", ptrs[i], ptrs[i]);
    		pe->trace_entries = 0;
  	}

 But shouldn't this be using `%px' then instead?  It would be sad if all 
the address information from error reports such as below:

EEH: Call Trace:
EEH: [000000008985bc3b] __eeh_send_failure_event+0x78/0x150
EEH: [000000008c4c5782] eeh_dev_check_failure+0x388/0x6b0
EEH: [000000001fb766c1] eeh_check_failure+0x98/0x100
EEH: [000000004b9af8c6] dfx_port_read_long+0xb0/0x130 [defxx]
EEH: [00000000e23999c1] dfx_interrupt+0x80/0x8c0 [defxx]
EEH: [00000000c7884fb7] __handle_irq_event_percpu+0x9c/0x2f0
EEH: [000000008d4e9afd] handle_irq_event_percpu+0x44/0xc0
EEH: [000000008c39ece4] handle_irq_event+0x74/0xc0
EEH: [00000000d85114a9] handle_fasteoi_irq+0xd4/0x220
EEH: [00000000a692ef4e] generic_handle_irq+0x54/0x80
EEH: [00000000a6db243b] __do_irq+0x68/0x200
EEH: [0000000040ccff9e] call_do_irq+0x14/0x24
EEH: [00000000e8e9ddf7] do_IRQ+0x78/0xd0
EEH: [0000000031916539] replay_soft_interrupts+0x180/0x370
EEH: [000000001b7e5728] arch_local_irq_restore+0x48/0xc0
EEH: [00000000088691b7] cpuidle_enter_state+0x108/0x560
EEH: [00000000e6e26f30] cpuidle_enter+0x50/0x70
EEH: [000000007c26474c] call_cpuidle+0x4c/0x80
EEH: [0000000036b8a2fc] do_idle+0x360/0x3b0
EEH: [0000000048702083] cpu_startup_entry+0x38/0x40
EEH: [00000000d3b1fb8d] start_secondary+0x62c/0x660
EEH: [0000000041a9a815] start_secondary_prolog+0x10/0x14

was suddenly lost from the kernel log, the access to which unprivileged 
users can be denied if so desired according to the site policy.  Whereas 
running the kernel such as to have all output from plain `%p' exposed just 
to cope with this proposed change, now that seems like a security risk.

Your point makes sense.
*But* the addresses in your example are already hashed,
as indicated by the all-zero upper 32 bits.
By default, when kptr_restrict is set to 0, %pK behaves the same as %p.
The same happened for a bunch of other architectures and nobody seems
to have noticed in the past.
The symbol-relative pointers or pointer formats designed for backtraces,
as notes by Christophe, seem to be enough.

But personally I'm also fine with using %px, as my goal is to remove the
error-prone and confusing %pK.

Thomas