linux-kernel - Re: [PATCH v3 06/16] rbd: convert timeouts to secs_to

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: <df0c2400-147c-4104-a2e6-d1038ff31524@linux.microsoft.com>
Date: Tue, 25 Feb 2025 16:02:42 -0800
From: Easwar Hariharan <eahariha@...ux.microsoft.com>
To: Christophe JAILLET <christophe.jaillet@...adoo.fr>
Cc: eahariha@...ux.microsoft.com, Frank.Li@....com,
 James.Bottomley@...senPartnership.com, Julia.Lawall@...ia.fr,
 Shyam-sundar.S-k@....com, akpm@...ux-foundation.org, axboe@...nel.dk,
 broonie@...nel.org, cassel@...nel.org, cem@...nel.org,
 ceph-devel@...r.kernel.org, clm@...com, cocci@...ia.fr,
 dick.kennedy@...adcom.com, djwong@...nel.org, dlemoal@...nel.org,
 dongsheng.yang@...ystack.cn, dri-devel@...ts.freedesktop.org,
 dsterba@...e.com, festevam@...il.com, hch@....de, hdegoede@...hat.com,
 hmh@....eng.br, ibm-acpi-devel@...ts.sourceforge.net, idryomov@...il.com,
 ilpo.jarvinen@...ux.intel.com, imx@...ts.linux.dev,
 james.smart@...adcom.com, jgg@...pe.ca, josef@...icpanda.com,
 kalesh-anakkur.purayil@...adcom.com, kbusch@...nel.org,
 kernel@...gutronix.de, leon@...nel.org,
 linux-arm-kernel@...ts.infradead.org, linux-block@...r.kernel.org,
 linux-btrfs@...r.kernel.org, linux-ide@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-nvme@...ts.infradead.org,
 linux-pm@...r.kernel.org, linux-rdma@...r.kernel.org,
 linux-scsi@...r.kernel.org, linux-sound@...r.kernel.org,
 linux-spi@...r.kernel.org, linux-xfs@...r.kernel.org,
 martin.petersen@...cle.com, nicolas.palix@...g.fr, ogabbay@...nel.org,
 perex@...ex.cz, platform-driver-x86@...r.kernel.org, s.hauer@...gutronix.de,
 sagi@...mberg.me, selvin.xavier@...adcom.com, shawnguo@...nel.org,
 sre@...nel.org, tiwai@...e.com, xiubli@...hat.com, yaron.avizrat@...el.com
Subject: Re: [PATCH v3 06/16] rbd: convert timeouts to secs_to_jiffies()

On 2/25/2025 1:09 PM, Christophe JAILLET wrote:
> Le 25/02/2025 à 21:17, Easwar Hariharan a écrit :
>> Commit b35108a51cf7 ("jiffies: Define secs_to_jiffies()") introduced
>> secs_to_jiffies().  As the value here is a multiple of 1000, use
>> secs_to_jiffies() instead of msecs_to_jiffies() to avoid the multiplication
>>
>> This is converted using scripts/coccinelle/misc/secs_to_jiffies.cocci with
>> the following Coccinelle rules:
>>
>> @depends on patch@ expression E; @@
>>
>> -msecs_to_jiffies(E * 1000)
>> +secs_to_jiffies(E)
>>
>> @depends on patch@ expression E; @@
>>
>> -msecs_to_jiffies(E * MSEC_PER_SEC)
>> +secs_to_jiffies(E)
>>
>> While here, remove the no-longer necessary check for range since there's
>> no multiplication involved.
> 
> I'm not sure this is correct.
> Now you multiply by HZ and things can still overflow.
> 
> 
> Hoping I got casting right:
> 
> #define MSEC_PER_SEC    1000L
> #define HZ 100
> 
> 
> #define secs_to_jiffies(_secs) (unsigned long)((_secs) * HZ)
> 
> static inline unsigned long _msecs_to_jiffies(const unsigned int m)
> {
>     return (m + (MSEC_PER_SEC / HZ) - 1) / (MSEC_PER_SEC / HZ);
> }
> 
> int main() {
> 
>     int n = INT_MAX - 5;
> 
>     printf("res  = %ld\n", secs_to_jiffies(n));
>     printf("res  = %ld\n", _msecs_to_jiffies(1000 * n));
> 
>     return 0;
> }
> 
> 
> gives :
> 
> res  = -600
> res  = 429496130
> 
> with msec, the previous code would catch the overflow, now it overflows silently.
> 
> untested, but maybe:
>     if (result.uint_32 > INT_MAX / HZ)
>         goto out_of_range;
> 
> ?
> 
> CJ
> 

Thanks for the review! I was able to replicate your results, I'll try this range check
and get back.

Thanks,
Easwar (he/him)