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>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20210621142415.GA3604789@roeck-us.net>
Date:   Mon, 21 Jun 2021 07:24:15 -0700
From:   Guenter Roeck <linux@...ck-us.net>
To:     Dmitry Osipenko <digetx@...il.com>
Cc:     Zhang Rui <rui.zhang@...el.com>,
        Daniel Lezcano <daniel.lezcano@...aro.org>,
        Amit Kucheria <amitk@...nel.org>,
        Jean Delvare <jdelvare@...e.com>, linux-hwmon@...r.kernel.org,
        linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-tegra@...r.kernel.org
Subject: Re: [PATCH v2 1/2] hwmon: (lm90) Prevent integer overflow of
 temperature calculations

On Mon, Jun 21, 2021 at 03:14:40PM +0300, Dmitry Osipenko wrote:
> 21.06.2021 15:12, Guenter Roeck пишет:
> > On Mon, Jun 21, 2021 at 12:14:07AM +0300, Dmitry Osipenko wrote:
> >> The minimum temperature value that is passed to the driver is unlimited
> >> and value that is close to INT_MIN results in integer overflow of
> >> temperature calculations made by the driver. Limit the value in order
> >> to prevent the overflow. For now the overflow condition is harmless,
> >> but thermal framework won't work properly once we will support the
> >> set_trips() callback because it will pass INT_MIN value to the driver.
> >>
> > AFAICS that should only happen for lm99 because all other values
> > are bound in the temp_to_xxx functions. Where else do you see an
> > overflow (or underflow) ?
> 
> You're correct that the overflow affects only lm99. But why we should
> ignore it?

That isn't the point. The point is that you claimed there would be a
generic underflow, which is not the case. That means we'll only need
to apply the fix to the lm99 specific code (which unconditionally
subtracts an offset from the provided value, causing the underflow).

Anyway, thanks for alerting me to the issue. As it turns out, there are
other underflow issues in the driver. With improved module test scripts,
I get:

Testing lm90 ...
temp1_crit_hyst: Suspected underflow: [min=54000, read 85000, written -9223372036854775808]
Testing lm99 ...
temp1_crit_hyst: Suspected underflow: [min=96000, read 127000, written -9223372036854775808]
temp2_crit: Suspected underflow: [min=-112000, read 143000, written -9223372036854775808]
temp2_min: Suspected underflow: [min=-112000, read 143875, written -9223372036854775808]
temp2_max: Suspected underflow: [min=-112000, read 143875, written -9223372036854775808]

So we'll need fixes for lm99 temp2_{min/max/crit} and for temp1_crit_hyst
(the latter affects all chips supported by the driver).

Thanks,
Guenter

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ