The series is separated from [1] to show the independency and compare potential use cases easier. This use case uses the primitive revocable APIs directly. It relies on the revocable core part [2].
It tries to fix an UAF in the fops of cros_ec_chardev after the underlying protocol device has gone by using revocable.
The file operations make sure the resources are available when using them.
Even though it has the finest grain for accessing the resources, it makes the user code verbose. Per feedback from the community, I'm looking for some subsystem level helpers so that user code can be simlper.
The 1st patch converts existing protocol devices to resource providers of cros_ec_device.
The 2nd patch converts cros_ec_chardev to a resource consumer of cros_ec_device to fix the UAF.
[1] https://lore.kernel.org/chrome-platform/20251016054204.1523139-1-tzungbi@ker... [2] https://lore.kernel.org/chrome-platform/20251106152330.11733-1-tzungbi@kerne...
v6: - New, separated from an existing series.
Tzung-Bi Shih (2): platform/chrome: Protect cros_ec_device lifecycle with revocable platform/chrome: cros_ec_chardev: Consume cros_ec_device via revocable
drivers/platform/chrome/cros_ec.c | 5 ++ drivers/platform/chrome/cros_ec_chardev.c | 71 ++++++++++++++++----- include/linux/platform_data/cros_ec_proto.h | 4 ++ 3 files changed, 65 insertions(+), 15 deletions(-)
The cros_ec_device can be unregistered when the underlying device is removed. Other kernel drivers that interact with the EC may hold a pointer to the cros_ec_device, creating a risk of a use-after-free error if the EC device is removed while still being referenced.
To prevent this, leverage the revocable and convert the underlying device drivers to resource providers of cros_ec_device.
Signed-off-by: Tzung-Bi Shih tzungbi@kernel.org --- v6: - No changes.
v5: https://lore.kernel.org/chrome-platform/20251016054204.1523139-5-tzungbi@ker... - No changes.
v4: https://lore.kernel.org/chrome-platform/20250923075302.591026-5-tzungbi@kern... - No changes.
v3: https://lore.kernel.org/chrome-platform/20250912081718.3827390-5-tzungbi@ker... - Initialize the revocable provider in cros_ec_device_alloc() instead of spreading in protocol device drivers.
v2: https://lore.kernel.org/chrome-platform/20250820081645.847919-5-tzungbi@kern... - Rename "ref_proxy" -> "revocable".
v1: https://lore.kernel.org/chrome-platform/20250814091020.1302888-3-tzungbi@ker...
drivers/platform/chrome/cros_ec.c | 5 +++++ include/linux/platform_data/cros_ec_proto.h | 4 ++++ 2 files changed, 9 insertions(+)
diff --git a/drivers/platform/chrome/cros_ec.c b/drivers/platform/chrome/cros_ec.c index 1da79e3d215b..95e3e898e3da 100644 --- a/drivers/platform/chrome/cros_ec.c +++ b/drivers/platform/chrome/cros_ec.c @@ -16,6 +16,7 @@ #include <linux/platform_device.h> #include <linux/platform_data/cros_ec_commands.h> #include <linux/platform_data/cros_ec_proto.h> +#include <linux/revocable.h> #include <linux/slab.h> #include <linux/suspend.h>
@@ -47,6 +48,10 @@ struct cros_ec_device *cros_ec_device_alloc(struct device *dev) if (!ec_dev) return NULL;
+ ec_dev->revocable_provider = devm_revocable_provider_alloc(dev, ec_dev); + if (!ec_dev->revocable_provider) + return NULL; + ec_dev->din_size = sizeof(struct ec_host_response) + sizeof(struct ec_response_get_protocol_info) + EC_MAX_RESPONSE_OVERHEAD; diff --git a/include/linux/platform_data/cros_ec_proto.h b/include/linux/platform_data/cros_ec_proto.h index de14923720a5..fbb6ca34a40f 100644 --- a/include/linux/platform_data/cros_ec_proto.h +++ b/include/linux/platform_data/cros_ec_proto.h @@ -12,6 +12,7 @@ #include <linux/lockdep_types.h> #include <linux/mutex.h> #include <linux/notifier.h> +#include <linux/revocable.h>
#include <linux/platform_data/cros_ec_commands.h>
@@ -165,6 +166,7 @@ struct cros_ec_command { * @pd: The platform_device used by the mfd driver to interface with the * PD behind an EC. * @panic_notifier: EC panic notifier. + * @revocable_provider: The revocable_provider to this device. */ struct cros_ec_device { /* These are used by other drivers that want to talk to the EC */ @@ -211,6 +213,8 @@ struct cros_ec_device { struct platform_device *pd;
struct blocking_notifier_head panic_notifier; + + struct revocable_provider *revocable_provider; };
/**
Hi Tzung-Bi,
kernel test robot noticed the following build errors:
[auto build test ERROR on chrome-platform/for-next] [also build test ERROR on chrome-platform/for-firmware-next driver-core/driver-core-testing driver-core/driver-core-next driver-core/driver-core-linus linus/master v6.18-rc4 next-20251107] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Tzung-Bi-Shih/platform-chrome... base: https://git.kernel.org/pub/scm/linux/kernel/git/chrome-platform/linux.git for-next patch link: https://lore.kernel.org/r/20251106152602.11814-2-tzungbi%40kernel.org patch subject: [PATCH v6 1/2] platform/chrome: Protect cros_ec_device lifecycle with revocable config: hexagon-randconfig-002-20251107 (https://download.01.org/0day-ci/archive/20251107/202511071425.qwhK2D9r-lkp@i...) compiler: clang version 22.0.0git (https://github.com/llvm/llvm-project d2625a438020ad35330cda29c3def102c1687b1b) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251107/202511071425.qwhK2D9r-lkp@i...)
If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot lkp@intel.com | Closes: https://lore.kernel.org/oe-kbuild-all/202511071425.qwhK2D9r-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from drivers/i2c/busses/i2c-cros-ec-tunnel.c:10:
include/linux/platform_data/cros_ec_proto.h:15:10: fatal error: 'linux/revocable.h' file not found
15 | #include <linux/revocable.h> | ^~~~~~~~~~~~~~~~~~~ 1 error generated.
vim +15 include/linux/platform_data/cros_ec_proto.h
10 11 #include <linux/device.h> 12 #include <linux/lockdep_types.h> 13 #include <linux/mutex.h> 14 #include <linux/notifier.h>
15 #include <linux/revocable.h>
16
Hi Tzung-Bi,
kernel test robot noticed the following build errors:
[auto build test ERROR on chrome-platform/for-next] [also build test ERROR on chrome-platform/for-firmware-next driver-core/driver-core-testing driver-core/driver-core-next driver-core/driver-core-linus linus/master v6.18-rc4 next-20251107] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Tzung-Bi-Shih/platform-chrome... base: https://git.kernel.org/pub/scm/linux/kernel/git/chrome-platform/linux.git for-next patch link: https://lore.kernel.org/r/20251106152602.11814-2-tzungbi%40kernel.org patch subject: [PATCH v6 1/2] platform/chrome: Protect cros_ec_device lifecycle with revocable config: arc-randconfig-r072-20251107 (https://download.01.org/0day-ci/archive/20251107/202511071536.lzOOC7SE-lkp@i...) compiler: arc-linux-gcc (GCC) 14.3.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251107/202511071536.lzOOC7SE-lkp@i...)
If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot lkp@intel.com | Closes: https://lore.kernel.org/oe-kbuild-all/202511071536.lzOOC7SE-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from gpio-cros-ec.c:18:
include/linux/platform_data/cros_ec_proto.h:15:10: fatal error: linux/revocable.h: No such file or directory
15 | #include <linux/revocable.h> | ^~~~~~~~~~~~~~~~~~~ compilation terminated.
vim +15 include/linux/platform_data/cros_ec_proto.h
10 11 #include <linux/device.h> 12 #include <linux/lockdep_types.h> 13 #include <linux/mutex.h> 14 #include <linux/notifier.h>
15 #include <linux/revocable.h>
16
The cros_ec_chardev driver provides a character device interface to the ChromeOS EC. A file handle to this device can remain open in userspace even if the underlying EC device is removed.
This creates a classic use-after-free vulnerability. Any file operation (ioctl, release, etc.) on the open handle after the EC device has gone would access a stale pointer, leading to a system crash.
To prevent this, leverage the revocable and convert cros_ec_chardev to a resource consumer of cros_ec_device.
Signed-off-by: Tzung-Bi Shih tzungbi@kernel.org --- v6: - Rename REVOCABLE_TRY_ACCESS_WITH() -> REVOCABLE_TRY_ACCESS_SCOPED(). - Use new REVOCABLE_TRY_ACCESS_WITH() if applicable.
v4-v5: - Doesn't exist.
v3: https://lore.kernel.org/chrome-platform/20250912081718.3827390-6-tzungbi@ker... - Use specific labels for different cleanup in cros_ec_chardev_open().
v2: https://lore.kernel.org/chrome-platform/20250820081645.847919-6-tzungbi@kern... - Rename "ref_proxy" -> "revocable". - Fix a sparse warning by removing the redundant __rcu annotation.
v1: https://lore.kernel.org/chrome-platform/20250814091020.1302888-4-tzungbi@ker...
drivers/platform/chrome/cros_ec_chardev.c | 71 ++++++++++++++++++----- 1 file changed, 56 insertions(+), 15 deletions(-)
diff --git a/drivers/platform/chrome/cros_ec_chardev.c b/drivers/platform/chrome/cros_ec_chardev.c index c9d80ad5b57e..bc152c206fb8 100644 --- a/drivers/platform/chrome/cros_ec_chardev.c +++ b/drivers/platform/chrome/cros_ec_chardev.c @@ -22,6 +22,7 @@ #include <linux/platform_data/cros_ec_proto.h> #include <linux/platform_device.h> #include <linux/poll.h> +#include <linux/revocable.h> #include <linux/slab.h> #include <linux/types.h> #include <linux/uaccess.h> @@ -32,7 +33,7 @@ #define CROS_MAX_EVENT_LEN PAGE_SIZE
struct chardev_priv { - struct cros_ec_device *ec_dev; + struct revocable *ec_dev_rev; struct notifier_block notifier; wait_queue_head_t wait_event; unsigned long event_mask; @@ -55,6 +56,7 @@ static int ec_get_version(struct chardev_priv *priv, char *str, int maxlen) }; struct ec_response_get_version *resp; struct cros_ec_command *msg; + struct cros_ec_device *ec_dev; int ret;
msg = kzalloc(sizeof(*msg) + sizeof(*resp), GFP_KERNEL); @@ -64,7 +66,13 @@ static int ec_get_version(struct chardev_priv *priv, char *str, int maxlen) msg->command = EC_CMD_GET_VERSION + priv->cmd_offset; msg->insize = sizeof(*resp);
- ret = cros_ec_cmd_xfer_status(priv->ec_dev, msg); + REVOCABLE_TRY_ACCESS_WITH(priv->ec_dev_rev, ec_dev); + if (!ec_dev) { + ret = -ENODEV; + goto exit; + } + + ret = cros_ec_cmd_xfer_status(ec_dev, msg); if (ret < 0) { snprintf(str, maxlen, "Unknown EC version, returned error: %d\n", @@ -92,10 +100,17 @@ 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; 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; + + REVOCABLE_TRY_ACCESS_WITH(priv->ec_dev_rev, ec_dev); + if (!ec_dev) + return NOTIFY_DONE; + + 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) @@ -166,7 +181,12 @@ static int cros_ec_chardev_open(struct inode *inode, struct file *filp) if (!priv) return -ENOMEM;
- priv->ec_dev = ec_dev; + priv->ec_dev_rev = revocable_alloc(ec_dev->revocable_provider); + if (!priv->ec_dev_rev) { + ret = -ENOMEM; + goto free_priv; + } + priv->cmd_offset = ec->cmd_offset; filp->private_data = priv; INIT_LIST_HEAD(&priv->events); @@ -178,9 +198,14 @@ static int cros_ec_chardev_open(struct inode *inode, struct file *filp) &priv->notifier); if (ret) { dev_err(ec_dev->dev, "failed to register event notifier\n"); - kfree(priv); + goto free_revocable; }
+ return 0; +free_revocable: + revocable_free(priv->ec_dev_rev); +free_priv: + kfree(priv); return ret; }
@@ -251,11 +276,15 @@ 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 cros_ec_device *ec_dev; struct ec_event *event, *e;
- blocking_notifier_chain_unregister(&ec_dev->event_notifier, - &priv->notifier); + REVOCABLE_TRY_ACCESS_SCOPED(priv->ec_dev_rev, ec_dev) { + if (ec_dev) + blocking_notifier_chain_unregister(&ec_dev->event_notifier, + &priv->notifier); + } + revocable_free(priv->ec_dev_rev);
list_for_each_entry_safe(event, e, &priv->events, node) { list_del(&event->node); @@ -273,6 +302,7 @@ static long cros_ec_chardev_ioctl_xcmd(struct chardev_priv *priv, void __user *a { struct cros_ec_command *s_cmd; struct cros_ec_command u_cmd; + struct cros_ec_device *ec_dev; long ret;
if (copy_from_user(&u_cmd, arg, sizeof(u_cmd))) @@ -299,10 +329,17 @@ static long cros_ec_chardev_ioctl_xcmd(struct chardev_priv *priv, void __user *a }
s_cmd->command += priv->cmd_offset; - ret = cros_ec_cmd_xfer(priv->ec_dev, s_cmd); - /* Only copy data to userland if data was received. */ - if (ret < 0) - goto exit; + REVOCABLE_TRY_ACCESS_SCOPED(priv->ec_dev_rev, ec_dev) { + if (!ec_dev) { + ret = -ENODEV; + goto exit; + } + + ret = cros_ec_cmd_xfer(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; @@ -313,10 +350,14 @@ 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; struct cros_ec_readmem s_mem = { }; long num;
+ REVOCABLE_TRY_ACCESS_WITH(priv->ec_dev_rev, ec_dev); + if (!ec_dev) + return -ENODEV; + /* Not every platform supports direct reads */ if (!ec_dev->cmd_readmem) return -ENOTTY;
On Thu, Nov 06, 2025 at 11:26:02PM +0800, Tzung-Bi Shih wrote:
@@ -166,7 +181,12 @@ static int cros_ec_chardev_open(struct inode *inode, struct file *filp) if (!priv) return -ENOMEM;
- priv->ec_dev = ec_dev;
- priv->ec_dev_rev = revocable_alloc(ec_dev->revocable_provider);
- if (!priv->ec_dev_rev) {
ret = -ENOMEM;goto free_priv;- }
The lifecyle of ec_dev->ec_dev->revocable_provider memory is controlled by dev:
+ ec_dev->revocable_provider = devm_revocable_provider_alloc(dev, ec_dev);
Under the lifecycle of some other driver.
The above only works because misc calls open under the misc_mtx so it open has "sync" behavior during misc_unregister, and other rules ensure that ec_dev is valid during the full lifecycle of this driver.
So, I think this cross-driver design an abusive use of the revocable idea.
It should not be allocated by the parent driver, it should be fully contained to this driver alone and used only to synchronize the fops. This would make it clear that the ec_dev pointer must be valid during the *entire* lifecycle of this driver.
What you have here by putting the providing in another driver is too magic and obfuscates what the actual lifetime rules are while providing a giant foot gun for someone to think that just because it is marked revocable it is fully safe to touch revocable_provider at any time.
Broadly I think embedding a revocable in the memory that it is trying to protect is probably an anti-pattern as you must somehow already have a valid pointer to thing to get the revocable in the first place. This severely muddies the whole notion of when it can actually be revoked nor not.
Jason
linux-kselftest-mirror@lists.linaro.org
-
Jason Gunthorpe -
kernel test robot -
Tzung-Bi Shih