[PATCH 1/4] revocable: Fix races in revocable_alloc() using RCU

Tzung-Bi Shih
2026-01-29T14:38:03Z

There are two race conditions when allocating a revocable instance:

1. After a struct revocable_provider is revoked, the caller might still
hold a dangling pointer to it. A subsequent call to
revocable_alloc() can trigger a use-after-free.
2. If revocable_provider_release() runs concurrently with
revocable_alloc(), the memory of struct revocable_provider can be
accessed during or after kfree().

To fix these:
- Manage the lifetime of struct revocable_provider using RCU. Annotate
pointers to it with __rcu and use kfree_rcu() for deallocation.
- Update revocable_alloc() to safely acquire a reference using RCU
primitives.
- Update revocable_provider_revoke() to take a double pointer (`**rp`).
It atomically NULLs out the caller's pointer before starting
revocation. This prevents the caller from holding a dangling pointer.
- Drop devm_revocable_provider_alloc(). The devm-managed model cannot
support the required double-pointer semantic for safe pointer nulling.

Reported-by: Johan Hovold <johan@kernel.org>
Closes: https://lore.kernel.org/all/aXdy-b3GOJkzGqYo@hovoldconsulting.com/
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
.../driver-api/driver-model/revocable.rst | 3 -
drivers/base/revocable.c | 93 ++++++++++---------
drivers/base/revocable_test.c | 20 ++--
include/linux/revocable.h | 8 +-
.../revocable/test_modules/revocable_test.c | 19 ++--
5 files changed, 72 insertions(+), 71 deletions(-)

diff --git a/Documentation/driver-api/driver-model/revocable.rst b/Documentation/driver-api/driver-model/revocable.rst
index 22a442cc8d7f..ab7c0af5fbb6 100644
--- a/Documentation/driver-api/driver-model/revocable.rst
+++ b/Documentation/driver-api/driver-model/revocable.rst
@@ -76,9 +76,6 @@ For Resource Providers
.. kernel-doc:: drivers/base/revocable.c
:identifiers: revocable_provider_alloc

-.. kernel-doc:: drivers/base/revocable.c
- :identifiers: devm_revocable_provider_alloc
-
.. kernel-doc:: drivers/base/revocable.c
:identifiers: revocable_provider_revoke

diff --git a/drivers/base/revocable.c b/drivers/base/revocable.c
index b068e18a847d..1bcd1cf54764 100644
--- a/drivers/base/revocable.c
+++ b/drivers/base/revocable.c
@@ -64,11 +64,13 @@
* @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;
};

/**
@@ -88,8 +90,9 @@ struct revocable {
* 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 *revocable_provider_alloc(void *res)
+struct revocable_provider __rcu *revocable_provider_alloc(void *res)
{
struct revocable_provider *rp;

@@ -98,10 +101,10 @@ struct revocable_provider *revocable_provider_alloc(void *res)
return NULL;

init_srcu_struct(&rp->srcu);
- rcu_assign_pointer(rp->res, res);
+ RCU_INIT_POINTER(rp->res, res);
kref_init(&rp->kref);

- return rp;
+ return (struct revocable_provider __rcu *)rp;
}
EXPORT_SYMBOL_GPL(revocable_provider_alloc);

@@ -111,82 +114,80 @@ static void revocable_provider_release(struct kref *kref)
struct revocable_provider, kref);

cleanup_srcu_struct(&rp->srcu);
- kfree(rp);
+ kfree_rcu(rp, rcu);
}

/**
* revocable_provider_revoke() - Revoke the managed resource.
- * @rp: The pointer of resource provider.
+ * @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.
- */
-void revocable_provider_revoke(struct revocable_provider *rp)
-{
- rcu_assign_pointer(rp->res, NULL);
- synchronize_srcu(&rp->srcu);
- kref_put(&rp->kref, revocable_provider_release);
-}
-EXPORT_SYMBOL_GPL(revocable_provider_revoke);
-
-static void devm_revocable_provider_revoke(void *data)
-{
- struct revocable_provider *rp = data;
-
- revocable_provider_revoke(rp);
-}
-
-/**
- * devm_revocable_provider_alloc() - Dev-managed revocable_provider_alloc().
- * @dev: The device.
- * @res: The pointer of resource.
- *
- * It is convenient to allocate providers via this function if the @res is
- * also tied to the lifetime of the @dev. revocable_provider_revoke() will
- * be called automatically when the device is unbound.
*
- * This holds an initial refcount to the struct.
- *
- * Return: The pointer of struct revocable_provider. NULL on errors.
+ * 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.
*/
-struct revocable_provider *devm_revocable_provider_alloc(struct device *dev,
- void *res)
+void revocable_provider_revoke(struct revocable_provider __rcu **rp_ptr)
{
struct revocable_provider *rp;

- rp = revocable_provider_alloc(res);
+ rp = rcu_replace_pointer(*rp_ptr, NULL, 1);
if (!rp)
- return NULL;
+ return;

- if (devm_add_action_or_reset(dev, devm_revocable_provider_revoke, rp))
- return NULL;
-
- return rp;
+ rcu_assign_pointer(rp->res, NULL);
+ synchronize_srcu(&rp->srcu);
+ kref_put(&rp->kref, revocable_provider_release);
}
-EXPORT_SYMBOL_GPL(devm_revocable_provider_alloc);
+EXPORT_SYMBOL_GPL(revocable_provider_revoke);

/**
* revocable_alloc() - Allocate struct revocable.
- * @rp: The pointer of resource provider.
+ * @_rp: The pointer of resource provider.
*
* This holds a refcount to the resource provider.
*
* Return: The pointer of struct revocable. NULL on errors.
*/
-struct revocable *revocable_alloc(struct revocable_provider *rp)
+struct revocable *revocable_alloc(struct revocable_provider __rcu *_rp)
{
+ struct revocable_provider *rp;
struct revocable *rev;

+ if (!_rp)
+ return NULL;
+
+ /*
+ * 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 NULL;
+
+ if (!kref_get_unless_zero(&rp->kref))
+ /*
+ * The revocable provider is releasing (i.e.,
+ * revocable_provider_release() has been called).
+ */
+ return NULL;
+ }
+ /* At this point, `rp` is safe to access as holding a kref of it */
+
rev = kzalloc(sizeof(*rev), GFP_KERNEL);
- if (!rev)
+ if (!rev) {
+ kref_put(&rp->kref, revocable_provider_release);
return NULL;
+ }

rev->rp = rp;
- kref_get(&rp->kref);
-
return rev;
}
EXPORT_SYMBOL_GPL(revocable_alloc);
diff --git a/drivers/base/revocable_test.c b/drivers/base/revocable_test.c
index 873a44082b6c..1622aae92fd3 100644
--- a/drivers/base/revocable_test.c
+++ b/drivers/base/revocable_test.c
@@ -21,7 +21,7 @@

static void revocable_test_basic(struct kunit *test)
{
- struct revocable_provider *rp;
+ struct revocable_provider __rcu *rp;
struct revocable *rev;
void *real_res = (void *)0x12345678, *res;

@@ -36,12 +36,13 @@ static void revocable_test_basic(struct kunit *test)
revocable_withdraw_access(rev);

revocable_free(rev);
- revocable_provider_revoke(rp);
+ revocable_provider_revoke(&rp);
+ KUNIT_EXPECT_PTR_EQ(test, unrcu_pointer(rp), NULL);
}

static void revocable_test_revocation(struct kunit *test)
{
- struct revocable_provider *rp;
+ struct revocable_provider __rcu *rp;
struct revocable *rev;
void *real_res = (void *)0x12345678, *res;

@@ -55,7 +56,8 @@ static void revocable_test_revocation(struct kunit *test)
KUNIT_EXPECT_PTR_EQ(test, res, real_res);
revocable_withdraw_access(rev);

- revocable_provider_revoke(rp);
+ revocable_provider_revoke(&rp);
+ KUNIT_EXPECT_PTR_EQ(test, unrcu_pointer(rp), NULL);

res = revocable_try_access(rev);
KUNIT_EXPECT_PTR_EQ(test, res, NULL);
@@ -66,7 +68,7 @@ static void revocable_test_revocation(struct kunit *test)

static void revocable_test_try_access_macro(struct kunit *test)
{
- struct revocable_provider *rp;
+ struct revocable_provider __rcu *rp;
struct revocable *rev;
void *real_res = (void *)0x12345678, *res;

@@ -81,7 +83,8 @@ static void revocable_test_try_access_macro(struct kunit *test)
KUNIT_EXPECT_PTR_EQ(test, res, real_res);
}

- revocable_provider_revoke(rp);
+ revocable_provider_revoke(&rp);
+ KUNIT_EXPECT_PTR_EQ(test, unrcu_pointer(rp), NULL);

{
REVOCABLE_TRY_ACCESS_WITH(rev, res);
@@ -93,7 +96,7 @@ static void revocable_test_try_access_macro(struct kunit *test)

static void revocable_test_try_access_macro2(struct kunit *test)
{
- struct revocable_provider *rp;
+ struct revocable_provider __rcu *rp;
struct revocable *rev;
void *real_res = (void *)0x12345678, *res;
bool accessed;
@@ -111,7 +114,8 @@ static void revocable_test_try_access_macro2(struct kunit *test)
}
KUNIT_EXPECT_TRUE(test, accessed);

- revocable_provider_revoke(rp);
+ revocable_provider_revoke(&rp);
+ KUNIT_EXPECT_PTR_EQ(test, unrcu_pointer(rp), NULL);

accessed = false;
REVOCABLE_TRY_ACCESS_SCOPED(rev, res) {
diff --git a/include/linux/revocable.h b/include/linux/revocable.h
index 659ba01c58db..d5da3584adbe 100644
--- a/include/linux/revocable.h
+++ b/include/linux/revocable.h
@@ -13,12 +13,10 @@ struct device;
struct revocable;
struct revocable_provider;

-struct revocable_provider *revocable_provider_alloc(void *res);
-void revocable_provider_revoke(struct revocable_provider *rp);
-struct revocable_provider *devm_revocable_provider_alloc(struct device *dev,
- void *res);
+struct revocable_provider __rcu *revocable_provider_alloc(void *res);
+void revocable_provider_revoke(struct revocable_provider __rcu **rp);

-struct revocable *revocable_alloc(struct revocable_provider *rp);
+struct revocable *revocable_alloc(struct revocable_provider __rcu *rp);
void revocable_free(struct revocable *rev);
void *revocable_try_access(struct revocable *rev) __acquires(&rev->rp->srcu);
void revocable_withdraw_access(struct revocable *rev) __releases(&rev->rp->srcu);
diff --git a/tools/testing/selftests/drivers/base/revocable/test_modules/revocable_test.c b/tools/testing/selftests/drivers/base/revocable/test_modules/revocable_test.c
index 1b0692eb75f3..ae6c67e65f3d 100644
--- a/tools/testing/selftests/drivers/base/revocable/test_modules/revocable_test.c
+++ b/tools/testing/selftests/drivers/base/revocable/test_modules/revocable_test.c
@@ -17,7 +17,7 @@
static struct dentry *debugfs_dir;

struct revocable_test_provider_priv {
- struct revocable_provider *rp;
+ struct revocable_provider __rcu *rp;
struct dentry *dentry;
char res[16];
};
@@ -25,7 +25,7 @@ struct revocable_test_provider_priv {
static int revocable_test_consumer_open(struct inode *inode, struct file *filp)
{
struct revocable *rev;
- struct revocable_provider *rp = inode->i_private;
+ struct revocable_provider __rcu *rp = inode->i_private;

rev = revocable_alloc(rp);
if (!rev)
@@ -106,8 +106,8 @@ static int revocable_test_provider_release(struct inode *inode,
struct revocable_test_provider_priv *priv = filp->private_data;

debugfs_remove(priv->dentry);
- if (priv->rp)
- revocable_provider_revoke(priv->rp);
+ if (unrcu_pointer(priv->rp))
+ revocable_provider_revoke(&priv->rp);
kfree(priv);

return 0;
@@ -137,8 +137,8 @@ static ssize_t revocable_test_provider_write(struct file *filp,
* gone.
*/
if (!strcmp(data, TEST_CMD_RESOURCE_GONE)) {
- revocable_provider_revoke(priv->rp);
- priv->rp = NULL;
+ revocable_provider_revoke(&priv->rp);
+ rcu_assign_pointer(priv->rp, NULL);
} else {
if (priv->res[0] != '\0')
return 0;
@@ -146,14 +146,15 @@ static ssize_t revocable_test_provider_write(struct file *filp,
strscpy(priv->res, data);

priv->rp = revocable_provider_alloc(&priv->res);
- if (!priv->rp)
+ if (!unrcu_pointer(priv->rp))
return -ENOMEM;

priv->dentry = debugfs_create_file("consumer", 0400,
- debugfs_dir, priv->rp,
+ debugfs_dir,
+ unrcu_pointer(priv->rp),
&revocable_test_consumer_fops);
if (!priv->dentry) {
- revocable_provider_revoke(priv->rp);
+ revocable_provider_revoke(&priv->rp);
return -ENOMEM;