linux-kernel - Re: xt_hashlimig build error (was Re: [RFC 01/17] x86/asm/64: Remove the restore_c_regs_and

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: <CA+55aFxkyvW7jEzET65GymJbKZppb6se589_J1nnN0JXW5cBpg@mail.gmail.com>
Date:   Thu, 7 Sep 2017 13:45:36 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Vishwanath Pai <vpai@...mai.com>
Cc:     Ingo Molnar <mingo@...nel.org>,
        Igor Lubashev <ilubashe@...mai.com>,
        Josh Hunt <johunt@...mai.com>,
        Pablo Neira Ayuso <pablo@...filter.org>,
        Borislav Petkov <bp@...en8.de>,
        Andy Lutomirski <luto@...nel.org>,
        "the arch/x86 maintainers" <x86@...nel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Brian Gerst <brgerst@...il.com>,
        Andrew Cooper <andrew.cooper3@...rix.com>,
        Juergen Gross <jgross@...e.com>,
        Boris Ostrovsky <boris.ostrovsky@...cle.com>,
        Kees Cook <keescook@...omium.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        "David S. Miller" <davem@...emloft.net>,
        Arnd Bergmann <arnd@...db.de>
Subject: Re: xt_hashlimig build error (was Re: [RFC 01/17] x86/asm/64: Remove
 the restore_c_regs_and_iret label)

On Thu, Sep 7, 2017 at 1:16 PM, Vishwanath Pai <vpai@...mai.com> wrote:
>
> Writing U32INT_MAX as 0xFFFFFFFFULL was a mistake on my part. I could
> have avoided all of this by using built-in constants instead of trying
> to define them myself. I will rewrite the function as below and send out
> another patch:
>
> static u64 user2rate_bytes(u64 user)
> {
>         u64 r;
>
>         r = user ? U32_MAX / (u32) user : U32_MAX;
>         r = (r - 1) << XT_HASHLIMIT_BYTE_SHIFT;
>         return r;
> }

No, that is *still* wrong.

In particular, the test for "user" being zero is done in 64 bits, but
then when you do the divide, the cast to (u32) will take the low 32
bits - which may be zero, because only upper bits were set.

So now you get a divide-by-zero.

What seems to be going on is that a value larger than UINT32_MAX is
basically "invalid", since the reverse function cannot possibly
generate that.

So one possible fix is to just make that an error case in the caller,
and then make user2rate_bytes() not take (or return) "u64" at all, but
simply use u32.

Please be more careful here.

              Linus