Re: [PATCH] lib/hexdump: introduce DUMP_PREFIX_UNHASHED for unhashed addresses

[Petr Mladek]

On Thu 2021-01-14 20:56:36, Timur Tabi wrote:
> On 1/11/21 7:30 PM, Andrew Morton wrote:
> > I doubt if Kees (or I or anyone else) can review this change because
> > there are no callers which actually use the new DUMP_PREFIX_UNHASHED.
> > Is it intended that some other places in the kernel be changed to use
> > this?  If so, please describe where and why, so that others can better
> > understand both the requirement and the security implications.
> 
> In my opinion, hashed addresses make no sense in a hexdump, so I would say
> that ALL callers should change.  But none of the drivers I've written call
> print_hex_dump(), so I can't make those changes myself.

I know that you probably know it because you introduced new mode
instead of updating the existing one. But to be sure.

We need to be careful here. The hashed pointer has been introduced for
a reason. It prevents leaking pointers and helping bad guys.

The original plan was to introduce %pK. It was supposed to prevent
non-privileged users from seeing the real pointer value. It did not
really worked because it was only rarely used. The plain %p was
heavily used in historical and even in a new code.

By other words, every print_hex_dump() used need to be reviewed in
which context might be called.

> > If it is intended that this be used mainly for developer debug and not
> > to be shipped in the mainline kernel then let's get this info into the
> > changelog as well.
> 
> I definitely want this patch included in the mainline kernel.  Just because
> there aren't any users today doesn't mean that there won't be. In fact, I
> suspect that most current users haven't noticed that the addresses have
> changed or don't care any more, but if they were to write the code today,
> they would use unhashed addresses.

I am pretty sure that will look for this functionality sooner or
later. The hashed pointers make debugging really complicated.

> If you want, I can include a patch that changes a few callers of
> print_hex_dump() to use DUMP_PREFIX_UNHASHED, based on what I think would be
> useful.

It would be nice.

Best Regards,
Petr