| Message ID | 20190530064509.GA13789@localhost.localdomain (mailing list archive) |
|---|---|
| State | Rejected |
| Headers | show |
| Series | hwmon: pmbus: protect read-modify-write with lock | expand |
Hi! On 30/05/2019 08:45, 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> > --- > > I'm resending this patch to proper recipients this time. Sorry if the > previous submission confused anybody. > > drivers/hwmon/pmbus/pmbus_core.c | 11 ++++++++--- > 1 file changed, 8 insertions(+), 3 deletions(-) > > 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); This has to be mutex_unlock()... > + return rv; > } > EXPORT_SYMBOL_GPL(pmbus_update_fan);
On Mon, Jun 03, 2019 at 01:11:45PM +0000, Sverdlin, Alexander (Nokia - DE/Ulm) wrote: > Hi! > > On 30/05/2019 08:45, 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> > > --- > > > > I'm resending this patch to proper recipients this time. Sorry if the > > previous submission confused anybody. > > > > drivers/hwmon/pmbus/pmbus_core.c | 11 ++++++++--- > > 1 file changed, 8 insertions(+), 3 deletions(-) > > > > 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); > > This has to be mutex_unlock()... > Not only that - it is also recursive. Guenter
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);
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> --- I'm resending this patch to proper recipients this time. Sorry if the previous submission confused anybody. drivers/hwmon/pmbus/pmbus_core.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)