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

From	Linus Torvalds <torvalds@linux-foundation.org>
Newsgroups	linux.kernel
Subject	Re: xt_hashlimig build error (was Re: [RFC 01/17] x86/asm/64: Remove the restore_c_regs_and_iret label)
Date	2017-09-07 22:50 +0200
Message-ID	<unbUm-5ay-3@gated-at.bofh.it> (permalink)
References	<un9fT-3ll-73@gated-at.bofh.it> <un9IR-3K0-5@gated-at.bofh.it> <una2d-3SJ-9@gated-at.bofh.it> <unbrj-50W-7@gated-at.bofh.it>
Organization	linux.* mail to news gateway

Show all headers | View raw

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

Thread

xt_hashlimig build error (was Re: [RFC 01/17] x86/asm/64: Remove the  restore_c_regs_and_iret label) Linus Torvalds <torvalds@linux-foundation.org> - 2017-09-07 20:00 +0200
  Re: xt_hashlimig build error (was Re: [RFC 01/17] x86/asm/64: Remove  the restore_c_regs_and_iret label) Vishwanath Pai <vpai@akamai.com> - 2017-09-07 20:30 +0200
    Re: xt_hashlimig build error (was Re: [RFC 01/17] x86/asm/64: Remove  the restore_c_regs_and_iret label) Linus Torvalds <torvalds@linux-foundation.org> - 2017-09-07 20:50 +0200
      Re: xt_hashlimig build error (was Re: [RFC 01/17] x86/asm/64: Remove  the restore_c_regs_and_iret label) Vishwanath Pai <vpai@akamai.com> - 2017-09-07 22:20 +0200
        Re: xt_hashlimig build error (was Re: [RFC 01/17] x86/asm/64: Remove  the restore_c_regs_and_iret label) Linus Torvalds <torvalds@linux-foundation.org> - 2017-09-07 22:50 +0200
          Re: xt_hashlimig build error (was Re: [RFC 01/17] x86/asm/64: Remove  the restore_c_regs_and_iret label) Vishwanath Pai <vpai@akamai.com> - 2017-09-07 23:30 +0200
        RE: xt_hashlimig build error (was Re: [RFC 01/17] x86/asm/64: Remove  the restore_c_regs_and_iret label) "Lubashev, Igor" <ilubashe@akamai.com> - 2017-09-07 22:50 +0200

csiph-web