linux-kernel - Re: [PATCH] char: random: casting prevents missing calculations

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] [day] [month] [year] [list]

Message-ID: <CAK8P3a2T6wMFV--QN+zxX7+nLjc6LTHip4d-hdxx4rbk4b9csQ@mail.gmail.com>
Date:   Sun, 7 May 2017 23:34:47 +0200
From:   Arnd Bergmann <arnd@...db.de>
To:     Karim Eshapa <karim.eshapa@...il.com>
Cc:     "Theodore Ts'o" <tytso@....edu>,
        gregkh <gregkh@...uxfoundation.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] char: random: casting prevents missing calculations

On Sun, May 7, 2017 at 9:58 PM, Karim Eshapa <karim.eshapa@...il.com> wrote:
> On Sun, 7 May 2017 20:36:55 +0200, Arnd Bergmann wrote:
>>On Sun, May 7, 2017 at 2:47 PM, Karim Eshapa <karim.eshapa@...il.com> wrote:
>>> Cast (long)jiffies and (long)state->last_time beacause
>>> they tends to unsigned long. may cause a problem specially
>>> when comparison happens (< 0).
>>>
>>> Signed-off-by: Karim Eshapa <karim.eshapa@...il.com>
>>>
>>I don't understand what you are saying above, and the patch does not
>>appear to have any effect since the destination variable is already of
>>type 'long'.
>>
>>What problem did you observe?
>>
>
> I mean if jiffies = 0xf0000000 and
> state->last_time was = 0x70000000 then
>
> sample.jiffies = jiffies; here you assign signed long with
> unsigned with last bit = 1
> ..
> ...
> if (!state->dont_count_entropy) {
>         delta = sample.jiffies - state->last_time;
>         state->last_time = sample.jiffies;
>         ...
>         ...
>
>         if (delta < 0)
>         so, here this condition will be
>         true while it has to be false.
> }
>
> So, I think that may cause a problem.

No:

a) your patch does not change this behavior.
b) The case you describe (delta == -LONG_MIN) only hits when
    add_timer_randomness gets called after exactly a 0x80000000
    jiffies (24 days plus a bit, or more depending on CONFIG_HZ)
   delay, when in practice it should get called for every input
   or block event.
c) If you do manage to time the input even exactly to the right
    time interval of 24 days and the computer does nothing else
    in the meantime, min_t(int, fls(delta>>1), 11) is still limited to
    11 bits for this one time event, and that would be a reasonable
    number of entropy bits for waiting this long.

      Arnd