[PATCH v5 1/4] hwmon: (pmbus) add missing mutex_lock in regulator ops

[Pradhan, Sanman]

From: Sanman Pradhan <psanman@xxxxxxxxxxx>

The regulator voltage operations pmbus_regulator_get_voltage(),
pmbus_regulator_set_voltage(), and pmbus_regulator_list_voltage()
call PMBus access helpers without holding update_lock. These helpers
perform multi-step PMBus transactions involving shared core state such
as page selection and transaction timing. Without serialization, a
concurrent PMBus access can interleave with those operations and cause
reads from or writes to the wrong rail.

For set_voltage(), this is particularly dangerous because the
VOUT_COMMAND write could be directed to the wrong regulator output.

Add mutex_lock/unlock around the affected regulator voltage paths,
following the pattern already used by other PMBus regulator operations
such as _pmbus_regulator_on_off() and pmbus_regulator_get_status().

Signed-off-by: Sanman Pradhan <psanman@xxxxxxxxxxx>
---
v5:
- New patch in the series. Adds the missing update_lock mutex to the
  three regulator voltage ops that were missing serialization.
---
 drivers/hwmon/pmbus/pmbus_core.c | 46 ++++++++++++++++++++++++--------
 1 file changed, 35 insertions(+), 11 deletions(-)

diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
index 4d7634ee61484..3dad455448d05 100644
--- a/drivers/hwmon/pmbus/pmbus_core.c
+++ b/drivers/hwmon/pmbus/pmbus_core.c
@@ -3181,7 +3181,9 @@ static int pmbus_regulator_get_voltage(struct regulator_dev *rdev)
 		.convert = true,
 	};
 
+	mutex_lock(&data->update_lock);
 	s.data = _pmbus_read_word_data(client, s.page, 0xff, PMBUS_READ_VOUT);
+	mutex_unlock(&data->update_lock);
 	if (s.data < 0)
 		return s.data;
 
@@ -3202,16 +3204,23 @@ static int pmbus_regulator_set_voltage(struct regulator_dev *rdev, int min_uv,
 	};
 	int val = DIV_ROUND_CLOSEST(min_uv, 1000); /* convert to mV */
 	int low, high;
+	int ret;
 
 	*selector = 0;
 
+	mutex_lock(&data->update_lock);
+
 	low = pmbus_regulator_get_low_margin(client, s.page);
-	if (low < 0)
-		return low;
+	if (low < 0) {
+		ret = low;
+		goto unlock;
+	}
 
 	high = pmbus_regulator_get_high_margin(client, s.page);
-	if (high < 0)
-		return high;
+	if (high < 0) {
+		ret = high;
+		goto unlock;
+	}
 
 	/* Make sure we are within margins */
 	if (low > val)
@@ -3221,7 +3230,11 @@ static int pmbus_regulator_set_voltage(struct regulator_dev *rdev, int min_uv,
 
 	val = pmbus_data2reg(data, &s, val);
 
-	return _pmbus_write_word_data(client, s.page, PMBUS_VOUT_COMMAND, (u16)val);
+	ret = _pmbus_write_word_data(client, s.page, PMBUS_VOUT_COMMAND, (u16)val);
+
+unlock:
+	mutex_unlock(&data->update_lock);
+	return ret;
 }
 
 static int pmbus_regulator_list_voltage(struct regulator_dev *rdev,
@@ -3231,6 +3244,7 @@ static int pmbus_regulator_list_voltage(struct regulator_dev *rdev,
 	struct i2c_client *client = to_i2c_client(dev->parent);
 	struct pmbus_data *data = i2c_get_clientdata(client);
 	int val, low, high;
+	int ret;
 
 	if (data->flags & PMBUS_VOUT_PROTECTED)
 		return 0;
@@ -3243,18 +3257,28 @@ static int pmbus_regulator_list_voltage(struct regulator_dev *rdev,
 	val = DIV_ROUND_CLOSEST(rdev->desc->min_uV +
 				(rdev->desc->uV_step * selector), 1000); /* convert to mV */
 
+	mutex_lock(&data->update_lock);
+
 	low = pmbus_regulator_get_low_margin(client, rdev_get_id(rdev));
-	if (low < 0)
-		return low;
+	if (low < 0) {
+		ret = low;
+		goto unlock;
+	}
 
 	high = pmbus_regulator_get_high_margin(client, rdev_get_id(rdev));
-	if (high < 0)
-		return high;
+	if (high < 0) {
+		ret = high;
+		goto unlock;
+	}
 
 	if (val >= low && val <= high)
-		return val * 1000; /* unit is uV */
+		ret = val * 1000; /* unit is uV */
+	else
+		ret = 0;
 
-	return 0;
+unlock:
+	mutex_unlock(&data->update_lock);
+	return ret;
 }
 
 const struct regulator_ops pmbus_regulator_ops = {
-- 
2.34.1