Re: xt_hashlimig build error (was Re: [RFC 01/17] x86/asm/64: Remove the restore_c_regs_and_iret label)

[Linus Torvalds]

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