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.