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

Hillf Danton <[email protected]>
Newsgroups org.kernel.vger.linux-i2c,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
#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: [email protected]
Link: https://syzkaller.appspot.com/bug?extid=227dbc9afd022922d624
Signed-off-by: Rafael Alejandro Diaz Cruz <[email protected]>
---
 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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.