linux-kernel - Re: linux-5.3-rc4/drivers/staging/comedi/drivers/dt3000.c:373: (error) Signed integer overflow for expression 'divider*base'

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

Date:   Mon, 12 Aug 2019 11:14:19 +0100
From:   Ian Abbott <abbotti@....co.uk>
To:     David Binderman <dcb314@...mail.com>,
        "hsweeten@...ionengravers.com" <hsweeten@...ionengravers.com>,
        "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
        "devel@...verdev.osuosl.org" <devel@...verdev.osuosl.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: linux-5.3-rc4/drivers/staging/comedi/drivers/dt3000.c:373:
 (error) Signed integer overflow for expression 'divider*base'

On 12/08/2019 08:37, David Binderman wrote:
> Hello there,
> 
> Source code is
> 
>      prescale = 15;
>      base = timer_base * (1 << prescale);
>      divider = 65535;
>      *nanosec = divider * base;
> 
> timer_base seems to be 500 or 100.
> nanosec is a pointer to int, so it can only hold about 2,000,000,000 nanoseconds, or about 2 seconds.

Thanks for the report.

I couldn't reproduce the error with the compiler version I was using, 
but I can see that it is likely to result in an arithmetic overflow.

I think the main problem is that the line (in (dt3k_ns_to_timer()):

       base = timer_base * (1 << prescale);

should be:

       base = timer_base * (prescale + 1);

which matches the earlier instance of this calculation in the same function.

In practice, these lines of code should never be reached due to earlier 
range checks in dt3k_ai_cmdtest().

> Suggest rework code to use longs.

It wouldn't do any harm to change the `int` variables to `unsigned int`.

> 
> Regards
> 
> David Binderman
> 
> 
> time-_Base seems to be 50 or 100.
> 


-- 
-=( Ian Abbott <abbotti@....co.uk> || Web: www.mev.co.uk )=-
-=( MEV Ltd. is a company registered in England & Wales. )=-
-=( Registered number: 02862268.  Registered address:    )=-
-=( 15 West Park Road, Bramhall, STOCKPORT, SK7 3JZ, UK. )=-