diff options
| author | Johan Hovold <johan@kernel.org> | 2026-02-04 15:28:49 +0100 |
|---|---|---|
| committer | Greg Kroah-Hartman <gregkh@linuxfoundation.org> | 2026-02-06 16:08:48 +0100 |
| commit | 21bab791346e5b7902a04709231c0642ff6d69bc (patch) | |
| tree | 9745de59f6794843d33e096fb93e809ae4ae32ac /drivers/base | |
| parent | 7149ce34dd48886b3f69153c7f5533dd3fd5f47e (diff) | |
Revert "revocable: Revocable resource management"
This reverts commit 62eb557580eb2177cf16c3fd2b6efadff297b29a.
The revocable implementation uses two separate abstractions, struct
revocable_provider and struct revocable, in order to store the SRCU read
lock index which must be passed unaltered to srcu_read_unlock() in the
same context when a resource is no longer needed.
With the merged revocable API, multiple threads could however share the
same struct revocable and therefore potentially overwrite the SRCU index
of another thread which can cause the SRCU synchronisation in
revocable_provider_revoke() to never complete. [1]
An example revocable conversion of the gpiolib code also turned out to
be fundamentally flawed and could lead to use-after-free. [2]
An attempt to address both issues was quickly put together and merged,
but revocable is still fundamentally broken. [3]
Specifically, the latest design relies on RCU for storing a pointer to
the revocable provider, but since the resource can be shared by value
(e.g. as in the now reverted selftests) this does not work at all and
can also lead to use-after-free:
static void revocable_provider_release(struct kref *kref)
{
struct revocable_provider *rp = container_of(kref,
struct revocable_provider, kref);
cleanup_srcu_struct(&rp->srcu);
kfree_rcu(rp, rcu);
}
void revocable_provider_revoke(struct revocable_provider __rcu **rp_ptr)
{
struct revocable_provider *rp;
rp = rcu_replace_pointer(*rp_ptr, NULL, 1);
...
kref_put(&rp->kref, revocable_provider_release);
}
int revocable_init(struct revocable_provider __rcu *_rp,
struct revocable *rev)
{
struct revocable_provider *rp;
...
scoped_guard(rcu) {
rp = rcu_dereference(_rp);
if (!rp)
return -ENODEV;
if (!kref_get_unless_zero(&rp->kref))
return -ENODEV;
}
...
}
producer:
priv->rp = revocable_provider_alloc(&priv->res);
// pass priv->rp by value to consumer
revocable_provider_revoke(&priv->rp);
consumer:
struct revocable_provider __rcu *rp = filp->private_data;
struct revocable *rev;
revocable_init(rp, &rev);
as _rp would still be non-NULL in revocable_init() regardless of whether
the producer has revoked the resource and set its pointer to NULL.
Essentially revocable still relies on having a pointer to reference
counted driver data which holds the revocable provider, which makes all
the RCU protection unnecessary along with most of the current revocable
design and implementation.
As the above shows, and as has been pointed out repeatedly elsewhere,
these kind of issues are not something that should be addressed
incrementally. [4]
Revert the revocable implementation until a redesign has been proposed
and evaluated properly.
Link: https://lore.kernel.org/all/20260124170535.11756-4-johan@kernel.org/ [1]
Link: https://lore.kernel.org/all/aXT45B6vLf9R3Pbf@hovoldconsulting.com/ [2]
Link: https://lore.kernel.org/all/20260129143733.45618-1-tzungbi@kernel.org/ [3]
Link: https://lore.kernel.org/all/aXobzoeooJqxMkEj@hovoldconsulting.com/ [4]
Signed-off-by: Johan Hovold <johan@kernel.org>
Link: https://patch.msgid.link/20260204142849.22055-4-johan@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Diffstat (limited to 'drivers/base')
| -rw-r--r-- | drivers/base/revocable.c | 225 |
1 files changed, 0 insertions, 225 deletions
diff --git a/drivers/base/revocable.c b/drivers/base/revocable.c deleted file mode 100644 index 8532ca6a371c..000000000000 --- a/drivers/base/revocable.c +++ /dev/null @@ -1,225 +0,0 @@ -// SPDX-License-Identifier: GPL-2.0 -/* - * Copyright 2026 Google LLC - * - * Revocable resource management - */ - -#include <linux/device.h> -#include <linux/kref.h> -#include <linux/revocable.h> -#include <linux/slab.h> -#include <linux/srcu.h> - -/** - * DOC: Overview - * - * The "revocable" mechanism is a synchronization primitive designed to manage - * safe access to resources that can be asynchronously removed or invalidated. - * Its primary purpose is to prevent Use-After-Free (UAF) errors when - * interacting with resources whose lifetimes are not guaranteed to outlast - * their consumers. - * - * This is particularly useful in systems where resources can disappear - * unexpectedly, such as those provided by hot-pluggable devices like USB. - * When a consumer holds a reference to such a resource, the underlying device - * might be removed, causing the resource's memory to be freed. Subsequent - * access attempts by the consumer would then lead to UAF errors. - * - * Revocable addresses this by providing a form of "weak reference" and a - * controlled access method. It allows a resource consumer to safely attempt to - * access the resource. The mechanism guarantees that any access granted is - * valid for the duration of its use. If the resource has already been - * revoked (i.e., freed), the access attempt will fail safely, typically by - * returning NULL, instead of causing a crash. - * - * The implementation uses a provider/consumer model built on Sleepable - * RCU (SRCU) to guarantee safe memory access: - * - * - A resource provider, such as a driver for a hot-pluggable device, - * allocates a struct revocable_provider and initializes it with a pointer - * to the resource. - * - * - A resource consumer that wants to access the resource allocates a - * struct revocable which acts as a handle containing a reference to the - * provider. - * - * - To access the resource, the consumer uses revocable_try_access(). - * This function enters an SRCU read-side critical section and returns - * the pointer to the resource. If the provider has already freed the - * resource, it returns NULL. After use, the consumer calls - * revocable_withdraw_access() to exit the SRCU critical section. The - * REVOCABLE_TRY_ACCESS_WITH() and REVOCABLE_TRY_ACCESS_SCOPED() are - * convenient helpers for doing that. - * - * - When the provider needs to remove the resource, it calls - * revocable_provider_revoke(). This function sets the internal resource - * pointer to NULL and then calls synchronize_srcu() to wait for all - * current readers to finish before the resource can be completely torn - * down. - */ - -/** - * struct revocable_provider - A handle for resource provider. - * @srcu: The SRCU to protect the resource. - * @res: The pointer of resource. It can point to anything. - * @kref: The refcount for this handle. - * @rcu: The RCU to protect pointer to itself. - */ -struct revocable_provider { - struct srcu_struct srcu; - void __rcu *res; - struct kref kref; - struct rcu_head rcu; -}; - -/** - * revocable_provider_alloc() - Allocate struct revocable_provider. - * @res: The pointer of resource. - * - * This holds an initial refcount to the struct. - * - * Return: The pointer of struct revocable_provider. NULL on errors. - * It enforces the caller handles the returned pointer in RCU ways. - */ -struct revocable_provider __rcu *revocable_provider_alloc(void *res) -{ - struct revocable_provider *rp; - - rp = kzalloc(sizeof(*rp), GFP_KERNEL); - if (!rp) - return NULL; - - init_srcu_struct(&rp->srcu); - RCU_INIT_POINTER(rp->res, res); - kref_init(&rp->kref); - - return (struct revocable_provider __rcu *)rp; -} -EXPORT_SYMBOL_GPL(revocable_provider_alloc); - -static void revocable_provider_release(struct kref *kref) -{ - struct revocable_provider *rp = container_of(kref, - struct revocable_provider, kref); - - cleanup_srcu_struct(&rp->srcu); - kfree_rcu(rp, rcu); -} - -/** - * revocable_provider_revoke() - Revoke the managed resource. - * @rp_ptr: The pointer of pointer of resource provider. - * - * This sets the resource `(struct revocable_provider *)->res` to NULL to - * indicate the resource has gone. - * - * This drops the refcount to the resource provider. If it is the final - * reference, revocable_provider_release() will be called to free the struct. - * - * It enforces the caller to pass a pointer of pointer of resource provider so - * that it sets \*rp_ptr to NULL to prevent from keeping a dangling pointer. - */ -void revocable_provider_revoke(struct revocable_provider __rcu **rp_ptr) -{ - struct revocable_provider *rp; - - rp = rcu_replace_pointer(*rp_ptr, NULL, 1); - if (!rp) - return; - - rcu_assign_pointer(rp->res, NULL); - synchronize_srcu(&rp->srcu); - kref_put(&rp->kref, revocable_provider_release); -} -EXPORT_SYMBOL_GPL(revocable_provider_revoke); - -/** - * revocable_init() - Initialize struct revocable. - * @_rp: The pointer of resource provider. - * @rev: The pointer of resource consumer. - * - * This holds a refcount to the resource provider. - * - * Return: 0 on success, -errno otherwise. - */ -int revocable_init(struct revocable_provider __rcu *_rp, struct revocable *rev) -{ - struct revocable_provider *rp; - - if (!_rp) - return -ENODEV; - - /* - * Enter a read-side critical section. - * - * This prevents kfree_rcu() from freeing the struct revocable_provider - * memory, for the duration of this scope. - */ - scoped_guard(rcu) { - rp = rcu_dereference(_rp); - if (!rp) - /* The revocable provider has been revoked. */ - return -ENODEV; - - if (!kref_get_unless_zero(&rp->kref)) - /* - * The revocable provider is releasing (i.e., - * revocable_provider_release() has been called). - */ - return -ENODEV; - } - /* At this point, `rp` is safe to access as holding a kref of it */ - - rev->rp = rp; - return 0; -} -EXPORT_SYMBOL_GPL(revocable_init); - -/** - * revocable_deinit() - Deinitialize struct revocable. - * @rev: The pointer of struct revocable. - * - * This drops a refcount to the resource provider. If it is the final - * reference, revocable_provider_release() will be called to free the struct. - */ -void revocable_deinit(struct revocable *rev) -{ - struct revocable_provider *rp = rev->rp; - - kref_put(&rp->kref, revocable_provider_release); -} -EXPORT_SYMBOL_GPL(revocable_deinit); - -/** - * revocable_try_access() - Try to access the resource. - * @rev: The pointer of struct revocable. - * - * This tries to de-reference to the resource and enters a RCU critical - * section. - * - * Return: The pointer to the resource. NULL if the resource has gone. - */ -void *revocable_try_access(struct revocable *rev) __acquires(&rev->rp->srcu) -{ - struct revocable_provider *rp = rev->rp; - - rev->idx = srcu_read_lock(&rp->srcu); - return srcu_dereference(rp->res, &rp->srcu); -} -EXPORT_SYMBOL_GPL(revocable_try_access); - -/** - * revocable_withdraw_access() - Stop accessing to the resource. - * @rev: The pointer of struct revocable. - * - * Call this function to indicate the resource is no longer used. It exits - * the RCU critical section. - */ -void revocable_withdraw_access(struct revocable *rev) __releases(&rev->rp->srcu) -{ - struct revocable_provider *rp = rev->rp; - - srcu_read_unlock(&rp->srcu, rev->idx); -} -EXPORT_SYMBOL_GPL(revocable_withdraw_access); |
