[PATCH] char: random: casting prevents missing calculations

[PATCH] char: random: casting prevents missing calculations

[Karim Eshapa]

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@gmail.com>
---
 drivers/char/random.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/char/random.c b/drivers/char/random.c
index 0ab0249..f685e78 100644
--- a/drivers/char/random.c
+++ b/drivers/char/random.c
@@ -1012,7 +1012,7 @@ static void add_timer_randomness(struct timer_rand_state *state, unsigned num)
 
 	preempt_disable();
 
-	sample.jiffies = jiffies;
+	sample.jiffies = (long)jiffies;
 	sample.cycles = random_get_entropy();
 	sample.num = num;
 	r = &input_pool;
@@ -1025,7 +1025,7 @@ static void add_timer_randomness(struct timer_rand_state *state, unsigned num)
 	 */
 
 	if (!state->dont_count_entropy) {
-		delta = sample.jiffies - state->last_time;
+		delta = sample.jiffies - (long)state->last_time;
 		state->last_time = sample.jiffies;
 
 		delta2 = delta - state->last_delta;
-- 
2.7.4

Re: [PATCH] char: random: casting prevents missing calculations

[Arnd Bergmann]

On Sun, May 7, 2017 at 2:47 PM, Karim Eshapa <karim.eshapa@gmail.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@gmail.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?

       Arnd

RE: [PATCH] char: random: casting prevents missing calculations

[Karim Eshapa]

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@gmail.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@gmail.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.

Thanks,
Karim

Re: [PATCH] char: random: casting prevents missing calculations

[Arnd Bergmann]

On Sun, May 7, 2017 at 9:58 PM, Karim Eshapa <karim.eshapa@gmail.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@gmail.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@gmail.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