linux-kernel - Re: [PATCH V2 04/16] block, bfq: modify the peak-rate estimator

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <439E3C9C-B2D7-40F7-8831-129DCD0EC658@linaro.org>
Date:   Thu, 6 Apr 2017 21:37:11 +0200
From:   Paolo Valente <paolo.valente@...aro.org>
To:     Bart Van Assche <bart.vanassche@...disk.com>
Cc:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-block@...r.kernel.org" <linux-block@...r.kernel.org>,
        "fchecconi@...il.com" <fchecconi@...il.com>,
        "linus.walleij@...aro.org" <linus.walleij@...aro.org>,
        "axboe@...nel.dk" <axboe@...nel.dk>,
        Arianna Avanzini <avanzini.arianna@...il.com>,
        "broonie@...nel.org" <broonie@...nel.org>,
        "tj@...nel.org" <tj@...nel.org>,
        "ulf.hansson@...aro.org" <ulf.hansson@...aro.org>
Subject: Re: [PATCH V2 04/16] block, bfq: modify the peak-rate estimator


> Il giorno 04 apr 2017, alle ore 17:28, Bart Van Assche <bart.vanassche@...disk.com> ha scritto:
> 
> On Tue, 2017-04-04 at 12:42 +0200, Paolo Valente wrote:
>>> Il giorno 31 mar 2017, alle ore 17:31, Bart Van Assche <bart.vanassche@...disk.com> ha scritto:
>>> 
>>> On Fri, 2017-03-31 at 14:47 +0200, Paolo Valente wrote:
>>>> +               delta_ktime = ktime_get();
>>>> +       delta_ktime = ktime_sub(delta_ktime, bfqd->last_budget_start);
>>>> +       delta_usecs = ktime_to_us(delta_ktime);
>>> 
>>> This patch changes the type of the variable in which the result of ktime_to_us()
>>> is stored from u64 into u32 and next compares that result with LONG_MAX. Since
>>> ktime_to_us() returns a signed 64-bit number, are you sure you want to store that
>>> result in a 32-bit variable? If ktime_to_us() would e.g. return 0xffffffff00000100
>>> or 0x100000100 then the assignment will truncate these numbers to 0x100.
>> 
>> The instruction above the assignment you highlight stores in
>> delta_ktime the difference between 'now' and the last budget start.
>> The latter may have happened at most about 100 ms before 'now'.  So
>> there should be no overflow issue.
> 
> Hello Paolo,
> 
> Please double check the following code: if (delta_usecs < 1000 || delta_usecs >= LONG_MAX)
> Since delta_usecs is a 32-bit variable and LONG_MAX a 64-bit constant on 64-bit systems
> I'm not sure that code will do what it is intended to do.
> 

Yes, sorry. Actually, it never occurred to me to see that extra condition to hold over the last eight years on 32-bit systems. So I think I will just remove it. Unless Fabio, who inserted that condition several years ago, has something to say.

Thanks,
Paolo

> Thanks,
> 
> Bart.