[PATCH] mfd: mt6370: fix potential null-ptr-deref and uaf while removing device

Yang Yingliang yangyingliang at huawei.com
Mon Nov 28 06:36:01 PST 2022


I got the a UAF and a null-ptr-deref reports while
doing module loading test:

==================================================================
BUG: KASAN: use-after-free in regulator_register.cold.70+0x191/0xe78
Read of size 8 at addr ffff888118fb8c28 by task systemd-udevd/261

CPU: 2 PID: 261 Comm: systemd-udevd Tainted: G                 N 6.1.0-rc3+
Call Trace:
 <TASK>
 dump_stack_lvl+0x67/0x83
 print_report+0x178/0x4b0
 kasan_report+0x90/0x190
 regulator_register.cold.70+0x191/0xe78
 devm_regulator_register+0x57/0xb0
 mt6370_regulator_probe.cold.1+0xd8/0x222 [mt6370_regulator]

Allocated by task 261:
 kasan_save_stack+0x1c/0x40
 kasan_set_track+0x21/0x30
 __kasan_kmalloc+0x7e/0x90
 __kmalloc_node_track_caller+0x55/0x1b0
 devm_kmalloc+0x5e/0x110
 of_get_regulator_init_data+0x9e/0xd60
 regulator_of_get_init_data+0x1a3/0x270
 regulator_register+0x227/0x620
 devm_regulator_register+0x57/0xb0
 mt6370_regulator_probe.cold.1+0xd8/0x222 [mt6370_regulator]

Freed by task 250:
 kasan_save_stack+0x1c/0x40
 kasan_set_track+0x21/0x30
 kasan_save_free_info+0x2a/0x50
 __kasan_slab_free+0x102/0x190
 __kmem_cache_free+0xca/0x400
 release_nodes+0x9b/0xb9
 devres_release_group.cold.10+0x5f/0x64
 i2c_device_remove+0x60/0xf0
 device_remove+0x73/0xc0
 device_release_driver_internal+0x12d/0x210
 bus_remove_device+0x1bd/0x240
 device_del+0x357/0x770
 i2c_unregister_device.part.22+0xe2/0x100
 i2c_unregister_device+0x29/0x30
==================================================================

Here is how UAF happens:

CPU A				|CPU B
mt6370_probe()			|
  devm_mfd_add_devices()	|
				|mt6370_regulator_probe()
				|  regulator_register()
				|    //allocate init_data and add it to devres
				|    regulator_of_get_init_data()
i2c_unregister_device()		|
  device_del()			|
    devres_release_all()	|
      // init_data is freed	|
      release_nodes()		|
				|  // using init_data causes UAF
				|  regulator_register()

While probing mt6370 regulator, the init_data is allocated in
regulator_of_get_init_data(), and it's added to devres of
device mt6370. When the device mt6370 is deleting, the devres
is freed, then it causes a UAF in regulator_register().

==================================================================
BUG: kernel NULL pointer dereference, address: 0000000000000354
CPU: 2 PID: 261 Comm: systemd-udevd Tainted: G    B            N 6.1.0-rc3+
RIP: 0010:regmap_read+0x21/0xc0
Call Trace:
 <TASK>
 regulator_get_voltage_sel_regmap+0x98/0x110
 regulator_attr_is_visible+0x2da/0x4c0
 internal_create_group+0x1f5/0x6b0
 internal_create_groups.part.4+0x65/0xe0
 sysfs_create_groups+0x24/0x50
 device_add+0x5a9/0x1100
 regulator_register.cold.70+0xb06/0xe78
 devm_regulator_register+0x57/0xb0
 mt6370_regulator_probe.cold.1+0xd8/0x222 [mt6370_regulator]
==================================================================

Here is how null-ptr-deref happens:

CPU A				|CPU B
mt6370_probe()			|
  // add regmap to devres	|
  devm_regmap_init()		|
  devm_mfd_add_devices()	|
				|
i2c_unregister_device()		|
  device_del()			|
    devres_release_all()	|
      // regmap is removed	|
      remove_nodes()		|
				|mt6370_regulator_probe()
				|  regulator_register()
				|  // can't find regmap, it's NULL
				|  devm_get_regmap()
				|  device_add()
				|    // causes null-ptr-deref
				|    regulator_get_voltage_sel_regmap()

While probing mt6370 regulator, the mt6370 is deleting, the devres
are removed including regmap, so in regulator_register() get a NULL
regmap pointer, then it causes null-ptr-derf while using it in
regulator_get_voltage_sel_regmap().

To fix theses races, switch to use non-devm function to add mfd
devices, make removing child(regulator) devices earlier than
freeing parent device resources, so the devres can not be freed
until mt6370_regulator_probe() is finished.

Fixes: b2adf788e603 ("mfd: mt6370: Add MediaTek MT6370 support")
Signed-off-by: Yang Yingliang <yangyingliang at huawei.com>
---
 drivers/mfd/mt6370.c | 34 ++++++++++++++++++++--------------
 1 file changed, 20 insertions(+), 14 deletions(-)

diff --git a/drivers/mfd/mt6370.c b/drivers/mfd/mt6370.c
index cf19cce2fdc0..dd19146612cf 100644
--- a/drivers/mfd/mt6370.c
+++ b/drivers/mfd/mt6370.c
@@ -268,28 +268,33 @@ static int mt6370_probe(struct i2c_client *i2c)
 	switch (vid) {
 	case MT6370_VENID_MT6372P:
 	case MT6370_VENID_MT6372CP:
-		ret = devm_mfd_add_devices(dev, PLATFORM_DEVID_AUTO,
-					   mt6372_exclusive_devices,
-					   ARRAY_SIZE(mt6372_exclusive_devices),
-					   NULL, 0,
-					   regmap_irq_get_domain(info->irq_data));
+		ret = mfd_add_devices(dev, PLATFORM_DEVID_AUTO,
+				      mt6372_exclusive_devices,
+				      ARRAY_SIZE(mt6372_exclusive_devices),
+				      NULL, 0,
+				      regmap_irq_get_domain(info->irq_data));
 		break;
 	default:
-		ret = devm_mfd_add_devices(dev, PLATFORM_DEVID_AUTO,
-					   mt6370_exclusive_devices,
-					   ARRAY_SIZE(mt6370_exclusive_devices),
-					   NULL, 0,
-					   regmap_irq_get_domain(info->irq_data));
+		ret = mfd_add_devices(dev, PLATFORM_DEVID_AUTO,
+				      mt6370_exclusive_devices,
+				      ARRAY_SIZE(mt6370_exclusive_devices),
+				      NULL, 0,
+				      regmap_irq_get_domain(info->irq_data));
 		break;
 	}
 
 	if (ret)
 		return dev_err_probe(dev, ret, "Failed to add the exclusive devices\n");
 
-	return devm_mfd_add_devices(dev, PLATFORM_DEVID_AUTO,
-				    mt6370_devices, ARRAY_SIZE(mt6370_devices),
-				    NULL, 0,
-				    regmap_irq_get_domain(info->irq_data));
+	return mfd_add_devices(dev, PLATFORM_DEVID_AUTO,
+			       mt6370_devices, ARRAY_SIZE(mt6370_devices),
+			       NULL, 0,
+			       regmap_irq_get_domain(info->irq_data));
+}
+
+static void mt6370_remove(struct i2c_client *i2c)
+{
+	mfd_remove_devices(&i2c->dev);
 }
 
 static const struct of_device_id mt6370_match_table[] = {
@@ -304,6 +309,7 @@ static struct i2c_driver mt6370_driver = {
 		.of_match_table = mt6370_match_table,
 	},
 	.probe_new = mt6370_probe,
+	.remove = mt6370_remove,
 };
 module_i2c_driver(mt6370_driver);
 
-- 
2.25.1




More information about the Linux-mediatek mailing list