[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aXdxDBXdyqLFfKKI@hovoldconsulting.com>
Date: Mon, 26 Jan 2026 14:50:04 +0100
From: Johan Hovold <johan@...nel.org>
To: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc: Danilo Krummrich <dakr@...nel.org>,
"Rafael J . Wysocki" <rafael@...nel.org>,
Tzung-Bi Shih <tzungbi@...nel.org>,
Bartosz Golaszewski <bartosz.golaszewski@....qualcomm.com>,
Linus Walleij <linusw@...nel.org>, Jonathan Corbet <corbet@....net>,
Shuah Khan <shuah@...nel.org>,
Laurent Pinchart <laurent.pinchart@...asonboard.com>,
Wolfram Sang <wsa+renesas@...g-engineering.com>,
Simona Vetter <simona.vetter@...ll.ch>,
Dan Williams <dan.j.williams@...el.com>,
Jason Gunthorpe <jgg@...dia.com>, linux-doc@...r.kernel.org,
linux-kselftest@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 0/3] Revert "revocable: Revocable resource management"
On Sun, Jan 25, 2026 at 01:47:14PM +0100, Greg Kroah-Hartman wrote:
> On Sat, Jan 24, 2026 at 08:08:28PM +0100, Danilo Krummrich wrote:
> > On Sat Jan 24, 2026 at 6:05 PM CET, Johan Hovold wrote:
> > > this does not look like the right interface for the chardev unplug issue.
> >
> > I think it depends, we should do everything to prevent having the issue in the
> > first place, e.g. ensure that we synchronize the unplug properly on device
> > driver unbind.
> >
> > Sometimes, however, this isn't possible; this is where a revocable mechanism can
> > come in handy to prevent UAF of device resources -- DRM is a good example for
> > this.
>
> This is not "possible" for almost all real devices so we need something
> like this for almost all classes of devices, DRM just shows the extremes
> involved, v4l2 is also another good example.
It's certainly possible to handle the chardev unplug issue without
revocable as several subsystems already do. All you need is a refcount,
a lock and a flag.
It may be possible to provide a generic solutions at the chardev level
or some kind of helper implementation (similar to revocable) for
subsystems to use directly.
But revocable appears to be too fine grained for this as when the
device goes away all operations must cease. There's no need to track
mmio regions individually as was suggested. This may be the mental model
for someone working with rust, but it isn't necessarily a good fit for
the rest of the kernel.
> > But to be fair, I also want to point out that there is a quite significant
> > difference regarding the usefulness of the revocable concept in C compared to in
> > Rust due to language capabilities.
>
> True, but we do need something. I took these patches without a real
> user as a base for us to start working off of. The rust implementation
> has shown that the design-pattern is a good solution for the problem,
> and so I feel we should work with it and try to get this working
> properly. We've been sitting and talking about it for years now, and
> here is the first real code submission that is getting us closer to fix
> the problem properly. It might not be perfict, but let's evolve it from
> here for what is found not to work correctly.
It's a design pattern that's perhaps needed for rust, but not
necessarily elsewhere. But either way there is no need to rush this. If
it turns out to be usable, it can be merged along with a future user.
Dropping the revocable_provider and revocable abstraction split should
even make it more palatable.
And with a new interface and a non-trivial user we can see what the
end-result looks like and decide where to go from there.
> So I don't want to take these reverts, let's try this out, by putting
> this into the driver core now, we have the base to experiment with in a
> "safe" way in lots of different driver subsytems at the same time. If
> it doesn't work out, worst case we revert it in a release or two because
> it didn't get used.
Please reconsider. Perhaps I didn't stress the point enough that the
current API needs to be reworked completely since there's no longer any
need for the two revocable abstractions.
Johan
Powered by blists - more mailing lists