linux-kernel - Re: [PATCH] powerpc: Don't use %pK through printk

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <alpine.DEB.2.21.2502241840360.65342@angie.orcam.me.uk>
Date: Mon, 24 Feb 2025 18:54:47 +0000 (GMT)
From: "Maciej W. Rozycki" <macro@...am.me.uk>
To: Christophe Leroy <christophe.leroy@...roup.eu>
cc: Thomas Weißschuh <thomas.weissschuh@...utronix.de>, 
    Mahesh J Salgaonkar <mahesh@...ux.ibm.com>, 
    Oliver O'Halloran <oohall@...il.com>, 
    Madhavan Srinivasan <maddy@...ux.ibm.com>, 
    Michael Ellerman <mpe@...erman.id.au>, Nicholas Piggin <npiggin@...il.com>, 
    Naveen N Rao <naveen@...nel.org>, linuxppc-dev@...ts.ozlabs.org, 
    linux-kernel@...r.kernel.org
Subject: Re: [PATCH] powerpc: Don't use %pK through printk

On Mon, 24 Feb 2025, Christophe Leroy wrote:

> > 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 <thomas.weissschuh@...utronix.de>
> 
> Reviewed-by: Christophe Leroy <christophe.leroy@...roup.eu>
> 
> > ---
> >   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.

  Maciej