linux-kernel - hwmon: drivetemp: bogus values after wake up from suspend

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

Message-ID: <1af049a1-63ae-ee55-05d5-0e55eb00bd0e@applied-asynchrony.com>
Date:   Mon, 6 Apr 2020 18:23:01 +0200
From:   Holger Hoffstätte <holger@...lied-asynchrony.com>
To:     LKML <linux-kernel@...r.kernel.org>,
        Guenter Roeck <linux@...ck-us.net>
Subject: hwmon: drivetemp: bogus values after wake up from suspend

I've been giving the drivetemp hwmon driver a try and am very happy
with it; works right away and - much to my surprise - doesn't wake up
HDDs that have gone to sleep. Nice!

I did notice one tiny thing though: after waking up from suspend, my SSD
(Samsung 850 Pro) reports a few initial bogus values - suspiciously -128°,
which is definitely not the temperature in my office. While this is more
a cosmetic problem, it cramps my monitoring setup and leads to wrong graphs.
Can't have that!

So I looked into the source and found that the values are (understandably)
passed on unfiltered/uncapped. Since it's unlikely any active device has
operating temperature below-zero, I figured the laziest way is to cap the
value to positive:

diff -rup a/drivers/hwmon/drivetemp.c b/drivers/hwmon/drivetemp.c
--- a/drivers/hwmon/drivetemp.c	2020-04-02 08:02:32.000000000 +0200
+++ b/drivers/hwmon/drivetemp.c	2020-04-06 18:13:04.892554087 +0200
@@ -147,7 +147,7 @@ static LIST_HEAD(drivetemp_devlist);
  #define INVALID_TEMP		0x80

  #define temp_is_valid(temp)	((temp) != INVALID_TEMP)
-#define temp_from_sct(temp)	(((s8)(temp)) * 1000)
+#define temp_from_sct(temp)	(max(0, ((s8)(temp)) * 1000))

  static inline bool ata_id_smart_supported(u16 *id)
  {

The assumption is of course *theoretically* wrong since some
equipment might indeed operate in negative C°. One way might be
to use the device's "low" operating point first, but then that
might not be available and we'd be back to capping to 0.
I'm open to other suggestions. :)

thanks,
Holger