[PATCH v2 0/4] platform/chrome: cros_ec_chardev: Fix a potential UAF

[PATCH v2 0/4] platform/chrome: cros_ec_chardev: Fix a potential UAF

[Tzung-Bi Shih]

This series addresses a potential Use-After-Free error when a device is
deregistered while file operations are still in progress or files remain
open in cros_ec_chardev.

- Patches 1 to 3 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()`.
- Patch 4 introduces a rwsem for protecting `ec_dev` to prevent the UAF
  error.

---
v2:
- Merge patches 5 to 7 from
  https://lore.kernel.org/all/20260427134659.95181-1-tzungbi@kernel.org.

v1: Doesn't exist.

Tzung-Bi Shih (4):
  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: Introduce rwsem for protecting
    ec_dev

 drivers/platform/chrome/cros_ec_chardev.c | 171 +++++++++++++++++-----
 1 file changed, 132 insertions(+), 39 deletions(-)

-- 
2.51.0

[PATCH v2 1/4] 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>
---
v2:
- No changes.

v1: https://lore.kernel.org/all/20260427134659.95181-6-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 v2 2/4] 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>
---
v2:
- No changes.

v1: https://lore.kernel.org/all/20260427134659.95181-7-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 v2 3/4] 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/
---
v2:
- No changes.

v1: https://lore.kernel.org/all/20260427134659.95181-8-tzungbi@kernel.org

 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 v2 4/4] platform/chrome: cros_ec_chardev: Introduce rwsem for protecting ec_dev

[Tzung-Bi Shih]

Introduce a rwsem for protecting `ec_dev` to prevent Use-After-Free on
the `ec_dev`.

- Writers: In driver's probe() and remove().
- Readers: In file operations.

Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
v2:
- New to the series.

v1: Doesn't exist.

 drivers/platform/chrome/cros_ec_chardev.c | 68 +++++++++++++++++------
 1 file changed, 52 insertions(+), 16 deletions(-)

diff --git a/drivers/platform/chrome/cros_ec_chardev.c b/drivers/platform/chrome/cros_ec_chardev.c
index 7e046fc56998..450f79759122 100644
--- a/drivers/platform/chrome/cros_ec_chardev.c
+++ b/drivers/platform/chrome/cros_ec_chardev.c
@@ -23,6 +23,7 @@
 #include <linux/platform_data/cros_ec_proto.h>
 #include <linux/platform_device.h>
 #include <linux/poll.h>
+#include <linux/rwsem.h>
 #include <linux/slab.h>
 #include <linux/types.h>
 #include <linux/uaccess.h>
@@ -38,6 +39,7 @@
 struct chardev_pdata {
 	struct miscdevice misc;
 	struct kref kref;
+	struct rw_semaphore ec_dev_sem;
 	struct cros_ec_device *ec_dev;
 	u16 cmd_offset;
 	struct blocking_notifier_head subscribers;
@@ -94,12 +96,19 @@ static int ec_get_version(struct chardev_priv *priv, char *str, int maxlen)
 	msg->command = EC_CMD_GET_VERSION + priv->pdata->cmd_offset;
 	msg->insize = sizeof(*resp);
 
-	ret = cros_ec_cmd_xfer_status(priv->pdata->ec_dev, msg);
-	if (ret < 0) {
-		snprintf(str, maxlen,
-			 "Unknown EC version, returned error: %d\n",
-			 msg->result);
-		goto exit;
+	scoped_guard(rwsem_read, &priv->pdata->ec_dev_sem) {
+		if (!priv->pdata->ec_dev) {
+			ret = -ENODEV;
+			goto exit;
+		}
+
+		ret = cros_ec_cmd_xfer_status(priv->pdata->ec_dev, msg);
+		if (ret < 0) {
+			snprintf(str, maxlen,
+				 "Unknown EC version, returned error: %d\n",
+				 msg->result);
+			goto exit;
+		}
 	}
 
 	resp = (struct ec_response_get_version *)msg->data;
@@ -122,10 +131,18 @@ 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->pdata->ec_dev;
+	struct cros_ec_device *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;
+	unsigned long event_bit;
+	int total_size;
+
+	guard(rwsem_read)(&priv->pdata->ec_dev_sem);
+	if (!priv->pdata->ec_dev)
+		return NOTIFY_DONE;
+	ec_dev = priv->pdata->ec_dev;
+
+	event_bit = 1 << ec_dev->event_data.event_type;
+	total_size = sizeof(*event) + ec_dev->event_size;
 
 	if (!(event_bit & priv->event_mask) ||
 	    (priv->event_len + total_size) > CROS_MAX_EVENT_LEN)
@@ -206,8 +223,11 @@ static int cros_ec_chardev_open(struct inode *inode, struct file *filp)
 	ret = blocking_notifier_chain_register(&pdata->subscribers,
 					       &priv->notifier);
 	if (ret) {
-		dev_err(pdata->ec_dev->dev,
-			"failed to register event notifier\n");
+		scoped_guard(rwsem_read, &pdata->ec_dev_sem) {
+			if (pdata->ec_dev)
+				dev_err(pdata->ec_dev->dev,
+					"failed to register event notifier\n");
+		}
 		kref_put(&priv->pdata->kref, chardev_pdata_release);
 		kfree(priv);
 	}
@@ -330,10 +350,18 @@ static long cros_ec_chardev_ioctl_xcmd(struct chardev_priv *priv, void __user *a
 	}
 
 	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;
+
+	scoped_guard(rwsem_read, &priv->pdata->ec_dev_sem) {
+		if (!priv->pdata->ec_dev) {
+			ret = -ENODEV;
+			goto exit;
+		}
+
+		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;
+	}
 
 	if (copy_to_user(arg, s_cmd, sizeof(*s_cmd) + s_cmd->insize))
 		ret = -EFAULT;
@@ -344,10 +372,15 @@ 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->pdata->ec_dev;
+	struct cros_ec_device *ec_dev;
 	struct cros_ec_readmem s_mem = { };
 	long num;
 
+	guard(rwsem_read)(&priv->pdata->ec_dev_sem);
+	if (!priv->pdata->ec_dev)
+		return -ENODEV;
+	ec_dev = priv->pdata->ec_dev;
+
 	/* Not every platform supports direct reads */
 	if (!ec_dev->cmd_readmem)
 		return -ENOTTY;
@@ -414,6 +447,7 @@ static int cros_ec_chardev_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, pdata);
 	kref_init(&pdata->kref);
+	init_rwsem(&pdata->ec_dev_sem);
 	pdata->ec_dev = ec->ec_dev;
 	pdata->cmd_offset = ec->cmd_offset;
 	BLOCKING_INIT_NOTIFIER_HEAD(&pdata->subscribers);
@@ -451,6 +485,8 @@ static void cros_ec_chardev_remove(struct platform_device *pdev)
 
 	blocking_notifier_chain_unregister(&pdata->ec_dev->event_notifier,
 					   &pdata->relay);
+	scoped_guard(rwsem_write, &pdata->ec_dev_sem)
+		pdata->ec_dev = NULL;
 	misc_deregister(&pdata->misc);
 	kref_put(&pdata->kref, chardev_pdata_release);
 }
-- 
2.51.0

Re: [PATCH v2 4/4] platform/chrome: cros_ec_chardev: Introduce rwsem for protecting ec_dev

[Jason Gunthorpe]

On Sat, May 16, 2026 at 10:30:17PM +0800, Tzung-Bi Shih wrote:
> @@ -94,12 +96,19 @@ static int ec_get_version(struct chardev_priv *priv, char *str, int maxlen)
>  	msg->command = EC_CMD_GET_VERSION + priv->pdata->cmd_offset;
>  	msg->insize = sizeof(*resp);
>  
> -	ret = cros_ec_cmd_xfer_status(priv->pdata->ec_dev, msg);
> -	if (ret < 0) {
> -		snprintf(str, maxlen,
> -			 "Unknown EC version, returned error: %d\n",
> -			 msg->result);
> -		goto exit;
> +	scoped_guard(rwsem_read, &priv->pdata->ec_dev_sem) {

I would not use scoped_guard here, especially since it isn't used
consistently

> +		if (!priv->pdata->ec_dev) {
> +			ret = -ENODEV;
> +			goto exit;
> +		}
> +
> +		ret = cros_ec_cmd_xfer_status(priv->pdata->ec_dev, msg);
> +		if (ret < 0) {
> +			snprintf(str, maxlen,
> +				 "Unknown EC version, returned error: %d\n",
> +				 msg->result);
> +			goto exit;
> +		}
>  	}
>  
>  	resp = (struct ec_response_get_version *)msg->data;
> @@ -122,10 +131,18 @@ 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->pdata->ec_dev;
> +	struct cros_ec_device *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;
> +	unsigned long event_bit;
> +	int total_size;
> +
> +	guard(rwsem_read)(&priv->pdata->ec_dev_sem);
> +	if (!priv->pdata->ec_dev)
> +		return NOTIFY_DONE;
> +	ec_dev = priv->pdata->ec_dev;
> +
> +	event_bit = 1 << ec_dev->event_data.event_type;
> +	total_size = sizeof(*event) + ec_dev->event_size;
>  
>  	if (!(event_bit & priv->event_mask) ||
>  	    (priv->event_len + total_size) > CROS_MAX_EVENT_LEN)
> @@ -206,8 +223,11 @@ static int cros_ec_chardev_open(struct inode *inode, struct file *filp)
>  	ret = blocking_notifier_chain_register(&pdata->subscribers,
>  					       &priv->notifier);
>  	if (ret) {
> -		dev_err(pdata->ec_dev->dev,
> -			"failed to register event notifier\n");
> +		scoped_guard(rwsem_read, &pdata->ec_dev_sem) {
> +			if (pdata->ec_dev)
> +				dev_err(pdata->ec_dev->dev,
> +					"failed to register event notifier\n");
> +		}

open is in a context where dev_dev has to be valid, don't add
pointless locking.

If you want to have an assertion it should just be this at the top of the function:

	if (WARN_ON(!priv->pdata->ec_dev))
		return -ENXIO;

> @@ -330,10 +350,18 @@ static long cros_ec_chardev_ioctl_xcmd(struct chardev_priv *priv, void __user *a
>  	}
>  
>  	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;
> +
> +	scoped_guard(rwsem_read, &priv->pdata->ec_dev_sem) {
> +		if (!priv->pdata->ec_dev) {
> +			ret = -ENODEV;
> +			goto exit;
> +		}

Same remark, don't use scoped_guard. Each fops should simply start
with:

	guard(rwsem_read)(&priv->pdata->ec_dev_sem);
	if (!priv->pdata->ec_dev)
		return -ENXIO;

There is no point in trying to carefully partially do some part of the
ioctl of the driver has been removed.

>  static long cros_ec_chardev_ioctl_readmem(struct chardev_priv *priv, void __user *arg)
>  {
> -	struct cros_ec_device *ec_dev = priv->pdata->ec_dev;
> +	struct cros_ec_device *ec_dev;
>  	struct cros_ec_readmem s_mem = { };
>  	long num;
>  
> +	guard(rwsem_read)(&priv->pdata->ec_dev_sem);
> +	if (!priv->pdata->ec_dev)
> +		return -ENODEV;
> +	ec_dev = priv->pdata->ec_dev;

And it should be placed close to the top of the fop, not down inside
every child function.

Your mental model is still thinking about "revocable" this is really
entering a driver bound context at the start of every fop. Then
everything during the fop gets to assume the driver bound invariant.


> @@ -451,6 +485,8 @@ static void cros_ec_chardev_remove(struct platform_device *pdev)
>  
>  	blocking_notifier_chain_unregister(&pdata->ec_dev->event_notifier,
>  					   &pdata->relay);
> +	scoped_guard(rwsem_write, &pdata->ec_dev_sem)
> +		pdata->ec_dev = NULL;

This seems out of order.

  	misc_deregister(&pdata->misc);

^^ Is first because it stops new fops from being created

 +	scoped_guard(rwsem_write, &pdata->ec_dev_sem)
 +		pdata->ec_dev = NULL;

^^ Stops existing fops from running

Then you can go on to destroy the notifier chain and so on as there is
now no concurrent touches to pdata.

Jason

Re: [PATCH v2 4/4] platform/chrome: cros_ec_chardev: Introduce rwsem for protecting ec_dev

[Tzung-Bi Shih]

On Thu, May 21, 2026 at 10:58:30AM -0300, Jason Gunthorpe wrote:
> On Sat, May 16, 2026 at 10:30:17PM +0800, Tzung-Bi Shih wrote:
> > @@ -330,10 +350,18 @@ static long cros_ec_chardev_ioctl_xcmd(struct chardev_priv *priv, void __user *a
> >  	}
> >  
> >  	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;
> > +
> > +	scoped_guard(rwsem_read, &priv->pdata->ec_dev_sem) {
> > +		if (!priv->pdata->ec_dev) {
> > +			ret = -ENODEV;
> > +			goto exit;
> > +		}
> 
> Same remark, don't use scoped_guard. Each fops should simply start
> with:
> 
> 	guard(rwsem_read)(&priv->pdata->ec_dev_sem);
> 	if (!priv->pdata->ec_dev)
> 		return -ENXIO;
> 
> There is no point in trying to carefully partially do some part of the
> ioctl of the driver has been removed.

Fixed those in the next version [1].

Just a note, the code still uses -ENODEV instead of -ENXIO if you have no
objection.

> > @@ -451,6 +485,8 @@ static void cros_ec_chardev_remove(struct platform_device *pdev)
> >  
> >  	blocking_notifier_chain_unregister(&pdata->ec_dev->event_notifier,
> >  					   &pdata->relay);
> > +	scoped_guard(rwsem_write, &pdata->ec_dev_sem)
> > +		pdata->ec_dev = NULL;
> 
> This seems out of order.
> 
>   	misc_deregister(&pdata->misc);
> 
> ^^ Is first because it stops new fops from being created
> 
>  +	scoped_guard(rwsem_write, &pdata->ec_dev_sem)
>  +		pdata->ec_dev = NULL;
> 
> ^^ Stops existing fops from running
> 
> Then you can go on to destroy the notifier chain and so on as there is
> now no concurrent touches to pdata.

Fixed in the next version [1].

[1] https://lore.kernel.org/all/20260525052654.4076429-5-tzungbi@kernel.org

Re: [PATCH v2 0/4] platform/chrome: cros_ec_chardev: Fix a potential UAF

[Jason Gunthorpe]

On Sat, May 16, 2026 at 10:30:13PM +0800, Tzung-Bi Shih wrote:
> This series addresses a potential Use-After-Free error when a device is
> deregistered while file operations are still in progress or files remain
> open in cros_ec_chardev.
> 
> - Patches 1 to 3 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()`.
> - Patch 4 introduces a rwsem for protecting `ec_dev` to prevent the UAF
>   error.

Other than my fairly minor remarks on the last patch this whole thing
looks good to me

Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>

Jason