On Thu, Feb 05, 2026 at 03:03:21PM +0100, Johan Hovold wrote:

On Thu, Feb 05, 2026 at 08:51:19AM +0000, Tzung-Bi Shih wrote:

quoted

On Wed, Feb 04, 2026 at 03:28:49PM +0100, Johan Hovold wrote:

quoted

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:

[...]

quoted

	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.

But that is not what the selftest does currently, nor is it what you

Understood.  As mentioned in my previous email, I'll update the selftests to
reflect this.  The test case somehow is simplified to setup the scenario it
wants to test.

need for your initial motivation for this which was miscdev where you
don't have any reference counted driver data to store the pointer in.

You're right that cros_ec_chardev.c needs to be adjusted before using the
version of revocable.

quoted

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.

Right, but then you don't need all the RCU stuff. And revocable becomes
just a convoluted abstraction for a lock and flag (as was pointed out
early on).

I'm not sure if I understand you: as we're discussing about the stale pointer
of revocable provider consumers might cache here, do you mean it doesn't need
RCU at all to protect the race you reported in
https://lore.kernel.org/all/aXdy-b3GOJkzGqYo@hovoldconsulting.com/ (local)?

quoted

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.

And again, that's precisely why we need to evaluate the API in a
non-trivial context *before* merging yet another version of this.

quoted

quoted

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?

I wasn't thinking of any particular example.

The struct revocable_provider is already reference counted and you don't
need anything more than that as long as you only take another reference
in a context where you already hold a reference. (And struct

The reference count in struct revocable_provider can't protect the race
reported https://lore.kernel.org/all/aXdy-b3GOJkzGqYo@hovoldconsulting.com/ (local).

revocable_provider should be renamed struct revocable).

Probably no, struct revocable is for consumer handle.  It needs somewhere to
store the SRCU index.

SRCU is what prevents the race against revoke, no need for RCU.

No, without the RCU, it can't prevent the race between revoking and creating
a new consumer handle.

But this is designing yet another version of revocable. And that should
be done out-of-tree.

Johan