Skip to content

Commit 888e989

Browse files
cminyardgregkh
authored andcommitted
ipmi: Fix I2C client removal in the SSIF driver
commit 0745dde 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: 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 <[email protected]> Signed-off-by: Corey Minyard <[email protected]> Tested-by: George Cherian <[email protected]> Cc: <[email protected]> # 4.14.x Signed-off-by: Greg Kroah-Hartman <[email protected]>
1 parent f6e23e5 commit 888e989

File tree

1 file changed

+7
-12
lines changed

1 file changed

+7
-12
lines changed

drivers/char/ipmi/ipmi_ssif.c

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,8 @@ struct ssif_addr_info {
184184
struct device *dev;
185185
struct i2c_client *client;
186186

187+
struct i2c_client *added_client;
188+
187189
struct mutex clients_mutex;
188190
struct list_head clients;
189191

@@ -1710,15 +1712,7 @@ static int ssif_probe(struct i2c_client *client, const struct i2c_device_id *id)
17101712

17111713
out:
17121714
if (rv) {
1713-
/*
1714-
* Note that if addr_info->client is assigned, we
1715-
* leave it. The i2c client hangs around even if we
1716-
* return a failure here, and the failure here is not
1717-
* propagated back to the i2c code. This seems to be
1718-
* design intent, strange as it may be. But if we
1719-
* don't leave it, ssif_platform_remove will not remove
1720-
* the client like it should.
1721-
*/
1715+
addr_info->client = NULL;
17221716
dev_err(&client->dev, "Unable to start IPMI SSIF: %d\n", rv);
17231717
kfree(ssif_info);
17241718
}
@@ -1737,7 +1731,8 @@ static int ssif_adapter_handler(struct device *adev, void *opaque)
17371731
if (adev->type != &i2c_adapter_type)
17381732
return 0;
17391733

1740-
i2c_new_device(to_i2c_adapter(adev), &addr_info->binfo);
1734+
addr_info->added_client = i2c_new_device(to_i2c_adapter(adev),
1735+
&addr_info->binfo);
17411736

17421737
if (!addr_info->adapter_name)
17431738
return 1; /* Only try the first I2C adapter by default. */
@@ -2018,8 +2013,8 @@ static int ssif_platform_remove(struct platform_device *dev)
20182013
return 0;
20192014

20202015
mutex_lock(&ssif_infos_mutex);
2021-
if (addr_info->client)
2022-
i2c_unregister_device(addr_info->client);
2016+
if (addr_info->added_client)
2017+
i2c_unregister_device(addr_info->added_client);
20232018

20242019
list_del(&addr_info->link);
20252020
kfree(addr_info);

0 commit comments

Comments
 (0)