linux-kernel - Re: [PATCH 0/8] Device: generic accessors for drvdata + Driver::unbind()

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2025070142-difficult-lucid-d949@gregkh>
Date: Tue, 1 Jul 2025 11:25:41 +0200
From: Greg KH <gregkh@...uxfoundation.org>
To: Danilo Krummrich <dakr@...nel.org>
Cc: rafael@...nel.org, ojeda@...nel.org, alex.gaynor@...il.com,
	boqun.feng@...il.com, gary@...yguo.net, bjorn3_gh@...tonmail.com,
	benno.lossin@...ton.me, a.hindborg@...nel.org, aliceryhl@...gle.com,
	tmgross@...ch.edu, david.m.ertman@...el.com, ira.weiny@...el.com,
	leon@...nel.org, kwilczynski@...nel.org, bhelgaas@...gle.com,
	rust-for-linux@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-pci@...r.kernel.org
Subject: Re: [PATCH 0/8] Device: generic accessors for drvdata +
 Driver::unbind()

On Sat, Jun 21, 2025 at 09:43:26PM +0200, Danilo Krummrich wrote:
> This patch series consists of the following three parts.
> 
>   1. Introduce the 'Internal' device context (semantically identical to the
>      'Core' device context), but only accessible for bus abstractions.
> 
>   2. Introduce generic accessors for a device's driver_data  pointer. Those are
>      implemented for the 'Internal' device context only, in order to only enable
>      bus abstractions to mess with the driver_data pointer of struct device.
> 
>   3. Implement the Driver::unbind() callback (details below).
> 
> Driver::unbind()
> ----------------
> 
> Currently, there's really only one core callback for drivers, which is
> probe().
> 
> Now, this isn't entirely true, since there is also the drop() callback of
> the driver type (serving as the driver's private data), which is returned
> by probe() and is dropped in remove().
> 
> On the C side remove() mainly serves two purposes:
> 
>   (1) Tear down the device that is operated by the driver, e.g. call bus
>       specific functions, write I/O memory to reset the device, etc.
> 
>   (2) Release the resources that have been allocated by a driver for a
>       specific device.
> 
> The drop() callback mentioned above is intended to cover (2) as the Rust
> idiomatic way.
> 
> However, it is partially insufficient and inefficient to cover (1)
> properly, since drop() can't be called with additional arguments, such as
> the reference to the corresponding device that has the correct device
> context, i.e. the Core device context.

I'm missing something, why doesn't drop() have access to the device
itself, which has the Core device context?  It's the same "object",
right?

> This makes it inefficient (but not impossible) to access device
> resources, e.g. to write device registers, and impossible to call device
> methods, which are only accessible under the Core device context.
> 
> In order to solve this, add an additional callback for (1), which we
> call unbind().
> 
> The reason for calling it unbind() is that, unlike remove(), it is *only*
> meant to be used to perform teardown operations on the device (1), but
> *not* to release resources (2).

Ick.  I get the idea, but unbind() is going to get confusing fast.
Determining what is, and is not, a "resource" is going to be hard over
time.  In fact, how would you define it?  :)

Is "teardown" only allowed to write to resources, but not free them?  If
so, why can't that happen in drop() as the resources are available there.

I'm loath to have a 2-step destroy system here for rust only, and not
for C, as maintaining this over time is going to be rough.

thanks,

greg k-h