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.2502281957310.12637@angie.orcam.me.uk>
Date: Fri, 28 Feb 2025 20:15:02 +0000 (GMT)
From: "Maciej W. Rozycki" <macro@...am.me.uk>
To: Thomas Weißschuh <thomas.weissschuh@...utronix.de>
cc: Christophe Leroy <christophe.leroy@...roup.eu>, 
    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 Wed, 26 Feb 2025, Thomas Weißschuh wrote:

> > > 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.
> > 
> >  I do hope so.
> 
> As mentioned before, personally I am fine with using %px here.

 Glad to hear!

> The values are in the register dumps anyways and security sensitive deployments
> will panic on WARN(), making the information disclosure useless.

 And even more so, I wasn't aware of this feature.  But this code doesn't 
make use of the WARN() facility, it just prints at the heightened KERN_ERR 
priority.

> > > But personally I'm also fine with using %px, as my goal is to remove the
> > > error-prone and confusing %pK.
> > 
> >  It's clear that `%pK' was meant to restrict access to /proc files and the 
> > like that may be accessible by unprivileged users:
> 
> Then let's stop abusing it. For something that is clear, it is
> misunderstood very often.

 Absolutely, I haven't questioned the removal of `%pK', but the switch to 
`%p' rather than `%px' specifically for this single hunk of your patch.

> > "
> > kptr_restrict
> > =============
> > 
> > This toggle indicates whether restrictions are placed on
> > exposing kernel addresses via ``/proc`` and other interfaces.
> > "
> > 
> > and not the kernel log, the information in which may come from rare events 
> > that are difficult to trigger and hard to recover via other means.  Sigh. 
> > Once you've got access to the kernel log, you may as well wipe the system 
> > or do any other harm you might like.
> 
> As I understand it, both the security and printk maintainers don't want the
> kernel log in general to be security sensitive and restricted.
> My goal here is not to push site-specific policy into the kernel but make life
> easier for kernel developers by removing the confusing and error-prone %pK
> altogether.

 Let me ask a different question then: is your approach to bulk-switch all 
instances of `%pK' to `%p' as the safe default and let other people figure 
out afterwards whether a different conversion specifier ought to be used 
instead on a case-by-case basis and then follow up with another patch, or 
will you consider these alternatives right away?

> Security is only one aspect.

 I think it's important enough though for us to ensure we don't compromise 
it by chance.

  Maciej