Message-ID: <20250912142646.GI31682@pendragon.ideasonboard.com>
Date: Fri, 12 Sep 2025 17:26:46 +0300
From: Laurent Pinchart <laurent.pinchart@...asonboard.com>
To: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
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

(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

-- 
Regards,

Laurent Pinchart

Powered by blists - more mailing lists