linux-kernel - Re: [PATCH RFC] x86/earlyprintk: Don't fail the pciserial device with incorrect class code

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] [day] [month] [year] [list]

Message-ID: <20180928014739.owdi42l5q23f7uu7@shbuild888>
Date:   Fri, 28 Sep 2018 09:47:39 +0800
From:   Feng Tang <feng.tang@...el.com>
To:     Borislav Petkov <bp@...en8.de>
Cc:     Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...nel.org>,
        H Peter Anvin <hpa@...ux.intel.com>,
        Peter Zijlstra <peterz@...radead.org>,
        "Stuart R . Anderson" <stuart.r.anderson@...el.com>,
        alan@...ux.intel.com, x86@...nel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH RFC] x86/earlyprintk: Don't fail the pciserial device
 with incorrect class code

Hi Borislav,

Thanks for the review.

On Thu, Sep 27, 2018 at 03:30:50PM +0200, Borislav Petkov wrote:
> On Thu, Sep 27, 2018 at 08:43:20PM +0800, Feng Tang wrote:
> > "pciserial" earlyprintk helps much on many modern x86 platforms,
> > but unfortunately there are some platforms whose PCI UART devices
> > have wrong PCI class code, which will be blocked by current check.
> > 
> > So loose the class code check by giving a warning message instead.
> > This should be fine, as a developer who can give the accurate
> > BDF should know whether it's a usable UART device.
> 
> BDF? No.
> 
> Please write it out what it means.

Will do.

> > Signed-off-by: Feng Tang <feng.tang@...el.com>
> > ---
> >  arch/x86/kernel/early_printk.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/x86/kernel/early_printk.c b/arch/x86/kernel/early_printk.c
> > index 5e801c8..abe1d08 100644
> > --- a/arch/x86/kernel/early_printk.c
> > +++ b/arch/x86/kernel/early_printk.c
> > @@ -265,7 +265,8 @@ static __init void early_pci_serial_init(char *s)
> >  	if (((classcode >> 16 != PCI_CLASS_COMMUNICATION_MODEM) &&
> >  	     (classcode >> 16 != PCI_CLASS_COMMUNICATION_SERIAL)) ||
> >  	   (((classcode >> 8) & 0xff) != 0x02)) /* 16550 I/F at BAR0 */
> > -		return;
> > +		pr_warn("earlyprintk: classcode for pcidev %d:%d:%d shows it's not a UART like device, please check!\n",
> > +			bus, slot, func);
> 
> So where did the return statement go?
> 
> What are you trying to do here? If the device is still an UART device
> then we don't need the check at all as you're basically overriding it
> and only the class code is wrong.
> 
> If so, why do we need the pr_warn at all? What can the user do about the
> class code? Nothing, I'd say. I don't see her "fixing" PCI config space
> so that the device has a correct class code.

That's true. We can hardly do anything from OS side,

> 
> But what happens if the user really supplies the wrong BDF? We end up
> poking in PCI config space of *some* device and then the cat might catch
> fire.

Valid point, I can not argue on this :)

> 
> So it sounds to me like we need:
> 
> earlyprintk=pciserial,force,00:18.1,115200

I would follow this way, thanks

-Feng