linux-kernel - Re: [PATCH] x86/ioapic: Don't return 0 as valid virq

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: <PUZP153MB0749DD5C9CE7B60DDC75ADACBEB99@PUZP153MB0749.APCP153.PROD.OUTLOOK.COM>
Date:   Mon, 13 Mar 2023 03:37:44 +0000
From:   Saurabh Singh Sengar <ssengar@...rosoft.com>
To:     Saurabh Singh Sengar <ssengar@...rosoft.com>,
        Borislav Petkov <bp@...en8.de>,
        Saurabh Sengar <ssengar@...ux.microsoft.com>
CC:     "tglx@...utronix.de" <tglx@...utronix.de>,
        "mingo@...hat.com" <mingo@...hat.com>,
        "dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
        "x86@...nel.org" <x86@...nel.org>, "hpa@...or.com" <hpa@...or.com>,
        "johan+linaro@...nel.org" <johan+linaro@...nel.org>,
        "isaku.yamahata@...el.com" <isaku.yamahata@...el.com>,
        "Michael Kelley (LINUX)" <mikelley@...rosoft.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "andriy.shevchenko@...el.com" <andriy.shevchenko@...el.com>
Subject: Re: [PATCH] x86/ioapic: Don't return 0 as valid virq

I just see mail to rahul.tanwar@...ux.intel.com is undelivered, shall I still add it in "Cc:" ?
Please let me know what we usually do in such cases.

Regards,
Saurabh


> -----Original Message-----
> From: Saurabh Singh Sengar <ssengar@...rosoft.com>
> Sent: Monday, March 13, 2023 9:00 AM
> To: Borislav Petkov <bp@...en8.de>; Saurabh Sengar
> <ssengar@...ux.microsoft.com>
> Cc: tglx@...utronix.de; mingo@...hat.com; dave.hansen@...ux.intel.com;
> x86@...nel.org; hpa@...or.com; johan+linaro@...nel.org;
> isaku.yamahata@...el.com; Michael Kelley (LINUX)
> <mikelley@...rosoft.com>; linux-kernel@...r.kernel.org;
> rahul.tanwar@...ux.intel.com; andriy.shevchenko@...el.com
> Subject: RE: [EXTERNAL] Re: [PATCH] x86/ioapic: Don't return 0 as valid virq
> 
> Cc: rahul.tanwar@...ux.intel.com, andriy.shevchenko@...el.com
> 
> Thanks for you comments, please see my responses below.
> 
> > -----Original Message-----
> > From: Borislav Petkov <bp@...en8.de>
> > Sent: Monday, March 13, 2023 2:10 AM
> > To: Saurabh Sengar <ssengar@...ux.microsoft.com>
> > Cc: tglx@...utronix.de; mingo@...hat.com; dave.hansen@...ux.intel.com;
> > x86@...nel.org; hpa@...or.com; johan+linaro@...nel.org;
> > isaku.yamahata@...el.com; Michael Kelley (LINUX)
> > <mikelley@...rosoft.com>; linux-kernel@...r.kernel.org
> > Subject: [EXTERNAL] Re: [PATCH] x86/ioapic: Don't return 0 as valid
> > virq
> >
> > On Thu, Mar 02, 2023 at 11:34:46AM -0800, Saurabh Sengar wrote:
> > > Zero is invalid virq and should't be returned as a valid value for
> > > lower irq bound. If IO-APIC and gsi_top are not initialized return
> >
> > Why isn't gsi_top initialized?
> >
> > What is this fixing?
> 
> In the absence of a device tree node for IO-APIC,  IO-APIC is not registered,
> resulting in uninitialized gsi_top. And in such cases arch_dynirq_lower_bound
> will return 0. Returning 0 from this function will allow interrupts to have 0
> assigned as valid irq, which is wrong. In case gsi_top is 0, lower bound of irq
> should be derived from 'hint' value passed to function as 'from'.
> 
> I can add above info in commit message,  please let me know if anything
> more to be added.
> 
> To be specific in our system which is a guest VM we don't need IO-APIC and
> hence there is no device tree node for it. It is observed that we get irq 0
> assigned to PCI-MSI.
> 
> >
> > Don't be afraid to do
> >
> > git annotate arch/x86/kernel/apic/io_apic.c
> >
> > and see which commit added this. This one:
> >
> > 3e5bedc2c258 ("x86/apic: Fix arch_dynirq_lower_bound() bug for DT
> > enabled
> > machines")
> >
> > Now add the folks from this commit to Cc and tell them why in your
> > case gsi_top is not initialized and what they're breaking by doing that.
> 
> Thanks. I will add "Fixes:" and "Cc:" tag in next version.
> 
> >
> > The more your commit message explains *why* you're fixing something,
> > the better it is for the maintainers/reviewers to actually know what to do.
> >
> > Right now I'm reading this and I'm thinking, random, unjustified change.
> > Ignore.
> >
> > Ok?
> >
> > Thx.
> >
> > --
> > Regards/Gruss,
> >     Boris.
> >
> >
> https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpeop
> >
> l%2F&data=05%7C01%7Cssengar%40microsoft.com%7C0595a41023f849c5ee
> b308db
> >
> 23732cd0%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C638142749
> 8888650
> >
> 08%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzI
> iLCJBTiI
> >
> 6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=qpOM5MMYpUof
> VOaNsp8HxTmv
> > %2B80iVn5rFfzNQTlTwLw%3D&reserved=0
> > e.kernel.org%2Ftglx%2Fnotes-about-
> >
> netiquette&data=05%7C01%7Cssengar%40microsoft.com%7C6e1e0e21051c4
> >
> 9c1cfe008db233a0376%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%
> >
> 7C638142504360574969%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjA
> >
> wMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C
> >
> %7C&sdata=OLdgb1AuLbLvlzucgNFBQEEK6G%2FsFV%2BO2TqT%2FNCujJU%3
> > D&reserved=0