[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250922151040.GA2546062@nvidia.com>
Date: Mon, 22 Sep 2025 12:10:40 -0300
From: Jason Gunthorpe <jgg@...dia.com>
To: Laurent Pinchart <laurent.pinchart@...asonboard.com>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
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:
> 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 I've open coded something here many times.
The implementation challenge is performance..
The big ask would be to make file_operations replaceable without
races. I can't see a way to do that at the struct file level without
slowing down everything.
Dan's idea to provide a wrapper file_operations that then effectively
calls another set of file_operations under a synchornization is more
reasonable, but the performance cost is now a percpu ref and another
indirect function call for every file operation. It also relies on the
inode which some cdev users end up replacing.
Open coding the lock in the subsystems avoids the indirect function
call, flexible inode, and subsystems can choose their locking
preference (rwsem, srcu, percpu).
As was said later in this thread, it would be a real shame to see
people implement revocable in drivers instead of rely on subsystems to
have sane unregistration semantics where the subsystem guarentees that
no driver callbacks are running after unregister. You never need
driver revocable in a world like that.
Jason
Powered by blists - more mailing lists