Re: [PATCH i2c-fixes v1] i2c: core: Fix use-after-free during i2c device removal

[Hillf Danton]

#syz test

A race condition between a process calling i2c_device_probe()
and removal of a USB device by another process leading to both
processes calling debugfs_remove() opens up the possibility of
UAF.

Fix it by adding atomic xchg() and replacing &client->debugfs
with null before calling debugfs_remove() such that the
following statement: "if (IS_ERR_OR_NULL(dentry)) return;"
inside of debugfs_remove() executes properly.

Fixes: d06905d68610 ("i2c: add core-managed per-client directory in debugfs")
Reported-by: syzbot+227dbc9afd022922d624@xxxxxxxxxxxxxxxxxxxxxxxxx
Link: https://syzkaller.appspot.com/bug?extid=227dbc9afd022922d624
Signed-off-by: Rafael Alejandro Diaz Cruz <rafad900@xxxxxxxxx>
---
 drivers/i2c/i2c-core-base.c | 23 ++++++++++++++++++-----
 1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index fb25704219c7..6fe11232f5ee 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -586,8 +586,14 @@ static int i2c_device_probe(struct device *dev)
 		goto err_clear_wakeup_irq;
 	}
 
-	client->debugfs = debugfs_create_dir(dev_name(&client->dev),
-					     client->adapter->debugfs);
+	struct dentry *parent = READ_ONCE(client->adapter->debugfs);
+
+	if (!parent) {
+		status = -ENODEV;
+		goto err_clear_wakeup_irq;
+	}
+
+	client->debugfs = debugfs_create_dir(dev_name(&client->dev), parent);
 
 	if (driver->probe)
 		status = driver->probe(client);
@@ -608,7 +614,10 @@ static int i2c_device_probe(struct device *dev)
 	return 0;
 
 err_release_driver_resources:
-	debugfs_remove_recursive(client->debugfs);
+	// debugfs_remove_recursive(client->debugfs);
+	struct dentry *dir = xchg(&client->debugfs, NULL);
+
+	debugfs_remove_recursive(dir);
 	devres_release_group(&client->dev, client->devres_group_id);
 err_clear_wakeup_irq:
 	dev_pm_clear_wake_irq(&client->dev);
@@ -632,7 +641,9 @@ static void i2c_device_remove(struct device *dev)
 		driver->remove(client);
 	}
 
-	debugfs_remove_recursive(client->debugfs);
+	struct dentry *dir = xchg(&client->debugfs, NULL);
+
+	debugfs_remove_recursive(dir);
 
 	devres_release_group(&client->dev, client->devres_group_id);
 
@@ -1818,6 +1829,8 @@ void i2c_del_adapter(struct i2c_adapter *adap)
 
 	i2c_acpi_remove_space_handler(adap);
 
+	struct dentry *dir = xchg(&adap->debugfs, NULL);
+
 	i2c_deregister_clients(adap);
 
 	/* device name is gone after device_unregister */
@@ -1827,7 +1840,7 @@ void i2c_del_adapter(struct i2c_adapter *adap)
 
 	i2c_host_notify_irq_teardown(adap);
 
-	debugfs_remove_recursive(adap->debugfs);
+	debugfs_remove_recursive(dir);
 
 	/* wait until all references to the device are gone
 	 *
-- 
2.43.0