Re: [PATCH v2 3/3] Revert "revocable: Revocable resource management"

[Tzung-Bi Shih <tzungbi@kernel.org>]

On Wed, Feb 04, 2026 at 03:28:49PM +0100, Johan Hovold wrote:
> 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:
[...]
> 	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.

You're right to point out the issue with copying the pointer of revocable
provider.  If a consumer stores this pointer directly, rcu_replace_pointer()
in the producer's revocable_provider_revoke() will not affect the consumer's
copy.  I understand this concern.

The intention was never for consumers to cache the pointer of revocable
provider long-term.  The design relies on consumers obtaining the current
valid provider pointer at the point of access.

In the latest GPIO transition series [5], the usage pattern has been refined
to avoid locally storing the pointer of revocable provider.  Instead, it's
fetched from a source of truth when needed.

I agree that the risks and correct usage patterns need to be much clearer.
I'll update the Documentation and the selftests to explicitly highlight
this limitation and demonstrate the proper way to interact with the API,
avoiding the storage of the provider pointer by value in consumer contexts.

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

(I'm assuming you are referring to the example in [6].)

I'm not sure I follow your reasoning.  Per my understanding:
- The reference counted driver data (e.g. `gdev` in the GPIO example) is to
  ensure the pointer of revocable provider isn't freed.
- The RCU protects the pointer value from concurrent access and updates
  during the revocation process [7].

These seem to address different aspects.  Could you provide more context
on why you see the RCU protection as redundant?

[5] https://lore.kernel.org/all/20260203061059.975605-9-tzungbi@kernel.org
[6] https://lore.kernel.org/all/20260203061059.975605-8-tzungbi@kernel.org
[7] https://lore.kernel.org/all/20260129143733.45618-2-tzungbi@kernel.org