linux-kernel - [PATCH 1/3] hwmon: tmp421: handle I2C errors

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

Message-Id: <20210922134154.20766-1-fercerpav@gmail.com>
Date:   Wed, 22 Sep 2021 16:41:52 +0300
From:   Paul Fertser <fercerpav@...il.com>
To:     linux-hwmon@...r.kernel.org
Cc:     Jean Delvare <jdelvare@...e.com>,
        Guenter Roeck <linux@...ck-us.net>,
        linux-kernel@...r.kernel.org, Paul Fertser <fercerpav@...il.com>,
        stable@...r.kernel.org
Subject: [PATCH 1/3] hwmon: tmp421: handle I2C errors

Function i2c_smbus_read_byte_data() can return a negative error number
instead of the data read if I2C transaction failed for whatever reason.

I consider this fix to be stable material as lack of error checking here
leads to serious issues on production hardware. Errors treated as
temperatures produce spurious critical temperature-crossed-threshold
errors in BMC logs for OCP server hardware. The patch was tested with
Mellanox OCP Mezzanine card emulating TMP421 protocol for temperature
sensing which sometimes leads to I2C protocol error during early boot up
stage.

Cc: stable@...r.kernel.org
Signed-off-by: Paul Fertser <fercerpav@...il.com>
---
 drivers/hwmon/tmp421.c | 31 ++++++++++++++++++++++---------
 1 file changed, 22 insertions(+), 9 deletions(-)

diff --git a/drivers/hwmon/tmp421.c b/drivers/hwmon/tmp421.c
index ede66ea6a730..6175ed4b10bd 100644
--- a/drivers/hwmon/tmp421.c
+++ b/drivers/hwmon/tmp421.c
@@ -93,7 +93,7 @@ struct tmp421_data {
 	struct hwmon_channel_info temp_info;
 	const struct hwmon_channel_info *info[2];
 	struct hwmon_chip_info chip;
-	char valid;
+	int last_errno;
 	unsigned long last_updated;
 	unsigned long channels;
 	u8 config;
@@ -128,20 +128,30 @@ static struct tmp421_data *tmp421_update_device(struct device *dev)
 	mutex_lock(&data->update_lock);
 
 	if (time_after(jiffies, data->last_updated + (HZ / 2)) ||
-	    !data->valid) {
-		data->config = i2c_smbus_read_byte_data(client,
-			TMP421_CONFIG_REG_1);
+	    data->last_errno) {
+		data->last_errno = i2c_smbus_read_byte_data(client,
+							    TMP421_CONFIG_REG_1);
+		if (data->last_errno < 0)
+			goto exit;
+		data->config =  data->last_errno;
 
 		for (i = 0; i < data->channels; i++) {
-			data->temp[i] = i2c_smbus_read_byte_data(client,
-				TMP421_TEMP_MSB[i]) << 8;
-			data->temp[i] |= i2c_smbus_read_byte_data(client,
-				TMP421_TEMP_LSB[i]);
+			data->last_errno = i2c_smbus_read_byte_data(client,
+								    TMP421_TEMP_MSB[i]);
+			if (data->last_errno < 0)
+				goto exit;
+			data->temp[i] = data->last_errno << 8;
+			data->last_errno = i2c_smbus_read_byte_data(client,
+								    TMP421_TEMP_LSB[i]);
+			if (data->last_errno < 0)
+				goto exit;
+			data->temp[i] |= data->last_errno;
 		}
 		data->last_updated = jiffies;
-		data->valid = 1;
+		data->last_errno = 0;
 	}
 
+exit:
 	mutex_unlock(&data->update_lock);
 
 	return data;
@@ -152,6 +162,9 @@ static int tmp421_read(struct device *dev, enum hwmon_sensor_types type,
 {
 	struct tmp421_data *tmp421 = tmp421_update_device(dev);
 
+	if (tmp421->last_errno)
+		return tmp421->last_errno;
+
 	switch (attr) {
 	case hwmon_temp_input:
 		if (tmp421->config & TMP421_CONFIG_RANGE)
-- 
2.17.1