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: <a7a135c9-c65b-48a8-a3a8-6aa98afe77d0@csgroup.eu>
Date: Tue, 25 Feb 2025 06:58:44 +0100
From: Christophe Leroy <christophe.leroy@...roup.eu>
To: "Maciej W. Rozycki" <macro@...am.me.uk>
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



Le 24/02/2025 à 19:54, Maciej W. Rozycki a écrit :
> ***ATTENTION, Sopra Steria Group cannot confirm the identity of this email sender (SPF record failure). This might be a fake email from an attacker, if you have any doubts report and delete the email.***
> 
> ***ATTENTION, Sopra Steria Group ne peut pas confirmer l’identité de l’émetteur de ce message (SPF record failure). Il pourrait s’agir d’un faux message, à détruire si vous avez un doute ***
> 
> 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%7C9079ef2ec60e4717ec8e08dd5504b718%7C8b87af7d86474dc78df45f69a2011bb5%7C0%7C0%7C638760200949886583%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=d2QCtnssTlVmKOKR57rui%2Fq73UsAAoZrim%2FABaz5IFs%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.

The purpose of hashed addresses is to avoid kernel addresses to leak to 
the kernel log. Here you have function names, if you get real function 
addresses at the same time, then you know everything about kernel 
addresses and for instance KASLR becomes just pointless.

By the way, why do you need the addresses at all in addition to function 
names ? When I look at x86 dump stack, they only print function name, 
using %pBb

Christophe