linux-kernel - Re: [patch V3 00/35] genirq/msi, PCI/MSI: Spring cleaning

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: <20211214162247.ocjm7ihg5oi7uiuv@slider>
Date:   Tue, 14 Dec 2021 10:22:47 -0600
From:   Nishanth Menon <nm@...com>
To:     Thomas Gleixner <tglx@...utronix.de>
CC:     LKML <linux-kernel@...r.kernel.org>,
        Bjorn Helgaas <helgaas@...nel.org>,
        Marc Zygnier <maz@...nel.org>,
        Alex Williamson <alex.williamson@...hat.com>,
        Kevin Tian <kevin.tian@...el.com>,
        Jason Gunthorpe <jgg@...dia.com>,
        Megha Dey <megha.dey@...el.com>,
        Ashok Raj <ashok.raj@...el.com>, <linux-pci@...r.kernel.org>,
        Cedric Le Goater <clg@...d.org>,
        Juergen Gross <jgross@...e.com>,
        <xen-devel@...ts.xenproject.org>, Arnd Bergmann <arnd@...db.de>,
        Michael Ellerman <mpe@...erman.id.au>,
        Benjamin Herrenschmidt <benh@...nel.crashing.org>,
        <linuxppc-dev@...ts.ozlabs.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Bjorn Helgaas <bhelgaas@...gle.com>,
        Stuart Yoder <stuyoder@...il.com>,
        Laurentiu Tudor <laurentiu.tudor@....com>,
        Tero Kristo <kristo@...nel.org>,
        Santosh Shilimkar <ssantosh@...nel.org>,
        <linux-arm-kernel@...ts.infradead.org>,
        Vinod Koul <vkoul@...nel.org>, <dmaengine@...r.kernel.org>,
        Mark Rutland <mark.rutland@....com>,
        Will Deacon <will@...nel.org>,
        Robin Murphy <robin.murphy@....com>,
        Joerg Roedel <joro@...tes.org>,
        <iommu@...ts.linux-foundation.org>,
        Jassi Brar <jassisinghbrar@...il.com>,
        Peter Ujfalusi <peter.ujfalusi@...il.com>,
        Sinan Kaya <okaya@...nel.org>,
        <linux-wireless@...r.kernel.org>,
        Johannes Berg <johannes.berg@...el.com>
Subject: Re: [patch V3 00/35] genirq/msi, PCI/MSI: Spring cleaning - Part 2

On 10:41-20211214, Thomas Gleixner wrote:
> On Mon, Dec 13 2021 at 12:29, Nishanth Menon wrote:
> > On 23:18-20211210, Thomas Gleixner wrote:
> > Also while testing on TI K3 platforms, I noticed:
> >
> > msi_device_data_release/msi_device_destroy_sysfs in am64xx-evm / j7200
> 
> The warning complains about a device being released with MSI descriptors
> still attached to the device. This was added by:
> 
>   5b012cede0f7 ("device: Add device::msi_data pointer and struct msi_device_data")
> 
> That's not a regression caused by this commit. The warning is just
> exposing an already existing problem in the iwlwifi driver, which seems
> to do:
> 
>    probe()
>      setup_pci_msi[x]_interrupts()
>      start_drv()
>        if (try_to_load_firmware() == FAIL)
>        	   device_release_driver()
>                 ...
>                 msi_device_data_release()
>                     WARN()
> 

Agreed that the warning is fine, the null pointer exception that follows
[1] [2] it however does'nt look right and it can be trivially fixed with the
following fixup for ee90787487bc ("genirq/msi: Provide
msi_device_populate/destroy_sysfs()") below, with that the log looks
like [3] - the warn is good, the null pointer exception and resultant
crash could be avoided (not saying this is the best solution):

diff --git a/kernel/irq/msi.c b/kernel/irq/msi.c
index ab5e83f41188..24edb870c66f 100644
--- a/kernel/irq/msi.c
+++ b/kernel/irq/msi.c
@@ -252,11 +252,14 @@ int msi_device_populate_sysfs(struct device *dev)
  */
 void msi_device_destroy_sysfs(struct device *dev)
 {
-	const struct attribute_group **msi_irq_groups = dev->msi.data->attrs;
+	const struct attribute_group **msi_irq_groups;
 	struct device_attribute *dev_attr;
 	struct attribute **msi_attrs;
 	int count = 0;
 
+	if (!dev->msi.data)
+		return;
+	msi_irq_groups = dev->msi.data->attrs;
 	dev->msi.data->attrs = NULL;
 	if (!msi_irq_groups)
 		return;

[1] https://gist.github.com/nmenon/36899c7819681026cfe1ef185fb95f33#file-am64xx-evm-txt-L1049
[2] https://gist.github.com/nmenon/36899c7819681026cfe1ef185fb95f33#file-j7200-evm-txt-L1111

[3] https://gist.github.com/nmenon/575afe7d04463026a7e420a76c2c1c5b
	https://gist.github.com/nmenon/575afe7d04463026a7e420a76c2c1c5b#file-am64xx-evm-txt-L1018
	https://gist.github.com/nmenon/575afe7d04463026a7e420a76c2c1c5b#file-j7200-evm-txt-L1053
-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D)/Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 849D 1736 249D