linux-kernel - Re: [RFC][PATCH] <linux/stringhash.h>: fix end_name

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]

Date:   Mon, 5 Feb 2018 19:51:03 +0200
From:   Amir Goldstein <amir73il@...il.com>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     Al Viro <viro@...iv.linux.org.uk>,
        Miklos Szeredi <miklos@...redi.hu>,
        linux-fsdevel <linux-fsdevel@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [RFC][PATCH] <linux/stringhash.h>: fix end_name_hash() for 64bit long

On Mon, Feb 5, 2018 at 7:36 PM, Linus Torvalds
<torvalds@...ux-foundation.org> wrote:
> On Mon, Feb 5, 2018 at 9:32 AM, Amir Goldstein <amir73il@...il.com> wrote:
>>
>> Assuming the fix is not moronic, I wouldn't even know where to begin testing
>> its affects, or how to prove if there really is a problem.
>> Any suggestions?
>
> So I *think* that this was on purpose, but it's a long time ago, and
> we've changed some of the hashing since.
>
> And I think you're wrong that it's a no-op on 32-bit. It's a very
> expensive and pointless multiplication there too, even if the *shift*
> ends up being a nop-op.
>

I did not mean this is no-op on 32-bit, what I meant is that the patch
changes nothing for 32-bit long, besides the zero shift, because:

#if BITS_PER_LONG == 32
#define hash_long(val, bits) hash_32(val, bits)

and hash_32_generic() is just __hash_32() with zero shift.

I guess I cannot make that claim for platforms that HAVE_ARCH_HASH_32.

> The name hashing is pretty performance-sensitive.
>

I realize that, that's why I am asking.
BTW, I think this shouldn't affect dcache on x86_64 and arm64, because they
use the CONFIG_DCACHE_WORD_ACCESS variant of full_name_hash().

Anyway, my work does not depend on this fix, so I am fine with leaving it be.
I just wanted to point it out is case you knew what the original intention was.

Thanks,
Amir.