ipmi: Fix I2C client removal in the SSIF driver
authorCorey Minyard <cminyard@mvista.com>
Thu, 30 Aug 2018 18:06:21 +0000 (13:06 -0500)
committerCorey Minyard <cminyard@mvista.com>
Fri, 31 Aug 2018 13:43:12 +0000 (08:43 -0500)
The SSIF driver was removing any client that came in through the
platform interface, but it should only remove clients that it
added.  On a failure in the probe function, this could result
in the following oops when the driver is removed and the
client gets unregistered twice:

 CPU: 107 PID: 30266 Comm: rmmod Not tainted 4.18.0+ #80
 Hardware name: Cavium Inc. Saber/Saber, BIOS Cavium reference firmware version 7.0 08/04/2018
 pstate: 60400009 (nZCv daif +PAN -UAO)
 pc : kernfs_find_ns+0x28/0x120
 lr : kernfs_find_and_get_ns+0x40/0x60
 sp : ffff00002310fb50
 x29: ffff00002310fb50 x28: ffff800a8240f800
 x27: 0000000000000000 x26: 0000000000000000
 x25: 0000000056000000 x24: ffff000009073000
 x23: ffff000008998b38 x22: 0000000000000000
 x21: ffff800ed86de820 x20: 0000000000000000
 x19: ffff00000913a1d8 x18: 0000000000000000
 x17: 0000000000000000 x16: 0000000000000000
 x15: 0000000000000000 x14: 5300737265766972
 x13: 643d4d4554535953 x12: 0000000000000030
 x11: 0000000000000030 x10: 0101010101010101
 x9 : ffff800ea06cc3f9 x8 : 0000000000000000
 x7 : 0000000000000141 x6 : ffff000009073000
 x5 : ffff800adb706b00 x4 : 0000000000000000
 x3 : 00000000ffffffff x2 : 0000000000000000
 x1 : ffff000008998b38 x0 : ffff000008356760
 Process rmmod (pid: 30266, stack limit = 0x00000000e218418d)
 Call trace:
  kernfs_find_ns+0x28/0x120
  kernfs_find_and_get_ns+0x40/0x60
  sysfs_unmerge_group+0x2c/0x6c
  dpm_sysfs_remove+0x34/0x70
  device_del+0x58/0x30c
  device_unregister+0x30/0x7c
  i2c_unregister_device+0x84/0x90 [i2c_core]
  ssif_platform_remove+0x38/0x98 [ipmi_ssif]
  platform_drv_remove+0x2c/0x6c
  device_release_driver_internal+0x168/0x1f8
  driver_detach+0x50/0xbc
  bus_remove_driver+0x74/0xe8
  driver_unregister+0x34/0x5c
  platform_driver_unregister+0x20/0x2c
  cleanup_ipmi_ssif+0x50/0xd82c [ipmi_ssif]
  __arm64_sys_delete_module+0x1b4/0x220
  el0_svc_handler+0x104/0x160
  el0_svc+0x8/0xc
 Code: aa1e03e0 aa0203f6 aa0103f7 d503201f (7940e280)
 ---[ end trace 09f0e34cce8e2d8c ]---
 Kernel panic - not syncing: Fatal exception
 SMP: stopping secondary CPUs
 Kernel Offset: disabled
 CPU features: 0x23800c38

So track the clients that the SSIF driver adds and only remove
those.

Reported-by: George Cherian <george.cherian@cavium.com>
Signed-off-by: Corey Minyard <cminyard@mvista.com>
Tested-by: George Cherian <george.cherian@cavium.com>
Cc: <stable@vger.kernel.org> # 4.14.x
drivers/char/ipmi/ipmi_ssif.c

index c12edc8e91dfd50361d9dd53cd9735cb5dd86249..265d6a6583bc547971ad41e009906f5c95163b65 100644 (file)
@@ -181,6 +181,8 @@ struct ssif_addr_info {
        struct device *dev;
        struct i2c_client *client;
 
+       struct i2c_client *added_client;
+
        struct mutex clients_mutex;
        struct list_head clients;
 
@@ -1641,15 +1643,7 @@ static int ssif_probe(struct i2c_client *client, const struct i2c_device_id *id)
 
  out:
        if (rv) {
-               /*
-                * Note that if addr_info->client is assigned, we
-                * leave it.  The i2c client hangs around even if we
-                * return a failure here, and the failure here is not
-                * propagated back to the i2c code.  This seems to be
-                * design intent, strange as it may be.  But if we
-                * don't leave it, ssif_platform_remove will not remove
-                * the client like it should.
-                */
+               addr_info->client = NULL;
                dev_err(&client->dev, "Unable to start IPMI SSIF: %d\n", rv);
                kfree(ssif_info);
        }
@@ -1669,7 +1663,8 @@ static int ssif_adapter_handler(struct device *adev, void *opaque)
        if (adev->type != &i2c_adapter_type)
                return 0;
 
-       i2c_new_device(to_i2c_adapter(adev), &addr_info->binfo);
+       addr_info->added_client = i2c_new_device(to_i2c_adapter(adev),
+                                                &addr_info->binfo);
 
        if (!addr_info->adapter_name)
                return 1; /* Only try the first I2C adapter by default. */
@@ -1842,7 +1837,7 @@ static int ssif_platform_remove(struct platform_device *dev)
                return 0;
 
        mutex_lock(&ssif_infos_mutex);
-       i2c_unregister_device(addr_info->client);
+       i2c_unregister_device(addr_info->added_client);
 
        list_del(&addr_info->link);
        kfree(addr_info);