From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1755893AbdIGUpi (ORCPT <rfc822;w@1wt.eu>);
        Thu, 7 Sep 2017 16:45:38 -0400
Received: from mail-it0-f66.google.com ([209.85.214.66]:35164 "EHLO
        mail-it0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1751456AbdIGUph (ORCPT
        <rfc822;linux-kernel@vger.kernel.org>);
        Thu, 7 Sep 2017 16:45:37 -0400
X-Google-Smtp-Source: AOwi7QDAVEx1Wl2o07KPZlmj/braeFHxZYPNDvtC6Q7sflAdaEKeamRr5TklOWWv/M1AvWbhFgEGmlnhEN5bv3uj8HI=
MIME-Version: 1.0
In-Reply-To: <6667f710-68f3-b97e-b0eb-d9879476831e@akamai.com>
References: <CA+55aFznorYJd8OyV09eHt4zrRrrv2aEq_D=Q8k2w+CmgJ=+cw@mail.gmail.com>
 <69b38985-e094-ddcc-6f7e-d6e5cc2c657e@akamai.com> <CA+55aFxvD1R0xg8ZEjNyHtvhBZchWBJTU=FQNiY6Mv8E4TqZ=Q@mail.gmail.com>
 <6667f710-68f3-b97e-b0eb-d9879476831e@akamai.com>
From: Linus Torvalds <torvalds@linux-foundation.org>
Date: Thu, 7 Sep 2017 13:45:36 -0700
X-Google-Sender-Auth: ZFOywUf2JA1CmeQFoGZOnUcweEc
Message-ID: <CA+55aFxkyvW7jEzET65GymJbKZppb6se589_J1nnN0JXW5cBpg@mail.gmail.com>
Subject: Re: xt_hashlimig build error (was Re: [RFC 01/17] x86/asm/64: Remove
 the restore_c_regs_and_iret label)
To: Vishwanath Pai <vpai@akamai.com>
Cc: Ingo Molnar <mingo@kernel.org>, Igor Lubashev <ilubashe@akamai.com>,
        Josh Hunt <johunt@akamai.com>, Pablo Neira Ayuso <pablo@netfilter.org>,
        Borislav Petkov <bp@alien8.de>, Andy Lutomirski <luto@kernel.org>,
        "the arch/x86 maintainers" <x86@kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Brian Gerst <brgerst@gmail.com>,
        Andrew Cooper <andrew.cooper3@citrix.com>,
        Juergen Gross <jgross@suse.com>,
        Boris Ostrovsky <boris.ostrovsky@oracle.com>,
        Kees Cook <keescook@chromium.org>,
        Andrew Morton <akpm@linux-foundation.org>,
        "David S. Miller" <davem@davemloft.net>, Arnd Bergmann <arnd@arndb.de>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Thu, Sep 7, 2017 at 1:16 PM, Vishwanath Pai <vpai@akamai.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