[PATCH v6 0/2] platform/chrome: Fix an UAF via revocable primitive APIs

[PATCH v6 0/2] platform/chrome: Fix an UAF via revocable primitive APIs

[Tzung-Bi Shih]

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@kernel.org
[2] https://lore.kernel.org/chrome-platform/20251106152330.11733-1-tzungbi@kernel.org/

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(-)

-- 
2.48.1

[PATCH v6 1/2] platform/chrome: Protect cros_ec_device lifecycle with revocable

[Tzung-Bi Shih]

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@kernel.org
- No changes.

v4: https://lore.kernel.org/chrome-platform/20250923075302.591026-5-tzungbi@kernel.org
- No changes.

v3: https://lore.kernel.org/chrome-platform/20250912081718.3827390-5-tzungbi@kernel.org
- 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@kernel.org
- Rename "ref_proxy" -> "revocable".

v1: https://lore.kernel.org/chrome-platform/20250814091020.1302888-3-tzungbi@kernel.org

 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;
 };
 
 /**
-- 
2.48.1

Re: [PATCH v6 1/2] platform/chrome: Protect cros_ec_device lifecycle with revocable

[kernel test robot]

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-Protect-cros_ec_device-lifecycle-with-revocable/20251106-233349
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@intel.com/config)
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@intel.com/reproduce)

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	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v6 1/2] platform/chrome: Protect cros_ec_device lifecycle with revocable

[kernel test robot]

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-Protect-cros_ec_device-lifecycle-with-revocable/20251106-233349
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@intel.com/config)
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@intel.com/reproduce)

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	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

[PATCH v6 2/2] platform/chrome: cros_ec_chardev: Consume cros_ec_device via revocable

[Tzung-Bi Shih]

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@kernel.org/
- Use specific labels for different cleanup in cros_ec_chardev_open().

v2: https://lore.kernel.org/chrome-platform/20250820081645.847919-6-tzungbi@kernel.org
- 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@kernel.org

 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;
-- 
2.48.1

Re: [PATCH v6 2/2] platform/chrome: cros_ec_chardev: Consume cros_ec_device via revocable

[Jason Gunthorpe]

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

Re: [PATCH v6 2/2] platform/chrome: cros_ec_chardev: Consume cros_ec_device via revocable

[Tzung-Bi Shih]

On Thu, Nov 06, 2025 at 11:59:51AM -0400, Jason Gunthorpe wrote:
> 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

My understanding is that the file is available to be opened if and only if
the miscdevice is registered.  Are there any other exceptions or scenarios
I might be unaware of?

> ensure that ec_dev is valid during the full lifecycle of this driver.

To clarify, ec_dev is only required to be valid during the .open() call
itself, not for the entire lifecycle of the driver.  Since ec_dev can
become invalid at any other time, the driver uses ec_dev_rev to ensure
safe access.

> 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
  ^^^^
ec_dev_rev serves this purpose, not revocable_provider.

> 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.

ec_dev->revocable_provider should only be accessed directly within the
.open(), as ec_dev is guaranteed to be valid there.  For all other cases,
it uses ec_dev_rev and checks the validity with revocable_try_access()
to determine if ec_dev has been revoked.

Re: [PATCH v6 2/2] platform/chrome: cros_ec_chardev: Consume cros_ec_device via revocable

[Jason Gunthorpe]

On Wed, Nov 26, 2025 at 04:16:29AM +0000, Tzung-Bi Shih wrote:
> On Thu, Nov 06, 2025 at 11:59:51AM -0400, Jason Gunthorpe wrote:
> > 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
> 
> My understanding is that the file is available to be opened if and only if
> the miscdevice is registered.  

Yes, through misc_mtx.

> > ensure that ec_dev is valid during the full lifecycle of this driver.
> 
> To clarify, ec_dev is only required to be valid during the .open() call
> itself, not for the entire lifecycle of the driver.  Since ec_dev can
> become invalid at any other time, the driver uses ec_dev_rev to ensure
> safe access.

open can be called during the entire lifecycle of the driver,
misc_deregister() is called during remove. So this is a meaningless
distinction.

ec_dev cannot become invalid while the driver is bound.

> > 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
>   ^^^^
> ec_dev_rev serves this purpose, not revocable_provider.

How does this detail matter? It is still created by the wrong 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.
> 
> ec_dev->revocable_provider should only be accessed directly within the
> .open(), as ec_dev is guaranteed to be valid there.  For all other cases,
> it uses ec_dev_rev and checks the validity with revocable_try_access()
> to determine if ec_dev has been revoked.

I understand what this does and why it works, I am saying it is an
anti-pattern bad design to cross a revocable between two drivers like
this.

You want the driver creating the fops to revoke a pointer from its own
fops - not span across multiple drivers to achieve the same thing. It
significantly confuses what the actual lifecycle rules are.

Jason