Commit 888e989a authored by Corey Minyard's avatar Corey Minyard Committed by Greg Kroah-Hartman

ipmi: Fix I2C client removal in the SSIF driver

commit 0745dde6 upstream.

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:
  i2c_unregister_device+0x84/0x90 [i2c_core]
  ssif_platform_remove+0x38/0x98 [ipmi_ssif]
  cleanup_ipmi_ssif+0x50/0xd82c [ipmi_ssif]
 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
Reported-by: default avatarGeorge Cherian <>
Signed-off-by: default avatarCorey Minyard <>
Tested-by: default avatarGeorge Cherian <>
Cc: <> # 4.14.x
Signed-off-by: default avatarGreg Kroah-Hartman <>
parent f6e23e57
...@@ -184,6 +184,8 @@ struct ssif_addr_info { ...@@ -184,6 +184,8 @@ struct ssif_addr_info {
struct device *dev; struct device *dev;
struct i2c_client *client; struct i2c_client *client;
struct i2c_client *added_client;
struct mutex clients_mutex; struct mutex clients_mutex;
struct list_head clients; struct list_head clients;
...@@ -1710,15 +1712,7 @@ static int ssif_probe(struct i2c_client *client, const struct i2c_device_id *id) ...@@ -1710,15 +1712,7 @@ static int ssif_probe(struct i2c_client *client, const struct i2c_device_id *id)
out: out:
if (rv) { if (rv) {
/* addr_info->client = NULL;
* 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.
dev_err(&client->dev, "Unable to start IPMI SSIF: %d\n", rv); dev_err(&client->dev, "Unable to start IPMI SSIF: %d\n", rv);
kfree(ssif_info); kfree(ssif_info);
} }
...@@ -1737,7 +1731,8 @@ static int ssif_adapter_handler(struct device *adev, void *opaque) ...@@ -1737,7 +1731,8 @@ static int ssif_adapter_handler(struct device *adev, void *opaque)
if (adev->type != &i2c_adapter_type) if (adev->type != &i2c_adapter_type)
return 0; return 0;
i2c_new_device(to_i2c_adapter(adev), &addr_info->binfo); addr_info->added_client = i2c_new_device(to_i2c_adapter(adev),
if (!addr_info->adapter_name) if (!addr_info->adapter_name)
return 1; /* Only try the first I2C adapter by default. */ return 1; /* Only try the first I2C adapter by default. */
...@@ -2018,8 +2013,8 @@ static int ssif_platform_remove(struct platform_device *dev) ...@@ -2018,8 +2013,8 @@ static int ssif_platform_remove(struct platform_device *dev)
return 0; return 0;
mutex_lock(&ssif_infos_mutex); mutex_lock(&ssif_infos_mutex);
if (addr_info->client) if (addr_info->added_client)
i2c_unregister_device(addr_info->client); i2c_unregister_device(addr_info->added_client);
list_del(&addr_info->link); list_del(&addr_info->link);
kfree(addr_info); kfree(addr_info);
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment