Forwarded: [PATCH] i2c: core: fix debugfs UAF on adapter removal

[syzbot <syzbot+23ad911c819b923238b7@syzkaller.appspotmail.com>]

For archival purposes, forwarding an incoming command email to
linux-kernel@vger.kernel.org, syzkaller-bugs@googlegroups.com.

***

Subject: [PATCH] i2c: core: fix debugfs UAF on adapter removal
Author: vasilisalmpanis@gmail.com

#syz test

i2c_del_adapter() frees the adapter's debugfs directory before it
unregisters the adapter device, but the new_device sysfs attribute
stays writable until device_del(). A write racing with removal still
reaches i2c_device_probe(), which passes the freed adap->debugfs to
debugfs_create_dir() as the new client's parent:

  BUG: KASAN: slab-use-after-free in lookup_noperm_common+0x407/0x430
  Read of size 4 at addr ffff88803ef87810 by task syz.0.61/6090
   lookup_noperm_common+0x407/0x430
   simple_start_creating+0x9c/0x110
   debugfs_start_creating+0xdb/0x1a0
   debugfs_create_dir+0x24/0x350
   i2c_device_probe+0x814/0xbf0

Such a racing write can also instantiate a client after the client
sweep in i2c_deregister_clients() has run. That client is never
unregistered and its reference on the adapter device would make the
final wait_for_completion() hang.

Close the window by removing the new_device attribute at the start of
i2c_del_adapter(): device_remove_file() drains in-flight writers, so
every client created through this interface is caught by the client
sweep. The attribute group removal in device_del() silently skips the
already-removed file, so no double removal occurs.

With no way left to create clients during teardown, remove the debugfs
directory only once the adapter device has been released.

Fixes: 73febd775bdb ("i2c: create debugfs entry per adapter")
Reported-by: syzbot+23ad911c819b923238b7@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=23ad911c819b923238b7
Signed-off-by: Vasileios Almpanis <vasilisalmpanis@gmail.com>
---
 drivers/i2c/i2c-core-base.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index 3ec04787a737..b7f5dbca9fac 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -1815,6 +1815,13 @@ void i2c_del_adapter(struct i2c_adapter *adap)
 		return;
 	}
 
+	/*
+	 * Prevent new clients from being instantiated via sysfs while the
+	 * adapter is torn down. This drains any in-flight writers, so all
+	 * clients will be caught by i2c_deregister_clients() below.
+	 */
+	device_remove_file(&adap->dev, &dev_attr_new_device);
+
 	i2c_acpi_remove_space_handler(adap);
 
 	i2c_deregister_clients(adap);
@@ -1826,8 +1833,6 @@ void i2c_del_adapter(struct i2c_adapter *adap)
 
 	i2c_host_notify_irq_teardown(adap);
 
-	debugfs_remove_recursive(adap->debugfs);
-
 	/* wait until all references to the device are gone
 	 *
 	 * FIXME: This is old code and should ideally be replaced by an
@@ -1839,6 +1844,9 @@ void i2c_del_adapter(struct i2c_adapter *adap)
 	device_unregister(&adap->dev);
 	wait_for_completion(&adap->dev_released);
 
+	/* clients use this directory as their debugfs parent */
+	debugfs_remove_recursive(adap->debugfs);
+
 	/* free bus id */
 	mutex_lock(&core_lock);
 	idr_remove(&i2c_adapter_idr, adap->nr);
-- 
2.47.3