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

["Danilo Krummrich" <dakr@kernel.org>]

On Thu Feb 5, 2026 at 9:51 AM CET, Tzung-Bi Shih wrote:
> 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.

Yeah, I think this part is not a bug in the API, but I think revocable_init()
should be

	int revocable_init(struct revocable_provider __rcu **_rp, ...)

instead of

	int revocable_init(struct revocable_provider __rcu *_rp, ...)

for the same reason revocable_provider_revoke() takes a double pointer.

Otherwise this seems racy:

	int revocable_init(struct revocable_provider __rcu *_rp, struct revocable *rev)
	{
		struct revocable_provider *rp;

		if (!_rp)
			return -ENODEV;

		/*
		 * If revocable_provider_revoke() is called concurrently at this
		 * point, _rp is not affectd by rcu_replace_pointer().
		 *
		 * Additionally, nothing prevents a concurrent kfree_rcu() from
		 * freeing the revocable provider before we enter the RCU
		 * read-side critical section below.
		 */

		/*
		 * 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) {

		...
	}

Do I miss anything?