[PATCH 0/8] char: misc: Introduce misc_sync to fix UAF

[PATCH 0/8] char: misc: Introduce misc_sync to fix UAF

[Tzung-Bi Shih]

This series introduces misc_sync to address potential Use-After-Free
errors when a device is deregistered while file operations are still in
progress or files remain open.  It then adopts the mechanism in
cros_ec_chardev driver.

- Patches 1 and 2 are independent refactors in the misc subsystem.
  These are safe cleanups and should be fine to apply.

- Patch 3 is the initial Proof-of-Concept for misc_sync.  It uses a
  global lock to serialize file operations, so a performance downgrade
  is expected.

- Patch 4 is an improvement over the PoC that replaces the global lock
  with SRCU for list traversal in the file operations, avoiding
  performance bottlenecks.

- Patches 5 to 7 are preparation steps in cros_ec_chardev driver.  They
  introduce reference counting for platform driver data and an event
  relayer.  This removes the direct access to `ec_dev->event_notifier`
  in `cros_ec_chardev_release()`, which is a prerequisite for using
  misc_sync safely in this driver.

- Patch 8 switches cros_ec_chardev driver to use misc_sync.

Tzung-Bi Shih (8):
  char: misc: Simplify locking with guard()
  char: misc: Introduce misc_find() helper
  char: misc: Introduce misc_sync_register()
  char: misc: Use SRCU to protect list traversal
  platform/chrome: cros_ec_chardev: Introduce chardev_data
  platform/chrome: cros_ec_chardev: Move data to chardev_pdata
  platform/chrome: cros_ec_chardev: Add event relayer
  platform/chrome: cros_ec_chardev: Use misc_sync_register()

 drivers/char/misc.c                       | 228 +++++++++++++++++-----
 drivers/platform/chrome/cros_ec_chardev.c | 113 ++++++++---
 include/linux/miscdevice.h                |  10 +
 3 files changed, 272 insertions(+), 79 deletions(-)

-- 
2.51.0

[PATCH 1/8] char: misc: Simplify locking with guard()

[Tzung-Bi Shih]

Use guard() to simplify locking in misc_{de,}register().

Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
 drivers/char/misc.c | 32 +++++++++++---------------------
 1 file changed, 11 insertions(+), 21 deletions(-)

diff --git a/drivers/char/misc.c b/drivers/char/misc.c
index 726516fb0a3b..e2bea4f01851 100644
--- a/drivers/char/misc.c
+++ b/drivers/char/misc.c
@@ -36,6 +36,7 @@
 
 #include <linux/module.h>
 
+#include <linux/cleanup.h>
 #include <linux/fs.h>
 #include <linux/errno.h>
 #include <linux/miscdevice.h>
@@ -209,7 +210,6 @@ static const struct file_operations misc_fops = {
 int misc_register(struct miscdevice *misc)
 {
 	dev_t dev;
-	int err = 0;
 	bool is_dynamic = (misc->minor == MISC_DYNAMIC_MINOR);
 
 	if (misc->minor > MISC_DYNAMIC_MINOR) {
@@ -220,32 +220,26 @@ int misc_register(struct miscdevice *misc)
 
 	INIT_LIST_HEAD(&misc->list);
 
-	mutex_lock(&misc_mtx);
+	guard(mutex)(&misc_mtx);
 
 	if (is_dynamic) {
 		int i = misc_minor_alloc(misc->minor);
 
-		if (i < 0) {
-			err = -EBUSY;
-			goto out;
-		}
+		if (i < 0)
+			return -EBUSY;
 		misc->minor = i;
 	} else {
 		struct miscdevice *c;
 		int i;
 
 		list_for_each_entry(c, &misc_list, list) {
-			if (c->minor == misc->minor) {
-				err = -EBUSY;
-				goto out;
-			}
+			if (c->minor == misc->minor)
+				return -EBUSY;
 		}
 
 		i = misc_minor_alloc(misc->minor);
-		if (i < 0) {
-			err = -EBUSY;
-			goto out;
-		}
+		if (i < 0)
+			return -EBUSY;
 	}
 
 	dev = MKDEV(MISC_MAJOR, misc->minor);
@@ -258,8 +252,7 @@ int misc_register(struct miscdevice *misc)
 		if (is_dynamic) {
 			misc->minor = MISC_DYNAMIC_MINOR;
 		}
-		err = PTR_ERR(misc->this_device);
-		goto out;
+		return PTR_ERR(misc->this_device);
 	}
 
 	/*
@@ -267,9 +260,7 @@ int misc_register(struct miscdevice *misc)
 	 * earlier defaults
 	 */
 	list_add(&misc->list, &misc_list);
- out:
-	mutex_unlock(&misc_mtx);
-	return err;
+	return 0;
 }
 EXPORT_SYMBOL(misc_register);
 
@@ -283,13 +274,12 @@ EXPORT_SYMBOL(misc_register);
 
 void misc_deregister(struct miscdevice *misc)
 {
-	mutex_lock(&misc_mtx);
+	guard(mutex)(&misc_mtx);
 	list_del_init(&misc->list);
 	device_destroy(&misc_class, MKDEV(MISC_MAJOR, misc->minor));
 	misc_minor_free(misc->minor);
 	if (misc->minor > MISC_DYNAMIC_MINOR)
 		misc->minor = MISC_DYNAMIC_MINOR;
-	mutex_unlock(&misc_mtx);
 }
 EXPORT_SYMBOL(misc_deregister);
 
-- 
2.51.0

[PATCH 2/8] char: misc: Introduce misc_find() helper

[Tzung-Bi Shih]

Introduce misc_find() to search for a miscdevice by minor number in
`misc_list`.  This helper requires `misc_mtx` to be held.

Use the new helper to reduce code duplication.

Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
 drivers/char/misc.c | 43 +++++++++++++++++++++++--------------------
 1 file changed, 23 insertions(+), 20 deletions(-)

diff --git a/drivers/char/misc.c b/drivers/char/misc.c
index e2bea4f01851..c26315577d65 100644
--- a/drivers/char/misc.c
+++ b/drivers/char/misc.c
@@ -116,22 +116,32 @@ static const struct seq_operations misc_seq_ops = {
 };
 #endif
 
+static struct miscdevice *misc_find(int minor)
+{
+	struct miscdevice *iter;
+
+	lockdep_assert_held(&misc_mtx);
+
+	list_for_each_entry(iter, &misc_list, list) {
+		if (iter->minor == minor)
+			return iter;
+	}
+
+	return NULL;
+}
+
 static int misc_open(struct inode *inode, struct file *file)
 {
 	int minor = iminor(inode);
-	struct miscdevice *c = NULL, *iter;
+	struct miscdevice *c = NULL;
 	int err = -ENODEV;
 	const struct file_operations *new_fops = NULL;
 
 	mutex_lock(&misc_mtx);
 
-	list_for_each_entry(iter, &misc_list, list) {
-		if (iter->minor != minor)
-			continue;
-		c = iter;
-		new_fops = fops_get(iter->fops);
-		break;
-	}
+	c = misc_find(minor);
+	if (c)
+		new_fops = fops_get(c->fops);
 
 	/* Only request module for fixed minor code */
 	if (!new_fops && minor < MISC_DYNAMIC_MINOR) {
@@ -139,13 +149,9 @@ static int misc_open(struct inode *inode, struct file *file)
 		request_module("char-major-%d-%d", MISC_MAJOR, minor);
 		mutex_lock(&misc_mtx);
 
-		list_for_each_entry(iter, &misc_list, list) {
-			if (iter->minor != minor)
-				continue;
-			c = iter;
-			new_fops = fops_get(iter->fops);
-			break;
-		}
+		c = misc_find(minor);
+		if (c)
+			new_fops = fops_get(c->fops);
 	}
 
 	if (!new_fops)
@@ -229,13 +235,10 @@ int misc_register(struct miscdevice *misc)
 			return -EBUSY;
 		misc->minor = i;
 	} else {
-		struct miscdevice *c;
 		int i;
 
-		list_for_each_entry(c, &misc_list, list) {
-			if (c->minor == misc->minor)
-				return -EBUSY;
-		}
+		if (misc_find(misc->minor))
+			return -EBUSY;
 
 		i = misc_minor_alloc(misc->minor);
 		if (i < 0)
-- 
2.51.0

[PATCH 3/8] char: misc: Introduce misc_sync_register()

[Tzung-Bi Shih]

Introduce misc_sync_register() to support synchronous file operations
for misc devices.  This aims to prevent Use-After-Free errors when a
device is deregistered while file operations are still in progress or
files are open.

It creates a synchronization context that wraps supported file
operations and ensures the device is still registered before invoking
the file operations.

The minor number is deferred from being freed immediately on
deregistration and is used as a primary key to search for the
synchronization context in `misc_sync_ctx_list` after the device is
unregistered.

Performance impact:
- All file operations are serialized by a global lock.
- All file operations perform a linear search to find the corresponding
  miscdevice.

Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
 drivers/char/misc.c        | 123 ++++++++++++++++++++++++++++++++++++-
 include/linux/miscdevice.h |  10 +++
 2 files changed, 132 insertions(+), 1 deletion(-)

diff --git a/drivers/char/misc.c b/drivers/char/misc.c
index c26315577d65..87f47bdc7afb 100644
--- a/drivers/char/misc.c
+++ b/drivers/char/misc.c
@@ -56,6 +56,7 @@
  * Head entry for the doubly linked miscdevice list
  */
 static LIST_HEAD(misc_list);
+static LIST_HEAD(misc_sync_ctx_list);
 static DEFINE_MUTEX(misc_mtx);
 
 /*
@@ -130,6 +131,75 @@ static struct miscdevice *misc_find(int minor)
 	return NULL;
 }
 
+#define DEFINE_SYNC_FOPS(member, ret_type, PROTO, ARGS)			\
+	static ret_type misc_sync_##member PROTO			\
+	{								\
+		struct miscdevice *c;					\
+									\
+		guard(mutex)(&misc_mtx);				\
+									\
+		c = misc_find(iminor(filp->f_inode));			\
+		if (!c)							\
+			return -ENODEV;					\
+									\
+		return c->fops->member ARGS;				\
+	}
+
+DEFINE_SYNC_FOPS(read, ssize_t,
+	(struct file *filp, char __user *buf, size_t len, loff_t *off),
+	(filp, buf, len, off))
+DEFINE_SYNC_FOPS(unlocked_ioctl, long,
+		 (struct file *filp, unsigned int cmd, unsigned long arg),
+		 (filp, cmd, arg))
+DEFINE_SYNC_FOPS(compat_ioctl, long,
+		 (struct file *filp, unsigned int cmd, unsigned long arg),
+		 (filp, cmd, arg))
+
+static void misc_sync_ctx_release(struct kref *kref)
+{
+	struct miscdevice_sync_ctx *ctx = container_of(kref, typeof(*ctx), kref);
+
+	misc_minor_free(ctx->minor);
+	list_del(&ctx->list);
+	kfree(ctx);
+}
+
+static int misc_sync_release(struct inode *inode, struct file *filp)
+{
+	int minor = iminor(filp->f_inode);
+	struct miscdevice *c;
+	struct miscdevice_sync_ctx *iter, *ctx = NULL;
+
+	guard(mutex)(&misc_mtx);
+
+	c = misc_find(minor);
+	if (c) {
+		/* The miscdevice is still registered. */
+		ctx = c->sync_ctx;
+	} else {
+		/* The miscdeivce is unregistered.  Search in the list. */
+		list_for_each_entry(iter, &misc_sync_ctx_list, list) {
+			if (iter->minor == minor) {
+				ctx = iter;
+				break;
+			}
+		}
+		if (!ctx) {
+			pr_err("Cannot find miscdevice_sync_ctx\n");
+			return -ENOENT;
+		}
+	}
+
+	/* Restore it so that the corresponding fops_put() works. */
+	filp->f_op = ctx->orig_fops;
+	kref_put(&ctx->kref, misc_sync_ctx_release);
+
+	/* Call to the original .release() if any. */
+	if (filp->f_op->release)
+		return filp->f_op->release(inode, filp);
+	return 0;
+}
+
 static int misc_open(struct inode *inode, struct file *file)
 {
 	int minor = iminor(inode);
@@ -166,6 +236,10 @@ static int misc_open(struct inode *inode, struct file *file)
 
 	err = 0;
 	replace_fops(file, new_fops);
+	if (c->sync_ctx) {
+		file->f_op = &c->sync_ctx->fops;
+		kref_get(&c->sync_ctx->kref);
+	}
 	if (file->f_op->open)
 		err = file->f_op->open(inode, file);
 fail:
@@ -280,12 +354,59 @@ void misc_deregister(struct miscdevice *misc)
 	guard(mutex)(&misc_mtx);
 	list_del_init(&misc->list);
 	device_destroy(&misc_class, MKDEV(MISC_MAJOR, misc->minor));
-	misc_minor_free(misc->minor);
+
+	/* Defer to free the minor number for sync fops */
+	if (!misc->sync_ctx) {
+		misc_minor_free(misc->minor);
+	} else {
+		list_add(&misc->sync_ctx->list, &misc_sync_ctx_list);
+		kref_put(&misc->sync_ctx->kref, misc_sync_ctx_release);
+	}
+
 	if (misc->minor > MISC_DYNAMIC_MINOR)
 		misc->minor = MISC_DYNAMIC_MINOR;
 }
 EXPORT_SYMBOL(misc_deregister);
 
+int misc_sync_register(struct miscdevice *misc)
+{
+	struct miscdevice_sync_ctx *ctx;
+	int ret;
+
+	ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
+	if (!ctx)
+		return -ENOMEM;
+
+	ret = misc_register(misc);
+	if (ret) {
+		kfree(ctx);
+		return ret;
+	}
+
+	ctx->minor = misc->minor;
+	kref_init(&ctx->kref);
+	ctx->orig_fops = misc->fops;
+	INIT_LIST_HEAD(&ctx->list);
+
+	/* Use any fops as default in case the misc sync doesn't support them. */
+	memcpy(&ctx->fops, misc->fops, sizeof(struct file_operations));
+
+	/* Override fops that support sync. */
+	if (misc->fops->read)
+		ctx->fops.read = misc_sync_read;
+	if (misc->fops->unlocked_ioctl)
+		ctx->fops.unlocked_ioctl = misc_sync_unlocked_ioctl;
+	if (misc->fops->compat_ioctl)
+		ctx->fops.compat_ioctl = misc_sync_compat_ioctl;
+
+	/* .release() is used to drop the reference to the sync context. */
+	ctx->fops.release = misc_sync_release;
+
+	misc->sync_ctx = ctx;
+	return 0;
+}
+EXPORT_SYMBOL(misc_sync_register);
+
 static int __init misc_init(void)
 {
 	int err;
diff --git a/include/linux/miscdevice.h b/include/linux/miscdevice.h
index fa9000f68523..a60d8281e1dd 100644
--- a/include/linux/miscdevice.h
+++ b/include/linux/miscdevice.h
@@ -81,6 +81,14 @@
  */
 #define MISC_DYNAMIC_MINOR	255
 
+struct miscdevice_sync_ctx {
+	int minor;
+	struct kref kref;
+	const struct file_operations *orig_fops;
+	struct list_head list;
+	struct file_operations fops;
+};
+
 struct miscdevice {
 	int minor;
 	const char *name;
@@ -91,10 +99,12 @@ struct miscdevice {
 	const struct attribute_group **groups;
 	const char *nodename;
 	umode_t mode;
+	struct miscdevice_sync_ctx *sync_ctx;
 };
 
 extern int misc_register(struct miscdevice *misc);
 extern void misc_deregister(struct miscdevice *misc);
+extern int misc_sync_register(struct miscdevice *misc);
 
 /*
  * Helper macro for drivers that don't do anything special in the initcall.
-- 
2.51.0

[PATCH 4/8] char: misc: Use SRCU to protect list traversal

[Tzung-Bi Shih]

Replace the global mutex with SRCU to protect list traversal in the
file operations.  This allows concurrent file operations on misc devices
without contending for a global lock.

Writers (registration and deregistration) continue to be serialized
by a mutex when modifying the lists.

Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
 drivers/char/misc.c | 84 ++++++++++++++++++++++++++-------------------
 1 file changed, 48 insertions(+), 36 deletions(-)

diff --git a/drivers/char/misc.c b/drivers/char/misc.c
index 87f47bdc7afb..8514287b8ff9 100644
--- a/drivers/char/misc.c
+++ b/drivers/char/misc.c
@@ -45,6 +45,7 @@
 #include <linux/mutex.h>
 #include <linux/proc_fs.h>
 #include <linux/seq_file.h>
+#include <linux/srcu.h>
 #include <linux/stat.h>
 #include <linux/init.h>
 #include <linux/device.h>
@@ -58,6 +59,7 @@
 static LIST_HEAD(misc_list);
 static LIST_HEAD(misc_sync_ctx_list);
 static DEFINE_MUTEX(misc_mtx);
+DEFINE_STATIC_SRCU(misc_srcu);
 
 /*
  * Assigned numbers.
@@ -86,23 +88,23 @@ static void misc_minor_free(int minor)
 #ifdef CONFIG_PROC_FS
 static void *misc_seq_start(struct seq_file *seq, loff_t *pos)
 {
-	mutex_lock(&misc_mtx);
-	return seq_list_start(&misc_list, *pos);
+	seq->private = (void *)(long)srcu_read_lock(&misc_srcu);
+	return seq_list_start_rcu(&misc_list, *pos);
 }
 
 static void *misc_seq_next(struct seq_file *seq, void *v, loff_t *pos)
 {
-	return seq_list_next(v, &misc_list, pos);
+	return seq_list_next_rcu(v, &misc_list, pos);
 }
 
 static void misc_seq_stop(struct seq_file *seq, void *v)
 {
-	mutex_unlock(&misc_mtx);
+	srcu_read_unlock(&misc_srcu, (int)(long)seq->private);
 }
 
 static int misc_seq_show(struct seq_file *seq, void *v)
 {
-	const struct miscdevice *p = list_entry(v, struct miscdevice, list);
+	const struct miscdevice *p = list_entry_rcu(v, struct miscdevice, list);
 
 	seq_printf(seq, "%3i %s\n", p->minor, p->name ? p->name : "");
 	return 0;
@@ -121,9 +123,8 @@ static struct miscdevice *misc_find(int minor)
 {
 	struct miscdevice *iter;
 
-	lockdep_assert_held(&misc_mtx);
-
-	list_for_each_entry(iter, &misc_list, list) {
+	list_for_each_entry_srcu(iter, &misc_list, list,
+				 srcu_read_lock_held(&misc_srcu)) {
 		if (iter->minor == minor)
 			return iter;
 	}
@@ -136,7 +137,7 @@ static struct miscdevice *misc_find(int minor)
 	{								\
 		struct miscdevice *c;					\
 									\
-		guard(mutex)(&misc_mtx);				\
+		guard(srcu)(&misc_srcu);				\
 									\
 		c = misc_find(iminor(filp->f_inode));			\
 		if (!c)							\
@@ -160,7 +161,9 @@ static void misc_sync_ctx_release(struct kref *kref)
 	struct miscdevice_sync_ctx *ctx = container_of(kref, typeof(*ctx), kref);
 
 	misc_minor_free(ctx->minor);
-	list_del(&ctx->list);
+	scoped_guard(mutex, &misc_mtx)
+		list_del_rcu(&ctx->list);
+	synchronize_srcu(&misc_srcu);
 	kfree(ctx);
 }
 
@@ -170,23 +173,24 @@ static int misc_sync_release(struct inode *inode, struct file *filp)
 	struct miscdevice *c;
 	struct miscdevice_sync_ctx *iter, *ctx = NULL;
 
-	guard(mutex)(&misc_mtx);
-
-	c = misc_find(minor);
-	if (c) {
-		/* The miscdevice is still registered. */
-		ctx = c->sync_ctx;
-	} else {
-		/* The miscdeivce is unregistered.  Search in the list. */
-		list_for_each_entry(iter, &misc_sync_ctx_list, list) {
-			if (iter->minor == minor) {
-				ctx = iter;
-				break;
+	scoped_guard(srcu, &misc_srcu) {
+		c = misc_find(minor);
+		if (c) {
+			/* The miscdevice is still registered. */
+			ctx = c->sync_ctx;
+		} else {
+			/* The miscdeivce is unregistered.  Search in the list. */
+			list_for_each_entry_srcu(iter, &misc_sync_ctx_list,
+					list, srcu_read_lock_held(&misc_srcu)) {
+				if (iter->minor == minor) {
+					ctx = iter;
+					break;
+				}
+			}
+			if (!ctx) {
+				pr_err("Cannot find miscdevice_sync_ctx\n");
+				return -ENOENT;
 			}
-		}
-		if (!ctx) {
-			pr_err("Cannot find miscdevice_sync_ctx\n");
-			return -ENOENT;
 		}
 	}
 
@@ -206,8 +210,9 @@ static int misc_open(struct inode *inode, struct file *file)
 	struct miscdevice *c = NULL;
 	int err = -ENODEV;
 	const struct file_operations *new_fops = NULL;
+	int idx;
 
-	mutex_lock(&misc_mtx);
+	idx = srcu_read_lock(&misc_srcu);
 
 	c = misc_find(minor);
 	if (c)
@@ -215,9 +220,9 @@ static int misc_open(struct inode *inode, struct file *file)
 
 	/* Only request module for fixed minor code */
 	if (!new_fops && minor < MISC_DYNAMIC_MINOR) {
-		mutex_unlock(&misc_mtx);
+		srcu_read_unlock(&misc_srcu, idx);
 		request_module("char-major-%d-%d", MISC_MAJOR, minor);
-		mutex_lock(&misc_mtx);
+		idx = srcu_read_lock(&misc_srcu);
 
 		c = misc_find(minor);
 		if (c)
@@ -243,7 +248,7 @@ static int misc_open(struct inode *inode, struct file *file)
 	if (file->f_op->open)
 		err = file->f_op->open(inode, file);
 fail:
-	mutex_unlock(&misc_mtx);
+	srcu_read_unlock(&misc_srcu, idx);
 	return err;
 }
 
@@ -311,8 +316,10 @@ int misc_register(struct miscdevice *misc)
 	} else {
 		int i;
 
-		if (misc_find(misc->minor))
-			return -EBUSY;
+		scoped_guard(srcu, &misc_srcu) {
+			if (misc_find(misc->minor))
+				return -EBUSY;
+		}
 
 		i = misc_minor_alloc(misc->minor);
 		if (i < 0)
@@ -336,7 +343,7 @@ int misc_register(struct miscdevice *misc)
 	 * Add it to the front, so that later devices can "override"
 	 * earlier defaults
 	 */
-	list_add(&misc->list, &misc_list);
+	list_add_rcu(&misc->list, &misc_list);
 	return 0;
 }
 EXPORT_SYMBOL(misc_register);
@@ -351,15 +358,20 @@ EXPORT_SYMBOL(misc_register);
 
 void misc_deregister(struct miscdevice *misc)
 {
-	guard(mutex)(&misc_mtx);
-	list_del_init(&misc->list);
+	scoped_guard(mutex, &misc_mtx)
+		list_del_rcu(&misc->list);
+	synchronize_srcu(&misc_srcu);
+	INIT_LIST_HEAD(&misc->list);
+
 	device_destroy(&misc_class, MKDEV(MISC_MAJOR, misc->minor));
 
 	/* Defer to free the minor number for sync fops */
 	if (!misc->sync_ctx) {
 		misc_minor_free(misc->minor);
 	} else {
-		list_add(&misc->sync_ctx->list, &misc_sync_ctx_list);
+		scoped_guard(mutex, &misc_mtx)
+			list_add_rcu(&misc->sync_ctx->list,
+				     &misc_sync_ctx_list);
 		kref_put(&misc->sync_ctx->kref, misc_sync_ctx_release);
 	}
 
-- 
2.51.0

[PATCH 5/8] platform/chrome: cros_ec_chardev: Introduce chardev_data

[Tzung-Bi Shih]

Introduce struct chardev_pdata to hold platform driver data.

The platform driver data is allocated by kzalloc() instead of devm
variant, allowing for managed cleanup that can eventually extend beyond
device removal if files are still open.

Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
 drivers/platform/chrome/cros_ec_chardev.c | 51 +++++++++++++++++------
 1 file changed, 39 insertions(+), 12 deletions(-)

diff --git a/drivers/platform/chrome/cros_ec_chardev.c b/drivers/platform/chrome/cros_ec_chardev.c
index 002be3352100..e7012e44a006 100644
--- a/drivers/platform/chrome/cros_ec_chardev.c
+++ b/drivers/platform/chrome/cros_ec_chardev.c
@@ -13,6 +13,7 @@
 #include <linux/init.h>
 #include <linux/device.h>
 #include <linux/fs.h>
+#include <linux/kref.h>
 #include <linux/miscdevice.h>
 #include <linux/mod_devicetable.h>
 #include <linux/module.h>
@@ -31,6 +32,21 @@
 /* Arbitrary bounded size for the event queue */
 #define CROS_MAX_EVENT_LEN	PAGE_SIZE
 
+/*
+ * Platform device driver data.
+ */
+struct chardev_pdata {
+	struct miscdevice misc;
+	struct kref kref;
+};
+
+static void chardev_pdata_release(struct kref *kref)
+{
+	struct chardev_pdata *pdata = container_of(kref, typeof(*pdata), kref);
+
+	kfree(pdata);
+}
+
 struct chardev_priv {
 	struct cros_ec_device *ec_dev;
 	struct notifier_block notifier;
@@ -374,28 +390,39 @@ static int cros_ec_chardev_probe(struct platform_device *pdev)
 {
 	struct cros_ec_dev *ec = dev_get_drvdata(pdev->dev.parent);
 	struct cros_ec_platform *ec_platform = dev_get_platdata(ec->dev);
-	struct miscdevice *misc;
+	struct chardev_pdata *pdata;
+	int ret;
 
-	/* Create a char device: we want to create it anew */
-	misc = devm_kzalloc(&pdev->dev, sizeof(*misc), GFP_KERNEL);
-	if (!misc)
+	pdata = kzalloc_obj(*pdata);
+	if (!pdata)
 		return -ENOMEM;
 
-	misc->minor = MISC_DYNAMIC_MINOR;
-	misc->fops = &chardev_fops;
-	misc->name = ec_platform->ec_name;
-	misc->parent = pdev->dev.parent;
+	platform_set_drvdata(pdev, pdata);
+	kref_init(&pdata->kref);
 
-	dev_set_drvdata(&pdev->dev, misc);
+	pdata->misc.minor = MISC_DYNAMIC_MINOR;
+	pdata->misc.fops = &chardev_fops;
+	pdata->misc.name = ec_platform->ec_name;
+	pdata->misc.parent = pdev->dev.parent;
 
-	return misc_register(misc);
+	ret = misc_register(&pdata->misc);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to register misc device\n");
+		goto err_put_pdata;
+	}
+
+	return 0;
+err_put_pdata:
+	kref_put(&pdata->kref, chardev_pdata_release);
+	return ret;
 }
 
 static void cros_ec_chardev_remove(struct platform_device *pdev)
 {
-	struct miscdevice *misc = dev_get_drvdata(&pdev->dev);
+	struct chardev_pdata *pdata = platform_get_drvdata(pdev);
 
-	misc_deregister(misc);
+	misc_deregister(&pdata->misc);
+	kref_put(&pdata->kref, chardev_pdata_release);
 }
 
 static const struct platform_device_id cros_ec_chardev_id[] = {
-- 
2.51.0

[PATCH 6/8] platform/chrome: cros_ec_chardev: Move data to chardev_pdata

[Tzung-Bi Shih]

Move `ec_dev` and `cmd_offset` from `chardev_priv` to `chardev_pdata` as
they are per-device properties but not per-open-file properties.

Hold a reference to `chardev_pdata` for each open file to ensure the
data remains valid even if the underlying platform device is removed.

Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
 drivers/platform/chrome/cros_ec_chardev.c | 36 +++++++++++++----------
 1 file changed, 20 insertions(+), 16 deletions(-)

diff --git a/drivers/platform/chrome/cros_ec_chardev.c b/drivers/platform/chrome/cros_ec_chardev.c
index e7012e44a006..352d61a2f3c6 100644
--- a/drivers/platform/chrome/cros_ec_chardev.c
+++ b/drivers/platform/chrome/cros_ec_chardev.c
@@ -38,6 +38,8 @@
 struct chardev_pdata {
 	struct miscdevice misc;
 	struct kref kref;
+	struct cros_ec_device *ec_dev;
+	u16 cmd_offset;
 };
 
 static void chardev_pdata_release(struct kref *kref)
@@ -48,13 +50,12 @@ static void chardev_pdata_release(struct kref *kref)
 }
 
 struct chardev_priv {
-	struct cros_ec_device *ec_dev;
 	struct notifier_block notifier;
 	wait_queue_head_t wait_event;
 	unsigned long event_mask;
 	struct list_head events;
 	size_t event_len;
-	u16 cmd_offset;
+	struct chardev_pdata *pdata;
 };
 
 struct ec_event {
@@ -77,10 +78,10 @@ static int ec_get_version(struct chardev_priv *priv, char *str, int maxlen)
 	if (!msg)
 		return -ENOMEM;
 
-	msg->command = EC_CMD_GET_VERSION + priv->cmd_offset;
+	msg->command = EC_CMD_GET_VERSION + priv->pdata->cmd_offset;
 	msg->insize = sizeof(*resp);
 
-	ret = cros_ec_cmd_xfer_status(priv->ec_dev, msg);
+	ret = cros_ec_cmd_xfer_status(priv->pdata->ec_dev, msg);
 	if (ret < 0) {
 		snprintf(str, maxlen,
 			 "Unknown EC version, returned error: %d\n",
@@ -108,7 +109,7 @@ static int cros_ec_chardev_mkbp_event(struct notifier_block *nb,
 {
 	struct chardev_priv *priv = container_of(nb, struct chardev_priv,
 						 notifier);
-	struct cros_ec_device *ec_dev = priv->ec_dev;
+	struct cros_ec_device *ec_dev = priv->pdata->ec_dev;
 	struct ec_event *event;
 	unsigned long event_bit = 1 << ec_dev->event_data.event_type;
 	int total_size = sizeof(*event) + ec_dev->event_size;
@@ -173,8 +174,7 @@ static struct ec_event *cros_ec_chardev_fetch_event(struct chardev_priv *priv,
 static int cros_ec_chardev_open(struct inode *inode, struct file *filp)
 {
 	struct miscdevice *mdev = filp->private_data;
-	struct cros_ec_dev *ec = dev_get_drvdata(mdev->parent);
-	struct cros_ec_device *ec_dev = ec->ec_dev;
+	struct chardev_pdata *pdata = container_of(mdev, typeof(*pdata), misc);
 	struct chardev_priv *priv;
 	int ret;
 
@@ -182,18 +182,20 @@ static int cros_ec_chardev_open(struct inode *inode, struct file *filp)
 	if (!priv)
 		return -ENOMEM;
 
-	priv->ec_dev = ec_dev;
-	priv->cmd_offset = ec->cmd_offset;
+	priv->pdata = pdata;
+	kref_get(&pdata->kref);
 	filp->private_data = priv;
 	INIT_LIST_HEAD(&priv->events);
 	init_waitqueue_head(&priv->wait_event);
 	nonseekable_open(inode, filp);
 
 	priv->notifier.notifier_call = cros_ec_chardev_mkbp_event;
-	ret = blocking_notifier_chain_register(&ec_dev->event_notifier,
+	ret = blocking_notifier_chain_register(&pdata->ec_dev->event_notifier,
 					       &priv->notifier);
 	if (ret) {
-		dev_err(ec_dev->dev, "failed to register event notifier\n");
+		dev_err(pdata->ec_dev->dev,
+			"failed to register event notifier\n");
+		kref_put(&priv->pdata->kref, chardev_pdata_release);
 		kfree(priv);
 	}
 
@@ -267,11 +269,11 @@ static ssize_t cros_ec_chardev_read(struct file *filp, char __user *buffer,
 static int cros_ec_chardev_release(struct inode *inode, struct file *filp)
 {
 	struct chardev_priv *priv = filp->private_data;
-	struct cros_ec_device *ec_dev = priv->ec_dev;
 	struct ec_event *event, *e;
 
-	blocking_notifier_chain_unregister(&ec_dev->event_notifier,
+	blocking_notifier_chain_unregister(&priv->pdata->ec_dev->event_notifier,
 					   &priv->notifier);
+	kref_put(&priv->pdata->kref, chardev_pdata_release);
 
 	list_for_each_entry_safe(event, e, &priv->events, node) {
 		list_del(&event->node);
@@ -314,8 +316,8 @@ static long cros_ec_chardev_ioctl_xcmd(struct chardev_priv *priv, void __user *a
 		goto exit;
 	}
 
-	s_cmd->command += priv->cmd_offset;
-	ret = cros_ec_cmd_xfer(priv->ec_dev, s_cmd);
+	s_cmd->command += priv->pdata->cmd_offset;
+	ret = cros_ec_cmd_xfer(priv->pdata->ec_dev, s_cmd);
 	/* Only copy data to userland if data was received. */
 	if (ret < 0)
 		goto exit;
@@ -329,7 +331,7 @@ static long cros_ec_chardev_ioctl_xcmd(struct chardev_priv *priv, void __user *a
 
 static long cros_ec_chardev_ioctl_readmem(struct chardev_priv *priv, void __user *arg)
 {
-	struct cros_ec_device *ec_dev = priv->ec_dev;
+	struct cros_ec_device *ec_dev = priv->pdata->ec_dev;
 	struct cros_ec_readmem s_mem = { };
 	long num;
 
@@ -399,6 +401,8 @@ static int cros_ec_chardev_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, pdata);
 	kref_init(&pdata->kref);
+	pdata->ec_dev = ec->ec_dev;
+	pdata->cmd_offset = ec->cmd_offset;
 
 	pdata->misc.minor = MISC_DYNAMIC_MINOR;
 	pdata->misc.fops = &chardev_fops;
-- 
2.51.0

[PATCH 7/8] platform/chrome: cros_ec_chardev: Add event relayer

[Tzung-Bi Shih]

Introduce an event relayer mechanism.  Instead of each open file
registering directly with `ec_dev->event_notifier`, the platform device
registers a single relayer notifier.  Individual files then register
with a local subscribers list in `chardev_pdata`.

This allows the driver to safely disconnect from the event chain
`ec_dev->event_notifier` during cros_ec_chardev_remove(), preventing
events from being delivered to open files after the device is removed,
while still allowing those files to be closed safely later.

Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
This is an implementation of the idea from [1].

[1] https://lore.kernel.org/all/20251117153301.GD10864@nvidia.com/
---
 drivers/platform/chrome/cros_ec_chardev.c | 32 ++++++++++++++++++++---
 1 file changed, 29 insertions(+), 3 deletions(-)

diff --git a/drivers/platform/chrome/cros_ec_chardev.c b/drivers/platform/chrome/cros_ec_chardev.c
index 352d61a2f3c6..7e046fc56998 100644
--- a/drivers/platform/chrome/cros_ec_chardev.c
+++ b/drivers/platform/chrome/cros_ec_chardev.c
@@ -40,6 +40,8 @@ struct chardev_pdata {
 	struct kref kref;
 	struct cros_ec_device *ec_dev;
 	u16 cmd_offset;
+	struct blocking_notifier_head subscribers;
+	struct notifier_block relay;
 };
 
 static void chardev_pdata_release(struct kref *kref)
@@ -49,6 +51,17 @@ static void chardev_pdata_release(struct kref *kref)
 	kfree(pdata);
 }
 
+static int cros_ec_chardev_relay_event(struct notifier_block *nb,
+				       unsigned long queued_during_suspend,
+				       void *_notify)
+{
+	struct chardev_pdata *pdata = container_of(nb, typeof(*pdata), relay);
+
+	blocking_notifier_call_chain(&pdata->subscribers, queued_during_suspend,
+				     _notify);
+	return NOTIFY_OK;
+}
+
 struct chardev_priv {
 	struct notifier_block notifier;
 	wait_queue_head_t wait_event;
@@ -190,7 +203,7 @@ static int cros_ec_chardev_open(struct inode *inode, struct file *filp)
 	nonseekable_open(inode, filp);
 
 	priv->notifier.notifier_call = cros_ec_chardev_mkbp_event;
-	ret = blocking_notifier_chain_register(&pdata->ec_dev->event_notifier,
+	ret = blocking_notifier_chain_register(&pdata->subscribers,
 					       &priv->notifier);
 	if (ret) {
 		dev_err(pdata->ec_dev->dev,
@@ -271,7 +284,7 @@ static int cros_ec_chardev_release(struct inode *inode, struct file *filp)
 	struct chardev_priv *priv = filp->private_data;
 	struct ec_event *event, *e;
 
-	blocking_notifier_chain_unregister(&priv->pdata->ec_dev->event_notifier,
+	blocking_notifier_chain_unregister(&priv->pdata->subscribers,
 					   &priv->notifier);
 	kref_put(&priv->pdata->kref, chardev_pdata_release);
 
@@ -403,6 +416,14 @@ static int cros_ec_chardev_probe(struct platform_device *pdev)
 	kref_init(&pdata->kref);
 	pdata->ec_dev = ec->ec_dev;
 	pdata->cmd_offset = ec->cmd_offset;
+	BLOCKING_INIT_NOTIFIER_HEAD(&pdata->subscribers);
+	pdata->relay.notifier_call = cros_ec_chardev_relay_event;
+	ret = blocking_notifier_chain_register(&pdata->ec_dev->event_notifier,
+					       &pdata->relay);
+	if (ret) {
+		dev_err(&pdev->dev, "failed to register event notifier\n");
+		goto err_put_pdata;
+	}
 
 	pdata->misc.minor = MISC_DYNAMIC_MINOR;
 	pdata->misc.fops = &chardev_fops;
@@ -412,10 +433,13 @@ static int cros_ec_chardev_probe(struct platform_device *pdev)
 	ret = misc_register(&pdata->misc);
 	if (ret) {
 		dev_err(&pdev->dev, "failed to register misc device\n");
-		goto err_put_pdata;
+		goto err_unregister_notifier;
 	}
 
 	return 0;
+err_unregister_notifier:
+	blocking_notifier_chain_unregister(&pdata->ec_dev->event_notifier,
+					   &pdata->relay);
 err_put_pdata:
 	kref_put(&pdata->kref, chardev_pdata_release);
 	return ret;
@@ -425,6 +449,8 @@ static void cros_ec_chardev_remove(struct platform_device *pdev)
 {
 	struct chardev_pdata *pdata = platform_get_drvdata(pdev);
 
+	blocking_notifier_chain_unregister(&pdata->ec_dev->event_notifier,
+					   &pdata->relay);
 	misc_deregister(&pdata->misc);
 	kref_put(&pdata->kref, chardev_pdata_release);
 }
-- 
2.51.0

[PATCH 8/8] platform/chrome: cros_ec_chardev: Use misc_sync_register()

[Tzung-Bi Shih]

Switch to use misc_sync_register() instead of misc_register() to
enable synchronous file operations support.  This helps prevent
Use-After-Free errors when the device is removed while files are still
open.

Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
 drivers/platform/chrome/cros_ec_chardev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/chrome/cros_ec_chardev.c b/drivers/platform/chrome/cros_ec_chardev.c
index 7e046fc56998..d44afa3ef92d 100644
--- a/drivers/platform/chrome/cros_ec_chardev.c
+++ b/drivers/platform/chrome/cros_ec_chardev.c
@@ -430,7 +430,7 @@ static int cros_ec_chardev_probe(struct platform_device *pdev)
 	pdata->misc.name = ec_platform->ec_name;
 	pdata->misc.parent = pdev->dev.parent;
 
-	ret = misc_register(&pdata->misc);
+	ret = misc_sync_register(&pdata->misc);
 	if (ret) {
 		dev_err(&pdev->dev, "failed to register misc device\n");
 		goto err_unregister_notifier;
-- 
2.51.0

Re: [PATCH 3/8] char: misc: Introduce misc_sync_register()

[Jason Gunthorpe]

On Mon, Apr 27, 2026 at 09:46:54PM +0800, Tzung-Bi Shih wrote:
> Introduce misc_sync_register() to support synchronous file operations
> for misc devices.  This aims to prevent Use-After-Free errors when a
> device is deregistered while file operations are still in progress or
> files are open.
> 
> It creates a synchronization context that wraps supported file
> operations and ensures the device is still registered before invoking
> the file operations.
> 
> The minor number is deferred from being freed immediately on
> deregistration and is used as a primary key to search for the
> synchronization context in `misc_sync_ctx_list` after the device is
> unregistered.
> 
> Performance impact:
> - All file operations are serialized by a global lock.
> - All file operations perform a linear search to find the corresponding
>   miscdevice.

This doesn't seem like a serious proposal, this is too much
performance cost.

Jason