linux-kernel - Re: [PATCH 0/3] Revert "revocable: Revocable resource management"

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: <DG1ET3ZMX3LK.QKKLPFK1424M@kernel.org>
Date: Thu, 29 Jan 2026 23:00:21 +0100
From: "Danilo Krummrich" <dakr@...nel.org>
To: "Laurent Pinchart" <laurent.pinchart@...asonboard.com>
Cc: "Bartosz Golaszewski" <brgl@...nel.org>, "Johan Hovold"
 <johan@...nel.org>, "Bartosz Golaszewski"
 <bartosz.golaszewski@....qualcomm.com>, "Greg Kroah-Hartman"
 <gregkh@...uxfoundation.org>, "Rafael J . Wysocki" <rafael@...nel.org>,
 "Tzung-Bi Shih" <tzungbi@...nel.org>, "Linus Walleij" <linusw@...nel.org>,
 "Jonathan Corbet" <corbet@....net>, "Shuah Khan" <shuah@...nel.org>,
 "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 Thu Jan 29, 2026 at 3:49 PM CET, Laurent Pinchart wrote:
> On Thu, Jan 29, 2026 at 08:50:30AM -0500, Bartosz Golaszewski wrote:
>> On Thu, 29 Jan 2026 11:56:34 +0100, Laurent Pinchart said:
>> > On Thu, Jan 29, 2026 at 10:11:46AM +0100, Bartosz Golaszewski wrote:
>> >>
>> >> For I2C both the problem is different (subsystem waiting forever for
>> >> consumers to release all references) and the culprit: memory used to
>> >> hold the reference-counted struct device is released the supplier
>> >> unbind unconditionally. Unfortunately there's no way around it other
>> >> than to first move it into a separate chunk managed by i2c core.
>> >
>> > Isn't there ? Can't the driver-specific data structure be
>> > reference-counted instead of unconditionally freed at unbind time ?
>> 
>> Oh, for sure, if we did from the start. But we did not and there are now
>> hundreds of i2c drivers that do:
>> 
>> struct my_i2c_drv_data {
>> 	struct i2c_adapter adap;
>> 	int my_other_drv_data;
>> };
>> 
>> and in probe:
>> 
>> struct my_i2c_drv_data *data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
>> 
>> (or just kzalloc() with kfree() in remove, it doesn't matter)
>> 
>> and the ownership of that data belongs to the driver. There's no way we could
>> address it now so the next best thing is to work towards moving the ownership
>> of struct i2c_adapter to the i2c core and make it reference counted using the
>> internal kobject of the associated struct device.
>
> What I'm reading here is essentially that we rolled out devm_kzalloc() too
> quickly without understanding the consequences, and it has spread so much that
> it can't be fixed properly now, so we need to find a workaround.

I'm pretty sure I don't have all the details about the problem with the
i2c_adapter, but from what I read here briefly the problem simply seems to be
that currently the driver providing the i2c_adapter frees the i2c_adapter on
driver unbind unconditionally.

I would argue that this is a violation of the driver core design anyways. I
mean, in the end an i2c_adapter is the same type of device as any other bus
device (platform, PCI, etc.).

For instance, when the physical device that is represented by a PCI device is
removed from the machine, the corresponding struct pci_dev is not forcefully
freed either, it stays around as long as there are references to the device.

So, I fail to see how devm_kmalloc() and frieds are the root cause of the
i2c_adapter lifetime problem?

> And now we're trying to work around the problem by rolling out a revocable API
> that has barely seen any testing, and is known to have design issues. Does any
> one else see the irony ? :-)

I don't think anyone is trying to work around problems with devm_kmalloc() and
friends. It's just system memory, i.e. nothing that needs to be revoked. We can
just not use devm_kmalloc() and friends if we need the memory to outlive driver
unbind for some reason. The problem is about real device resources, such as I/O
memory mappings that *must* be released when a driver is unbound from its
device. So, revocable is clearly not a fix for devm_kmalloc() et al.