linux-kernel - Re: [PATCH v3 15/16] rust: platform: add basic platform device / driver abstractions

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: <Z0XXnazDo8SmDNXb@cassiopeiae>
Date: Tue, 26 Nov 2024 15:13:49 +0100
From: Danilo Krummrich <dakr@...nel.org>
To: Alice Ryhl <aliceryhl@...gle.com>
Cc: gregkh@...uxfoundation.org, rafael@...nel.org, bhelgaas@...gle.com,
	ojeda@...nel.org, alex.gaynor@...il.com, boqun.feng@...il.com,
	gary@...yguo.net, bjorn3_gh@...tonmail.com, benno.lossin@...ton.me,
	tmgross@...ch.edu, a.hindborg@...sung.com, airlied@...il.com,
	fujita.tomonori@...il.com, lina@...hilina.net, pstanner@...hat.com,
	ajanulgu@...hat.com, lyude@...hat.com, robh@...nel.org,
	daniel.almeida@...labora.com, saravanak@...gle.com,
	rust-for-linux@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-pci@...r.kernel.org, devicetree@...r.kernel.org
Subject: Re: [PATCH v3 15/16] rust: platform: add basic platform device /
 driver abstractions

On Thu, Oct 31, 2024 at 09:23:58AM +0100, Alice Ryhl wrote:
> On Wed, Oct 30, 2024 at 7:07 PM Danilo Krummrich <dakr@...nel.org> wrote:
> >
> > On Wed, Oct 30, 2024 at 04:50:43PM +0100, Alice Ryhl wrote:
> > > On Tue, Oct 22, 2024 at 11:33 PM Danilo Krummrich <dakr@...nel.org> wrote:
> > > > +/// Drivers must implement this trait in order to get a platform driver registered. Please refer to
> > > > +/// the `Adapter` documentation for an example.
> > > > +pub trait Driver {
> > > > +    /// The type holding information about each device id supported by the driver.
> > > > +    ///
> > > > +    /// TODO: Use associated_type_defaults once stabilized:
> > > > +    ///
> > > > +    /// type IdInfo: 'static = ();
> > > > +    type IdInfo: 'static;
> > > > +
> > > > +    /// The table of device ids supported by the driver.
> > > > +    const ID_TABLE: IdTable<Self::IdInfo>;
> > > > +
> > > > +    /// Platform driver probe.
> > > > +    ///
> > > > +    /// Called when a new platform device is added or discovered.
> > > > +    /// Implementers should attempt to initialize the device here.
> > > > +    fn probe(dev: &mut Device, id_info: Option<&Self::IdInfo>) -> Result<Pin<KBox<Self>>>;
> > >
> > > This forces the user to put their driver data in a KBox, but they
> > > might want to use an Arc instead. You don't actually *need* a KBox -
> > > any ForeignOwnable seems to fit your purposes.
> >
> > This is intentional, I do need a `KBox` here.
> >
> > The reason is that I want to enforce that the returned `Pin<KBox<Self>>` has
> > exactly the lifetime of the binding of the device and driver, i.e. from probe()
> > until remove(). This is the lifetime the structure should actually represent.
> >
> > This way we can attach things like `Registration` objects to this structure, or
> > anything else that should only exist from probe() until remove().
> >
> > If a driver needs some private driver data that needs to be reference counted,
> > it is usually attached to the class representation of the driver.
> >
> > For instance, in Nova the reference counted stuff is attached to the DRM device
> > and then I just have the DRM device (which itself is reference counted) embedded
> > in the `Driver` structure.
> >
> > In any case, drivers can always embed a separate `Arc` in their `Driver`
> > structure if they really have a need for that.
> 
> Is this needed for soundness of those registrations?

Yes, we must ensure that the class is unregistered before the driver is removed.

However, I just noticed that by letting class abstractions return the
`Registration` without being wrapped as `Devres<Registration>` the driver can
theoretically still stuff them in whatever other structure and keep the
registration alive.

So, maybe there is no way around `Devres` even for class registrations.

> 
> Also, I've often seen drivers use devm_kzalloc or similar to allocate
> exactly this object. KBox doesn't allow for that.
> 
> 
> Alice
>