linux-kernel - Re: [PATCH] PCI: hv: fix reading of PCI_INTERRUPT_LINE and PCI_INTERRUPT

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: <ZnXVgy_hVz5JXncD@liuwe-devbox-debian-v2>
Date: Fri, 21 Jun 2024 19:33:23 +0000
From: Wei Liu <wei.liu@...nel.org>
To: Dexuan Cui <decui@...rosoft.com>
Cc: Jake Oshins <jakeo@...rosoft.com>,
	Saurabh Singh Sengar <ssengar@...ux.microsoft.com>,
	Wei Liu <wei.liu@...nel.org>, mhklinux <mhklinux@...look.com>,
	Linux on Hyper-V List <linux-hyperv@...r.kernel.org>,
	"stable@...nel.org" <stable@...nel.org>,
	KY Srinivasan <kys@...rosoft.com>,
	Haiyang Zhang <haiyangz@...rosoft.com>,
	Lorenzo Pieralisi <lpieralisi@...nel.org>,
	Krzysztof Wilczyński <kw@...ux.com>,
	Rob Herring <robh@...nel.org>, Bjorn Helgaas <bhelgaas@...gle.com>,
	"open list:PCI NATIVE HOST BRIDGE AND ENDPOINT DRIVERS" <linux-pci@...r.kernel.org>,
	open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] PCI: hv: fix reading of PCI_INTERRUPT_LINE and
 PCI_INTERRUPT_PIN

On Fri, Jun 21, 2024 at 06:41:04PM +0000, Dexuan Cui wrote:
> From: Jake Oshins <jakeo@...rosoft.com> 
> Sent: Friday, June 21, 2024 9:51 AM
> > [...]
> >On Fri, Jun 21, 2024 at 06:19:05AM +0000, Wei Liu wrote:
> > On Fri, Jun 21, 2024 at 03:15:19AM +0000, Michael Kelley wrote:
> > > From: Wei Liu <mailto:wei.liu@...nel.org> Sent: Thursday, June 20, 2024 6:48 PM
> > > >
> > > > The intent of the code snippet is to always return 0 for both fields.
> > > > The check is wrong though. Fix that.
> > > >
> > > > This is discovered by this call in VFIO:
> > > >
> > > >     pci_read_config_byte(vdev->pdev, PCI_INTERRUPT_PIN, &pin);
> > > >
> > > > The old code does not set *val to 0 because the second half of the check is
> > > > incorrect.
> 
> Hi Wei, so you got a non-zero 'pin' value returned by the host when the guest reads
> from the MMIO config page. What's the consequence? Will VFIO try to use the legacy INTx 
> rather than MSI/MSI-X? I'm curious how you noticed the bug. I'm also curious why the
> host doesn't return 0 for the 'PIN' register when the guest reads it from the config page.

It is not the guest reading the register. The VM has not launched yet.
Everything happens on the host side. The host side software is preparing
the device for the VM to use.

The consequence of this bug is that user space code will think INTx is
available while in fact it is not.

VFIO itself doesn't care much. I noticed the bug because our VMM (Cloud
Hypervisor) initializes INTx whenever it is available.

> 
> >  I believe that this fix is correct.  (And I'm frankly surprised that this bug didn't
> > cause a problem before this.  It's been there since I first wrote the code.)
> > -- Jake Oshins
> 
> I suppose it didn't cause any issue because the PCI device drivers use MSI/MSI-X,
> so they don't care about the values of the 'PIN' and 'LINE' registers.

I suspect the same. Drivers almost always prefer MSI / MSI-X over INTx.
No one else triggered that code path before.

Thanks,
Wei.

> 
> Thanks,
> Dexuan
>