]> git.ipfire.org Git - thirdparty/kernel/linux.git/commitdiff
eeprom: ee1004: Fix locking issues in ee1004_probe()
authorArmin Wolf <W_Armin@gmx.de>
Sat, 13 Jul 2024 23:48:13 +0000 (01:48 +0200)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Wed, 31 Jul 2024 11:41:42 +0000 (13:41 +0200)
Currently, the devres-based management of ee1004_bus_data has
several issues when it comes to locking:

1. It does not call mutex_unlock() before returning an error.

2. When encountering an error, it deadlocks when trying to recursively
   lock a mutex.

Fix this by moving the mutex-protected bus data initialization into
a separate function so that devm_add_action_or_reset() is called
without the mutex being held.

Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Fixes: 55d57ef6fa97 ("eeprom: ee1004: Use devres for bus data cleanup")
Signed-off-by: Armin Wolf <W_Armin@gmx.de>
Reviewed-by: Dan Carpenter <dan.carpenter@linaro.org>
Link: https://lore.kernel.org/r/20240713234813.21746-1-W_Armin@gmx.de
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
drivers/misc/eeprom/ee1004.c

index d4aeeb2b2169204183ddfeeafcbb58691222b24b..89224d4af4a201b0e6b7a4437cc90eb95c6e7b11 100644 (file)
@@ -233,6 +233,49 @@ static void ee1004_cleanup_bus_data(void *data)
        mutex_unlock(&ee1004_bus_lock);
 }
 
+static int ee1004_init_bus_data(struct i2c_client *client)
+{
+       struct ee1004_bus_data *bd;
+       int err, cnr = 0;
+
+       bd = ee1004_get_bus_data(client->adapter);
+       if (!bd)
+               return dev_err_probe(&client->dev, -ENOSPC, "Only %d busses supported",
+                                    EE1004_MAX_BUSSES);
+
+       i2c_set_clientdata(client, bd);
+
+       if (++bd->dev_count == 1) {
+               /* Use 2 dummy devices for page select command */
+               for (cnr = 0; cnr < EE1004_NUM_PAGES; cnr++) {
+                       struct i2c_client *cl;
+
+                       cl = i2c_new_dummy_device(client->adapter, EE1004_ADDR_SET_PAGE + cnr);
+                       if (IS_ERR(cl)) {
+                               err = PTR_ERR(cl);
+                               goto err_out;
+                       }
+
+                       bd->set_page[cnr] = cl;
+               }
+
+               /* Remember current page to avoid unneeded page select */
+               err = ee1004_get_current_page(bd);
+               if (err < 0)
+                       goto err_out;
+
+               dev_dbg(&client->dev, "Currently selected page: %d\n", err);
+               bd->current_page = err;
+       }
+
+       return 0;
+
+err_out:
+       ee1004_cleanup(cnr, bd);
+
+       return err;
+}
+
 static int ee1004_probe(struct i2c_client *client)
 {
        struct nvmem_config config = {
@@ -251,9 +294,8 @@ static int ee1004_probe(struct i2c_client *client)
                .compat = true,
                .base_dev = &client->dev,
        };
-       struct ee1004_bus_data *bd;
        struct nvmem_device *ndev;
-       int err, cnr = 0;
+       int err;
 
        /* Make sure we can operate on this adapter */
        if (!i2c_check_functionality(client->adapter,
@@ -264,46 +306,21 @@ static int ee1004_probe(struct i2c_client *client)
 
        mutex_lock(&ee1004_bus_lock);
 
-       bd = ee1004_get_bus_data(client->adapter);
-       if (!bd) {
+       err = ee1004_init_bus_data(client);
+       if (err < 0) {
                mutex_unlock(&ee1004_bus_lock);
-               return dev_err_probe(&client->dev, -ENOSPC,
-                                    "Only %d busses supported", EE1004_MAX_BUSSES);
-       }
-
-       err = devm_add_action_or_reset(&client->dev, ee1004_cleanup_bus_data, bd);
-       if (err < 0)
                return err;
-
-       i2c_set_clientdata(client, bd);
-
-       if (++bd->dev_count == 1) {
-               /* Use 2 dummy devices for page select command */
-               for (cnr = 0; cnr < EE1004_NUM_PAGES; cnr++) {
-                       struct i2c_client *cl;
-
-                       cl = i2c_new_dummy_device(client->adapter, EE1004_ADDR_SET_PAGE + cnr);
-                       if (IS_ERR(cl)) {
-                               mutex_unlock(&ee1004_bus_lock);
-                               return PTR_ERR(cl);
-                       }
-                       bd->set_page[cnr] = cl;
-               }
-
-               /* Remember current page to avoid unneeded page select */
-               err = ee1004_get_current_page(bd);
-               if (err < 0) {
-                       mutex_unlock(&ee1004_bus_lock);
-                       return err;
-               }
-               dev_dbg(&client->dev, "Currently selected page: %d\n", err);
-               bd->current_page = err;
        }
 
        ee1004_probe_temp_sensor(client);
 
        mutex_unlock(&ee1004_bus_lock);
 
+       err = devm_add_action_or_reset(&client->dev, ee1004_cleanup_bus_data,
+                                      i2c_get_clientdata(client));
+       if (err < 0)
+               return err;
+
        ndev = devm_nvmem_register(&client->dev, &config);
        if (IS_ERR(ndev))
                return PTR_ERR(ndev);