linux-kernel - [REGRESSION] Re: [patch V3 09/33] genirq/msi: Add range checking to msi_insert

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: <Y/Opu6ETe3ZzZ/8E@shell.armlinux.org.uk>
Date:   Mon, 20 Feb 2023 17:11:23 +0000
From:   "Russell King (Oracle)" <linux@...linux.org.uk>
To:     Thomas Gleixner <tglx@...utronix.de>
Cc:     LKML <linux-kernel@...r.kernel.org>, x86@...nel.org,
        Joerg Roedel <joro@...tes.org>, Will Deacon <will@...nel.org>,
        linux-pci@...r.kernel.org, Bjorn Helgaas <bhelgaas@...gle.com>,
        Lorenzo Pieralisi <lorenzo.pieralisi@....com>,
        Marc Zyngier <maz@...nel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Jason Gunthorpe <jgg@...lanox.com>,
        Dave Jiang <dave.jiang@...el.com>,
        Alex Williamson <alex.williamson@...hat.com>,
        Kevin Tian <kevin.tian@...el.com>,
        Dan Williams <dan.j.williams@...el.com>,
        Logan Gunthorpe <logang@...tatee.com>,
        Ashok Raj <ashok.raj@...el.com>, Jon Mason <jdmason@...zu.us>,
        Allen Hubbe <allenbh@...il.com>
Subject: [REGRESSION] Re: [patch V3 09/33] genirq/msi: Add range checking to
 msi_insert_desc()

On Fri, Nov 25, 2022 at 12:25:59AM +0100, Thomas Gleixner wrote:
> Per device domains provide the real domain size to the core code. This
> allows range checking on insertion of MSI descriptors and also paves the
> way for dynamic index allocations which are required e.g. for IMS. This
> avoids external mechanisms like bitmaps on the device side and just
> utilizes the core internal MSI descriptor storxe for it.
> 
> Signed-off-by: Thomas Gleixner <tglx@...utronix.de>

Hi Thomas,

This patch appears to cause a regression on Macchiatobin, delaying the
boot by about ten seconds due to all the warnings the kernel now
produces.

> @@ -136,11 +149,16 @@ static bool msi_desc_match(struct msi_de
>  
>  static bool msi_ctrl_valid(struct device *dev, struct msi_ctrl *ctrl)
>  {
> +	unsigned int hwsize;
> +
>  	if (WARN_ON_ONCE(ctrl->domid >= MSI_MAX_DEVICE_IRQDOMAINS ||
> -			 !dev->msi.data->__domains[ctrl->domid].domain ||
> -			 ctrl->first > ctrl->last ||
> -			 ctrl->first > MSI_MAX_INDEX ||
> -			 ctrl->last > MSI_MAX_INDEX))
> +			 !dev->msi.data->__domains[ctrl->domid].domain))
> +		return false;
> +
> +	hwsize = msi_domain_get_hwsize(dev, ctrl->domid);

This calls msi_get_device_domain() without taking dev->msi.data->mutex,
resulting in the lockdep_assert_held() firing for what seems to be every
MSI created by the Armada 8040 ICU driver, which suggests something isn't
taking the lock as you expect. Please can you take a look and propose a
patch to fix this regression.

Thanks.

[    0.960451] WARNING: CPU: 2 PID: 1 at kernel/irq/msi.c:588 msi_get_device_domain+0x70/0xa0
[    0.967454] Modules linked in:
[    0.969216] CPU: 2 PID: 1 Comm: swapper/0 Not tainted 6.2.0+ #1134
[    0.974116] Hardware name: Marvell 8040 MACCHIATOBin Single-shot (DT)
[    0.979276] pstate: 60400005 (nZCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[    0.984961] pc : msi_get_device_domain+0x70/0xa0
[    0.988292] lr : msi_get_device_domain+0x6c/0xa0
[    0.991623] sp : ffffffc080083460
[    0.993643] x29: ffffffc080083460 x28: 0000000000000000 x27: ffffffc041dcb6c8
[    0.999506] x26: ffffff8101f23810 x25: ffffffc080083668 x24: ffffff8101f23080
[    1.005370] x23: 0000000000000012 x22: ffffff81003d1000 x21: ffffff81025dfd90
[    1.011234] x20: ffffff8101f23810 x19: 0000000000000000 x18: 00000000fffffffd
[    1.017097] x17: 00000000cc510454 x16: 0000000000000051 x15: 0000000000000002
[    1.022960] x14: 00000000000389cb x13: 0000000000000001 x12: 0000000000000040
[    1.028822] x11: ffffff8100400490 x10: ffffff8100400492 x9 : 0000000000000000
[    1.034685] x8 : 0000000000000000 x7 : ffffff81001c8858 x6 : ffffffc0402ad718
[    1.040547] x5 : 00000000ffffffff x4 : ffffff81003d4c80 x3 : 0000000000000000
[    1.046410] x2 : ffffffc0fed09000 x1 : 0000000000000000 x0 : 0000000000000000
[    1.052274] Call trace:
[    1.053422]  msi_get_device_domain+0x70/0xa0
[    1.056404]  msi_ctrl_valid+0x5c/0x94
[    1.058775]  msi_domain_populate_irqs+0x64/0x1b0
[    1.062106]  platform_msi_device_domain_alloc+0x20/0x30
[    1.066048]  mvebu_icu_irq_domain_alloc+0xa0/0x1a0
[    1.069555]  __irq_domain_alloc_irqs+0xf8/0x46c
[    1.072799]  irq_create_fwspec_mapping+0x224/0x320
[    1.076303]  irq_create_of_mapping+0x68/0x90
[    1.079284]  of_irq_get+0x88/0xd0
[    1.081308]  platform_get_irq_optional+0x20/0x114
[    1.084725]  platform_get_irq+0x18/0x50
[    1.087269]  dw8250_probe+0x60/0x6e0
[    1.089552]  platform_probe+0x64/0xd0
[    1.091923]  really_probe+0xb8/0x2d4
[    1.094207]  __driver_probe_device+0x74/0xdc
[    1.097190]  driver_probe_device+0xd0/0x160
[    1.100085]  __driver_attach+0x94/0x1a0
[    1.102631]  bus_for_each_dev+0x6c/0xc0
[    1.105176]  driver_attach+0x20/0x30
[    1.107460]  bus_add_driver+0x148/0x200
[    1.110006]  driver_register+0x74/0x120
[    1.112550]  __platform_driver_register+0x24/0x30
[    1.115966]  dw8250_platform_driver_init+0x18/0x20
[    1.119473]  do_one_initcall+0x70/0x370
[    1.122018]  kernel_init_freeable+0x1d0/0x238
[    1.125087]  kernel_init+0x20/0x120
[    1.127283]  ret_from_fork+0x10/0x20
[    1.129567] ---[ end trace 0000000000000000 ]---

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!