linux-kernel - Re: [PATCH v3 0/5] platform/chrome: Fix a possible UAF via revocable

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: <68d1d0fc5ef09_1c79100e@dwillia2-mobl4.notmuch>
Date: Mon, 22 Sep 2025 15:43:08 -0700
From: <dan.j.williams@...el.com>
To: Laurent Pinchart <laurent.pinchart@...asonboard.com>, Danilo Krummrich
	<dakr@...nel.org>
CC: Bartosz Golaszewski <brgl@...ev.pl>, Greg Kroah-Hartman
	<gregkh@...uxfoundation.org>, Tzung-Bi Shih <tzungbi@...nel.org>, "Bartosz
 Golaszewski" <bartosz.golaszewski@...aro.org>, Krzysztof Kozlowski
	<krzk@...nel.org>, Benson Leung <bleung@...omium.org>, "Rafael J . Wysocki"
	<rafael@...nel.org>, Jonathan Corbet <corbet@....net>, Shuah Khan
	<shuah@...nel.org>, Dawid Niedzwiecki <dawidn@...gle.com>,
	<linux-doc@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
	<chrome-platform@...ts.linux.dev>, <linux-kselftest@...r.kernel.org>,
	"Wolfram Sang" <wsa+renesas@...g-engineering.com>, Dan Williams
	<dan.j.williams@...el.com>
Subject: Re: [PATCH v3 0/5] platform/chrome: Fix a possible UAF via revocable

Laurent Pinchart wrote:
> On Fri, Sep 12, 2025 at 06:22:48PM +0200, Danilo Krummrich wrote:
> > On 9/12/25 4:54 PM, Laurent Pinchart wrote:
> > > On Fri, Sep 12, 2025 at 04:44:56PM +0200, Bartosz Golaszewski wrote:
> > >> On Fri, Sep 12, 2025 at 4:40 PM Greg Kroah-Hartman wrote:
> > >>> Either way, I think this patch series stands on its own, it doesn't
> > >>> require cdev to implement it, drivers can use it to wrap a cdev if they
> > >>> want to.  We have other structures that want to do this type of thing
> > >>> today as is proof with the rust implementation for the devm api.
> > >>
> > >> Yeah, I'm not against this going upstream. If more development is
> > >> needed for this to be usable in other parts of the kernel, that can be
> > >> done gradually. Literally no subsystem ever was perfect on day 1.
> > > 
> > > To be clear, I'm not against the API being merged for the use cases that
> > > would benefit from it, but I don't want to see drivers using it to
> > > protect from the cdev/unregistration race.
> > 
> > I mean, revocable is really a synchronization primitive in the end that
> > "revokes" access to some resource in a race free way.
> > 
> > So, technically, it probably belongs into lib/.
> > 
> > I think the reason it ended up in drivers/base/ is that one common use case is
> > to revoke a device resource from a driver when the device is unbound from this
> > driver; or in other words devres is an obvious user.
> > 
> > So, I think that any other API (cdev, devres, etc.) should  be built on top of it.
> 
> No issue with that. I'm sure there are people who have better knowledge
> than me when it comes to implementing the low-level primitive in the
> most efficient way. What I have lots of experience with is the impact of
> API design on drivers, and the API misuse (including through cargo-cult
> programming) this can generate. Let's design the API towards drivers
> correctly.

Note that I dropped the "managed_fops" [1] effort, targeted for use for
CXL, in favor of simply this in the CXL ioctl device shutdown path:

        cdev_device_del(&cxlmd->cdev, &cxlmd->dev);
        scoped_guard(rwsem_write, &cxl_memdev_rwsem)
		cxlmd->cxlds = NULL;
        put_device(&cxlmd->dev);

Pair that with:

        guard(rwsem_read)(&cxl_memdev_rwsem);
        cxlds = cxlmd->cxlds;
        if (cxlds)
                return __cxl_memdev_ioctl(cxlmd, cmd, arg);
	return -ENXIO;

...on the ioctl invocation side.

This "revocable" mechanism looks useful for other inter-driver resource
sharing, but not for the well known issues with cdev. For cdev and the
design pattern of "shutdown the ioctl path on a core-subsystem device
object that is also a chardev", just use cdev_device_add() with an
rwsem.

[1]: http://lore.kernel.org/all/CAPcyv4h74NjqcuUjv4zFKHAxio_bV0bngLoxP=ACw=JvMfq-UA@mail.gmail.com