From: Joerg Roedel <joro@8bytes.org>
To: Andy Lutomirski <luto@amacapital.net>
Cc: Gleb Natapov <gleb@redhat.com>,
LKML <linux-kernel@vger.kernel.org>,
x86@kernel.org, "H. Peter Anvin" <hpa@zytor.com>,
Alex Williamson <alex.williamson@redhat.com>,
Don Zickus <dzickus@redhat.com>,
Prarit Bhargava <prarit@redhat.com>,
David Woodhouse <dwmw2@infradead.org>
Subject: Re: [PATCH] intel_iommu: Disable vfio and kvm interrupt assignment when unsafe
Date: Thu, 7 Feb 2013 18:27:01 +0100 [thread overview]
Message-ID: <20130207172701.GX25591@8bytes.org> (raw)
In-Reply-To: <CALCETrUzX9efPafJ0W2vxU-KgQ_FOxQ4YqGDKbw5z6d1P0ZYEQ@mail.gmail.com>
On Thu, Feb 07, 2013 at 08:29:42AM -0800, Andy Lutomirski wrote:
> On Thu, Feb 7, 2013 at 3:33 AM, Joerg Roedel <joro@8bytes.org> wrote:
> > On Wed, Feb 06, 2013 at 07:08:24PM -0800, Andy Lutomirski wrote:
> >> - if (x2apic_present)
> >> - WARN(1, KERN_WARNING
> >> - "Failed to enable irq remapping. You are vulnerable to irq-injection attacks.\n");
> >> -
> >> + irq_remapping_is_secure = 0;
> >> return -1;
> >> }
> >
> > Why do you remove this warning? It seems unrelated to the rest of the
> > patch.
>
> The idea is that setting irq_remapping_is_secure = 0 makes you (much
> less) vulnerable to irq-injection attacks: you're vulnerable to
> malicious hardware but not to attack via vfio or kvm, because those
> paths are disabled.
>
> I'd have no problem leaving the warning in and letting whoever manages
> to trigger it and get annoyed fix it. FWIW, it's actually likely to
> be interesting if the warning hits.
Hmm, looking into the intel_irq_remapping.c version in the tip tree
makes me wonder even more.
First, I wonder why the warning only hits when an x2apic is present. The
function is not x2apic-specific and the vulnerability also exists in
xapic mode. So that dependency can be removed.
Second, I think that it should be a pr_warn instead of a full WARN. When
IRQ remapping could not be enabled it's most likely because of the BIOS
or the hardware. So a message in the kernel log will do and the
backtrace provides no additional value.
Same is true for the warning in the function iommu_set_irq_remapping():
if (sts & DMA_GSTS_CFIS)
WARN(1, KERN_WARNING
"Compatibility-format IRQs enabled despite intr remapping;\n"
"you are vulnerable to IRQ injection.\n");
>From what I can see this condition depends only on the hardware too. So
a simple pr_warn() provides the same amount of information.
Regards,
Joerg
next prev parent reply other threads:[~2013-02-07 17:27 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-02-07 3:08 [PATCH] intel_iommu: Disable vfio and kvm interrupt assignment when unsafe Andy Lutomirski
2013-02-07 3:11 ` Andy Lutomirski
2013-02-07 11:33 ` Joerg Roedel
2013-02-07 16:29 ` Andy Lutomirski
2013-02-07 17:27 ` Joerg Roedel [this message]
2013-02-07 17:53 ` Andy Lutomirski
2013-02-07 20:51 ` Joerg Roedel
2013-02-07 21:02 ` David Woodhouse
2013-02-07 21:21 ` Joerg Roedel
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20130207172701.GX25591@8bytes.org \
--to=joro@8bytes.org \
--cc=alex.williamson@redhat.com \
--cc=dwmw2@infradead.org \
--cc=dzickus@redhat.com \
--cc=gleb@redhat.com \
--cc=hpa@zytor.com \
--cc=linux-kernel@vger.kernel.org \
--cc=luto@amacapital.net \
--cc=prarit@redhat.com \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.