On Sun, May 7, 2017 at 9:58 PM, Karim Eshapa <[email protected]> wrote:
> On Sun, 7 May 2017 20:36:55 +0200, Arnd Bergmann wrote:
>>On Sun, May 7, 2017 at 2:47 PM, Karim Eshapa <[email protected]> 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 <[email protected]>
>>>
>>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
