linux-kernel - Re: [PATCH] lib/hexdump: introduce DUMP_PREFIX

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: <YAFenc9MHvPNrZ06@alley>
Date:   Fri, 15 Jan 2021 10:21:33 +0100
From:   Petr Mladek <pmladek@...e.com>
To:     Timur Tabi <timur@...nel.org>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        torvalds@...ux-foundation.org, linux-kernel@...r.kernel.org,
        Sergey Senozhatsky <sergey.senozhatsky@...il.com>,
        Roman Fietze <roman.fietze@...na.com>,
        Kees Cook <keescook@...omium.org>
Subject: Re: [PATCH] lib/hexdump: introduce DUMP_PREFIX_UNHASHED for unhashed
 addresses

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