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

Message-ID: <2025091237-cortex-carnage-5c34@gregkh>
Date: Fri, 12 Sep 2025 16:40:27 +0200
From: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To: Laurent Pinchart <laurent.pinchart@...asonboard.com>
Cc: Bartosz Golaszewski <brgl@...ev.pl>, 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>,
	Danilo Krummrich <dakr@...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

On Fri, Sep 12, 2025 at 05:26:46PM +0300, Laurent Pinchart wrote:
> (CC'ing Dan Williams)
> 
> On Fri, Sep 12, 2025 at 04:19:53PM +0200, Greg KH wrote:
> > On Fri, Sep 12, 2025 at 04:59:16PM +0300, Laurent Pinchart wrote:
> > > On Fri, Sep 12, 2025 at 03:46:27PM +0200, Bartosz Golaszewski wrote:
> > > > On Fri, Sep 12, 2025 at 3:39 PM Greg Kroah-Hartman wrote:
> > > > >
> > > > > I have no objection moving this to the cdev api, BUT given that 'struct
> > > > > cdev' is embedded everywhere, I don't think it's going to be a simple
> > > > > task, but rather have to be done one-driver-at-a-time like the patch in
> > > > > this series does it.
> > > > 
> > > > I don't think cdev is the right place for this as user-space keeping a
> > > > reference to a file-descriptor whose "backend" disappeared is not the
> > > > only possible problem. We can easily create a use-case of a USB I2C
> > > > expander being used by some in-kernel consumer and then unplugged.
> > > > This has nothing to do with the character device. I believe the
> > > > sub-system level is the right place for this and every driver
> > > > subsystem would have to integrate it separately, taking its various
> > > > quirks into account.
> > > 
> > > That's why I mentioned in-kernel users previously. Drivers routinely
> > > acquire resources provided by other drivers, and having a way to revoke
> > > those is needed.
> > > 
> > > It is a different but related problem compared to userspace racing with
> > > .remove(). Could we solve both using the same backend concepts ?
> > > Perhaps, time will tell, and if that works nicely, great. But we still
> > > have lots of drivers exposing character devices to userspace (usually
> > > through a subsystem-specific API, drivers that create a cdev manually
> > > are the minority). That problem is in my opinion more urgent than
> > > handling the removal of in-kernel resources, because it's more common,
> > > and is easily triggerable by userspace. The good news is that it should
> > > also be simpler to solve, we should be able to address the enter/exit
> > > part entirely in cdev, and limit the changes to drivers in .remove() to
> > > the strict minimum.
> > > 
> > > What I'd like to see is if the proposed implementation of revocable
> > > resources can be used as a building block to fix the cdev issue. If it
> > > ca, great, let's solve it then. If it can't, that's still fine, it will
> > > still be useful for in-kernel resources, even if we need a different
> > > implementation for cdev.
> > 
> > Patch 5/5 in this series does just this for a specific use of a cdev in
> > the driver.  Is that what you are looking for?
> 
> Not quite, I would like to see the enter/exit (aka revocable scope if my
> understanding is correct) being pushed to char_dev.c, as Dan did in [1].
> I'm fine having to add an extra function call in the .remove() path of
> drivers, but I'm not fine with having to mark revocable sections
> manually in drivers. That part belongs to cdev.
> 
> [1] https://lore.kernel.org/r/161117153248.2853729.2452425259045172318.stgit@dwillia2-desk3.amr.corp.intel.com

Dan's proposal here is a good start, but the "sleep in cdev_del() until
the device drains all existing opens" is going to not really work well
for what we want.

So sure, make a new cdev api to use this, that's fine, then we will have
what, 5 different ways to use a cdev?  :)

Seriously, that would be good, then we can work to convert things over,
but I think overall it will look much the same as what patch 5/5 does
here.  But details matter, I don't really known for sure...

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.

thanks,

greg k-h