linux-kernel - Re: [PATCH v6 2/2] platform/chrome: cros_ec_chardev: Consume cros_ec

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] [day] [month] [year] [list]

Message-ID: <20251106155951.GC1732817@nvidia.com>
Date: Thu, 6 Nov 2025 11:59:51 -0400
From: Jason Gunthorpe <jgg@...dia.com>
To: Tzung-Bi Shih <tzungbi@...nel.org>
Cc: Benson Leung <bleung@...omium.org>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	"Rafael J . Wysocki" <rafael@...nel.org>,
	Danilo Krummrich <dakr@...nel.org>,
	Jonathan Corbet <corbet@....net>, Shuah Khan <shuah@...nel.org>,
	linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
	chrome-platform@...ts.linux.dev, linux-kselftest@...r.kernel.org,
	Laurent Pinchart <laurent.pinchart@...asonboard.com>,
	Bartosz Golaszewski <brgl@...ev.pl>,
	Wolfram Sang <wsa+renesas@...g-engineering.com>,
	Simona Vetter <simona.vetter@...ll.ch>,
	Dan Williams <dan.j.williams@...el.com>
Subject: Re: [PATCH v6 2/2] platform/chrome: cros_ec_chardev: Consume
 cros_ec_device via revocable

On Thu, Nov 06, 2025 at 11:26:02PM +0800, Tzung-Bi Shih wrote:
> @@ -166,7 +181,12 @@ static int cros_ec_chardev_open(struct inode *inode, struct file *filp)
>  	if (!priv)
>  		return -ENOMEM;
>  
> -	priv->ec_dev = ec_dev;
> +	priv->ec_dev_rev = revocable_alloc(ec_dev->revocable_provider);
> +	if (!priv->ec_dev_rev) {
> +		ret = -ENOMEM;
> +		goto free_priv;
> +	}

The lifecyle of ec_dev->ec_dev->revocable_provider memory is
controlled by dev:

+       ec_dev->revocable_provider = devm_revocable_provider_alloc(dev, ec_dev);

Under the lifecycle of some other driver.

The above only works because misc calls open under the misc_mtx so it
open has "sync" behavior during misc_unregister, and other rules
ensure that ec_dev is valid during the full lifecycle of this driver.

So, I think this cross-driver design an abusive use of the revocable
idea.

It should not be allocated by the parent driver, it should be fully
contained to this driver alone and used only to synchronize the
fops. This would make it clear that the ec_dev pointer must be valid
during the *entire* lifecycle of this driver.

What you have here by putting the providing in another driver is too
magic and obfuscates what the actual lifetime rules are while
providing a giant foot gun for someone to think that just because it
is marked revocable it is fully safe to touch revocable_provider at
any time.

Broadly I think embedding a revocable in the memory that it is trying
to protect is probably an anti-pattern as you must somehow already
have a valid pointer to thing to get the revocable in the first place.
This severely muddies the whole notion of when it can actually be
revoked nor not.

Jason