On Fri, Feb 28, 2025 at 08:15:02PM +0000, Maciej W. Rozycki wrote: > 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.
Indeed, I got confused with some other patches where WARN() is used mostly. This makes it a bit murkier. > > > > 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. Sure. It would be great if one of the maintainers could confirm this preference. > > > " > > > 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? I am considering on a case-by-case basis. But mostly the decision is that %p is enough, because by default %pK has been the same as %p anyways. Also the current wave of replacements does not touch valid users of %pK. They will stay and later be replaced with a new and better API. > > Security is only one aspect. > > I think it's important enough though for us to ensure we don't compromise > it by chance. Agreed.
