Re: [PATCH v9 1/9] revocable: Revocable resource management

[Bartosz Golaszewski]

On Mon, 27 Apr 2026 15:58:33 +0200, Tzung-Bi Shih <tzungbi@xxxxxxxxxx> said:
> 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.
>
> It 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 and initializes it with a pointer
>   to the resource.
>
> - A resource consumer that wants to access the resource allocates a
>   struct revocable_consumer 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.  There
>   are some macro level helpers for doing that.
>
>   The API provides the following contract:
>
>   - revocable_try_access() can be safely called from both process and
>     atomic contexts.
>   - It is permitted to sleep within the critical section established
>     between revocable_try_access() and revocable_withdraw_access().
>   - revocable_try_access() and the matching revocable_withdraw_access()
>     must occur in the same context.  For example, it is illegal to
>     invoke revocable_withdraw_access() in an irq handler if the matching
>     revocable_try_access() was invoked in process context.
>
> - When the provider needs to remove the resource, it calls
>   revocable_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.
>
> Signed-off-by: Tzung-Bi Shih <tzungbi@xxxxxxxxxx>
> ---

...

> diff --git a/include/linux/revocable.h b/include/linux/revocable.h
> new file mode 100644
> index 000000000000..2bcf23f01ace
> --- /dev/null
> +++ b/include/linux/revocable.h
> @@ -0,0 +1,214 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright 2026 Google LLC
> + */
> +
> +#ifndef __LINUX_REVOCABLE_H
> +#define __LINUX_REVOCABLE_H
> +
> +#include <linux/cleanup.h>
> +#include <linux/compiler.h>

I don't think you need this header.

> +#include <linux/kref.h>
> +#include <linux/srcu.h>
> +
> +/**
> + * enum revocable_alloc_type - The allocation method for a revocable provider.
> + * @REVOCABLE_DYNAMIC: The struct revocable was dynamically allocated using
> + *                     revocable_alloc() and its lifetime is managed by
> + *                     reference counting.
> + * @REVOCABLE_EMBEDDED: The struct revocable is embedded within another
> + *                      structure.  Its lifetime is tied to the parent
> + *                      structure and is not reference counted.
> + */
> +enum revocable_alloc_type {
> +	REVOCABLE_DYNAMIC,
> +	REVOCABLE_EMBEDDED,
> +};

Maybe we don't need this public enum at all, we could just use a different
release callback for kref_put() depending on how the revocable was allocated?

The enum is not used elsewhere so it doesn't make sense to document it as if it
was part of the revocable API.

> +
> +/**
> + * struct revocable - 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.
> + * @alloc_type: The memory allocation type.
> + */
> +struct revocable {
> +	struct srcu_struct srcu;
> +	void __rcu *res;
> +	struct kref kref;
> +	enum revocable_alloc_type alloc_type;

This could be replaced with the pointer to the release callback, assigned
by revocable_alloc()/revocable_init() respectively.

> +};
> +
> +/**
> + * struct revocable_consumer - A handle for resource consumer.
> + * @rev: The pointer of resource provider.
> + * @idx: The index for the SRCU critical section.

Should any of these be accessed directly by the user? Maybe document them
as __private?

> + */
> +struct revocable_consumer {
> +	struct revocable *rev;
> +	int idx;
> +};

I'd rename it to struct revocable_handle which indicates better what it is:
it's a handle *owned* by the consumer.

> +
> +void revocable_get(struct revocable *rev);
> +void revocable_put(struct revocable *rev);
> +
> +struct revocable *revocable_alloc(void *res);
> +void revocable_revoke(struct revocable *rev);
> +int revocable_embed_init(struct revocable *rev, void *res);
> +void revocable_embed_destroy(struct revocable *rev);
> +
> +void revocable_init(struct revocable *rev, struct revocable_consumer *rc);
> +void revocable_deinit(struct revocable_consumer *rc);

If we hid the release logic, we could drop revocable_embed_destroy() and use
the same refcounting functions for both variants. I'd suggest the following:

For refcounting (same for both variants):

	void revocable_get(struct revocable *rev);
	void revocable_put(struct revocable *rev);

For dynamic variant:

	struct revocable *revocable_alloc(void *res);

For embedded:

	int revocable_init(struct revocable *rev, void *res);

For handles:

	void revocable_handle_init(struct revocable *rev, struct
revocable_consumer *rc);
	void revocable_handle_deinit(struct revocable_consumer *rc);

Does it make sense?

Other (try_access_*, etc.) helpers look good. And the API in general looks
pretty good to me too.

Bart