linux-kernel - Re: [PATCH] xen-pciback: Consider MSI-X enabled only when MASKALL bit is cleared

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]

Date:   Thu, 17 Nov 2022 15:32:00 +0100
From:   Marek Marczykowski-Górecki 
        <marmarek@...isiblethingslab.com>
To:     Jan Beulich <jbeulich@...e.com>
Cc:     linux-kernel@...r.kernel.org, Juergen Gross <jgross@...e.com>,
        Stefano Stabellini <sstabellini@...nel.org>,
        Oleksandr Tyshchenko <oleksandr_tyshchenko@...m.com>,
        "moderated list:XEN HYPERVISOR INTERFACE" 
        <xen-devel@...ts.xenproject.org>, David Vrabel <dvrabel@...tab.net>
Subject: Re: [PATCH] xen-pciback: Consider MSI-X enabled only when MASKALL
 bit is cleared

On Thu, Nov 17, 2022 at 02:33:16PM +0100, Jan Beulich wrote:
> On 17.11.2022 14:13, Marek Marczykowski-Górecki wrote:
> > On Thu, Nov 17, 2022 at 12:54:51PM +0000, David Vrabel wrote:
> >> On 17/11/2022 11:41, Marek Marczykowski-Górecki wrote:
> >>> Linux enables MSI-X before disabling INTx, but keeps MSI-X masked until
> >>> the table is filled. Then it disables INTx just before clearing MASKALL
> >>> bit. Currently this approach is rejected by xen-pciback.
> >>> Allow setting PCI_MSIX_FLAGS_ENABLE while INTx is still enabled as long
> >>> as PCI_MSIX_FLAGS_MASKALL is set too.
> >>
> >> The use of MSI-X interrupts is conditional on only the MSI-X Enable bit.
> >> Setting MSI-X Enable effectively overrides the Interrupt Disable bit in the
> >> Command register.
> > 
> > That means the second chunk of the patch may even drop the '(new_value &
> > PCI_MSIX_FLAGS_MASKALL)' part, right? 
> > 
> >> PCIe 6.0.1 section 7.7.2.2. "MSI-X Enable ... is prohibited from using INTx
> >> interrupts (if implemented)." And there is similar wording for MSI Enable.
> > 
> > And this would mean the 'field_config->int_type == INTERRUPT_TYPE_MSIX'
> > part isn't necessary either.
> > 
> > Jan in another thread pointed out that disabling INTx explicitly is
> > still a useful workaround for a flawed hardware. But if that isn't
> > mandated by the spec, maybe it doesn't need to be enforced by pciback
> > either?
> 
> Well, allowing a device to go into a mode exhibiting undefined behavior
> is what we ought to prevent when it comes to a DomU doing so vs overall
> host safety.

If the spec prohibits using INTx if MSI/MSI-X is enabled (regardless of
PCI_COMMAND_INTX_DISABLE bit), then well-behaving device should be fine
(we aren't hitting undefined behavior). As for buggy device, it wouldn't
be much different from a device ignoring PCI_COMMAND_INTX_DISABLE
completely, no (besides the latter being probably much less probable
bug)?
If the above is assumption is correct, it seems such device may not
function correctly without extra workarounds (which are in the driver
interest to apply), but should not affect overall host safety (as in:
beyond the guest having that device assigned). I think pciback should
only enforce what's necessary to prevent one guest hurting others (or
the hypervisor), but it doesn't need to prevent guest hurting itself.

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)