[PATCH] platform/chrome: cros_hps_i2c: hold the client device for an open file

[PATCH] platform/chrome: cros_hps_i2c: hold the client device for an open file

[Chad Aldreda]

hps_release() finds struct hps_drvdata through file->private_data and
calls pm_runtime_put() on hps->client->dev. hps comes from
devm_kzalloc(), so it is freed when the driver is unbound, and a later
close() of a file opened before the unbind reads freed memory. If the
client was removed with delete_device, the i2c_client may have been
freed as well, and pm_runtime_put() runs on it. KASAN reports a
slab-use-after-free in hps_release() and, after delete_device, further
ones in the runtime PM code.

release() needs only the client device, so take a reference to it in
open() and keep it in file->private_data, and leave hps alone in
release(). hps is still valid in open(): misc_open() calls it with
misc_mtx held, and hps_i2c_remove() calls misc_deregister(), which takes
misc_mtx, before devres frees hps. Unless the driver has been bound
again, runtime PM on the device is disabled by the time such a release()
runs, so pm_runtime_put() drops the usage count and its idle request
fails with -EACCES.

Fixes: 5f9952548d91 ("platform/chrome: add a driver for HPS")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Chad Aldreda <ca@modelmiser.com>
---
Found with the coccinelle script posted at
https://lore.kernel.org/r/20261106193517.3267013-2-ca@modelmiser.com

Reproduced in QEMU 10.2.4 (q35, KASAN). The driver binds to a client
created with new_device on the i801 SMBus adapter, with the "enable"
GPIO supplied by gpio-mockup through a test-only lookup module. A
process opens /dev/cros-hps, the client is unbound or deleted through
sysfs, and the process then closes the file. Without this patch KASAN
reports one use-after-free after an unbind and six after delete_device;
with it there are none, and an unbind followed by a bind works. Not
tested on HPS hardware. Unbinding or deleting the client needs root.

Unchanged by this patch: if the driver is bound again while a file
opened before the unbind is still open, that file's pm_runtime_put()
lands on the new instance's usage count, so the sensor stays powered
until the old file is closed. The counts measured the same with and
without the patch (2, then 1, then 0); before it, that put went through
freed memory.

For stable: before v7.0, hps_release() ends in
"return pm_runtime_put(dev);". There it should end:

	pm_runtime_put(dev);
	put_device(dev);

	return 0;

This is independent of the revocable work for cros_ec_chardev; it only
stops release() from using the driver data.

 drivers/platform/chrome/cros_hps_i2c.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/platform/chrome/cros_hps_i2c.c b/drivers/platform/chrome/cros_hps_i2c.c
index 3b9485627831..290b136ce4fa 100644
--- a/drivers/platform/chrome/cros_hps_i2c.c
+++ b/drivers/platform/chrome/cros_hps_i2c.c
@@ -36,17 +36,26 @@ static int hps_open(struct inode *inode, struct file *file)
 	struct hps_drvdata *hps = container_of(file->private_data,
 					       struct hps_drvdata, misc_device);
 	struct device *dev = &hps->client->dev;
+	int ret;
+
+	ret = pm_runtime_resume_and_get(dev);
+	if (ret)
+		return ret;
 
-	return pm_runtime_resume_and_get(dev);
+	/*
+	 * hps is freed when the driver is unbound, which can happen before
+	 * release(), so release() uses only the device, held here.
+	 */
+	file->private_data = get_device(dev);
+	return 0;
 }
 
 static int hps_release(struct inode *inode, struct file *file)
 {
-	struct hps_drvdata *hps = container_of(file->private_data,
-					       struct hps_drvdata, misc_device);
-	struct device *dev = &hps->client->dev;
+	struct device *dev = file->private_data;
 
 	pm_runtime_put(dev);
+	put_device(dev);
 
 	return 0;
 }
-- 
2.43.0