diff mbox series

hwmon: pmbus: protect read-modify-write with lock

Message ID 20190528090746.GA31184@localhost.localdomain
State Not Applicable
Headers show
Series hwmon: pmbus: protect read-modify-write with lock | expand

Commit Message

Adamski, Krzysztof (Nokia - PL/Wroclaw) May 28, 2019, 9:08 a.m. UTC
The operation done in the pmbus_update_fan() function is a
read-modify-write operation but it lacks any kind of lock protection
which may cause problems if run more than once simultaneously. This
patch uses an existing update_lock mutex to fix this problem.

Signed-off-by: Krzysztof Adamski <krzysztof.adamski@nokia.com>
---
 drivers/hwmon/pmbus/pmbus_core.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

Comments

Wolfram Sang June 7, 2019, 10:32 p.m. UTC | #1
On Tue, May 28, 2019 at 09:08:21AM +0000, Adamski, Krzysztof (Nokia - PL/Wroclaw) wrote:
> The operation done in the pmbus_update_fan() function is a
> read-modify-write operation but it lacks any kind of lock protection
> which may cause problems if run more than once simultaneously. This
> patch uses an existing update_lock mutex to fix this problem.
> 
> Signed-off-by: Krzysztof Adamski <krzysztof.adamski@nokia.com>

Please use get_maintainer to find the people responsible for this file.
It is not my realm.

> +out:
> +	mutex_lock(&data->update_lock);

Despite the above, have you tested the code? This likely should be
mutex_unlock?
Adamski, Krzysztof (Nokia - PL/Wroclaw) June 10, 2019, 6:36 a.m. UTC | #2
On Sat, Jun 08, 2019 at 12:32:18AM +0200, Wolfram Sang wrote:
>On Tue, May 28, 2019 at 09:08:21AM +0000, Adamski, Krzysztof (Nokia - PL/Wroclaw) wrote:
>> The operation done in the pmbus_update_fan() function is a
>> read-modify-write operation but it lacks any kind of lock protection
>> which may cause problems if run more than once simultaneously. This
>> patch uses an existing update_lock mutex to fix this problem.
>>
>> Signed-off-by: Krzysztof Adamski <krzysztof.adamski@nokia.com>
>
>Please use get_maintainer to find the people responsible for this file.
>It is not my realm.
>
>> +out:
>> +	mutex_lock(&data->update_lock);
>
>Despite the above, have you tested the code? This likely should be
>mutex_unlock?
>

Sorry for that, I made a mistake and used wrong command to generate the
patch and send it to the wrong list of people. You can ignore this
patchset, it was resent to the proper mailing list instead.

Krzysztof
Wolfram Sang June 10, 2019, 7:04 a.m. UTC | #3
Hi Krzysztof,

> patch and send it to the wrong list of people. You can ignore this
> patchset, it was resent to the proper mailing list instead.

Thanks for the heads up.

Regards,

   Wolfram
diff mbox series

Patch

diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
index ef7ee90ee785..94adbede7912 100644
--- a/drivers/hwmon/pmbus/pmbus_core.c
+++ b/drivers/hwmon/pmbus/pmbus_core.c
@@ -268,6 +268,7 @@  int pmbus_update_fan(struct i2c_client *client, int page, int id,
 	int rv;
 	u8 to;
 
+	mutex_lock(&data->update_lock);
 	from = pmbus_read_byte_data(client, page,
 				    pmbus_fan_config_registers[id]);
 	if (from < 0)
@@ -278,11 +279,15 @@  int pmbus_update_fan(struct i2c_client *client, int page, int id,
 		rv = pmbus_write_byte_data(client, page,
 					   pmbus_fan_config_registers[id], to);
 		if (rv < 0)
-			return rv;
+			goto out;
 	}
 
-	return _pmbus_write_word_data(client, page,
-				      pmbus_fan_command_registers[id], command);
+	rv = _pmbus_write_word_data(client, page,
+				    pmbus_fan_command_registers[id], command);
+
+out:
+	mutex_lock(&data->update_lock);
+	return rv;
 }
 EXPORT_SYMBOL_GPL(pmbus_update_fan);