linux-kernel - Re: [PATCH RESEND V2 3/6] platform/x86/intel: Move intel

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: <YbEFuN7fwdiNI8vW@kroah.com>
Date:   Wed, 8 Dec 2021 20:21:28 +0100
From:   Greg KH <gregkh@...uxfoundation.org>
To:     "David E. Box" <david.e.box@...ux.intel.com>
Cc:     lee.jones@...aro.org, hdegoede@...hat.com, bhelgaas@...gle.com,
        andriy.shevchenko@...ux.intel.com, srinivas.pandruvada@...el.com,
        mgross@...ux.intel.com, linux-kernel@...r.kernel.org,
        platform-driver-x86@...r.kernel.org, linux-pci@...r.kernel.org,
        Mark Gross <markgross@...nel.org>
Subject: Re: [PATCH RESEND V2 3/6] platform/x86/intel: Move intel_pmt from
 MFD to Auxiliary Bus

On Wed, Dec 08, 2021 at 11:09:48AM -0800, David E. Box wrote:
> On Wed, 2021-12-08 at 19:11 +0100, Greg KH wrote:
> > On Wed, Dec 08, 2021 at 09:47:26AM -0800, David E. Box wrote:
> > > On Wed, 2021-12-08 at 17:22 +0100, Greg KH wrote:
> > > > On Tue, Dec 07, 2021 at 05:50:12PM -0800, David E. Box wrote:
> > > > > +static struct pci_driver intel_vsec_pci_driver = {
> > > > > +       .name = "intel_vsec",
> > > > > +       .id_table = intel_vsec_pci_ids,
> > > > > +       .probe = intel_vsec_pci_probe,
> > > > > +};
> > > > 
> > > > So when the PCI device is removed from the system you leak resources and
> > > > have dangling devices?
> > > 
> > > No.
> > > 
> > > > 
> > > > Why no PCI remove driver callback?
> > > 
> > > After probe all resources are device managed. There's nothing to explicitly clean up. When the
> > > PCI
> > > device is removed, all aux devices are automatically removed. This is the case for the SDSi
> > > driver
> > > as well.
> > 
> > Where is the "automatic cleanup" happening?  As this pci driver is bound
> > to the PCI device, when the device is removed, what is called in this
> > driver to remove the resources allocated in the probe callback?
> > 
> > confused,
> 
> devm_add_action_or_reset(&pdev->dev, intel_vsec_remove_aux, auxdev)

Wow that is opaque.  Why not do it on remove instead?

> intel_vsec_remove_aux() gets called when the PCI device is removed. It calls auxiliary_device_unit()
> which in turn calls the auxdev release() function that cleans up resources.

Does this happen when the device is removed, or when the binding of
driver <-> device is removed?

> When the auxdev is removed, all resources that were dev_m added by the SDSi driver are released too
> which is why it has no remove() either. I'll add the tests that check this.

Please do so and document it well, as that is an odd "pattern".

thanks,

greg k-h