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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <DG70CSP8NCJ0.316VXMVLD2ARN@kernel.org>
Date: Thu, 05 Feb 2026 12:56:47 +0100
From: "Danilo Krummrich" <dakr@...nel.org>
To: "Tzung-Bi Shih" <tzungbi@...nel.org>
Cc: "Johan Hovold" <johan@...nel.org>, "Greg Kroah-Hartman"
 <gregkh@...uxfoundation.org>, "Rafael J . Wysocki" <rafael@...nel.org>,
 "Bartosz Golaszewski" <bartosz.golaszewski@....qualcomm.com>, "Linus
 Walleij" <linusw@...nel.org>, "Jonathan Corbet" <corbet@....net>, "Shuah
 Khan" <shuah@...nel.org>, "Laurent Pinchart"
 <laurent.pinchart@...asonboard.com>, "Wolfram Sang"
 <wsa+renesas@...g-engineering.com>, "Simona Vetter"
 <simona.vetter@...ll.ch>, "Dan Williams" <dan.j.williams@...el.com>, "Jason
 Gunthorpe" <jgg@...dia.com>, <linux-doc@...r.kernel.org>,
 <linux-kselftest@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 3/3] Revert "revocable: Revocable resource
 management"

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?