From: Andrew Cooper <andrew.cooper3@citrix.com>
To: Thomas Gleixner <tglx@linutronix.de>,
LKML <linux-kernel@vger.kernel.org>
Cc: <x86@kernel.org>, Nadav Amit <namit@vmware.com>,
Ricardo Neri <ricardo.neri-calderon@linux.intel.com>,
Stephane Eranian <eranian@google.com>,
Feng Tang <feng.tang@intel.com>,
Andy Lutomirski <luto@kernel.org>
Subject: Re: [patch V2 04/25] x86/apic: Make apic_pending_intr_clear() more robust
Date: Fri, 5 Jul 2019 16:47:35 +0100 [thread overview]
Message-ID: <958a67c2-4dc0-52e6-43b2-1ebd25a59232@citrix.com> (raw)
In-Reply-To: <20190704155608.636478018@linutronix.de>
On 04/07/2019 16:51, Thomas Gleixner wrote:
> 2) The loop termination logic is interesting at best.
>
> If the machine has no TSC or cpu_khz is not known yet it tries 1
> million times to ack stale IRR/ISR bits. What?
>
> With TSC it uses the TSC to calculate the loop termination. It takes a
> timestamp at entry and terminates the loop when:
>
> (rdtsc() - start_timestamp) >= (cpu_hkz << 10)
>
> That's roughly one second.
>
> Both methods are problematic. The APIC has 256 vectors, which means
> that in theory max. 256 IRR/ISR bits can be set. In practice this is
> impossible as the first 32 vectors are reserved and not affected and
> the chance that more than a few bits are set is close to zero.
[Disclaimer. I talked to Thomas in private first, and he asked me to
post this publicly as the CVE is almost a decade old already.]
I'm afraid that this isn't quite true.
In terms of IDT vectors, the first 32 are reserved for exceptions, but
only the first 16 are reserved in the LAPIC. Vectors 16-31 are fair
game for incoming IPIs (SDM Vol3, 10.5.2 Valid Interrupt Vectors).
In practice, this makes Linux vulnerable to CVE-2011-1898 / XSA-3, which
I'm disappointed to see wasn't shared with other software vendors at the
time.
Because TPR is 0, an incoming IPI can trigger #AC, #CP, #VC or #SX
without an error code on the stack, which results in a corrupt pt_regs
in the exception handler, and a stack underflow on the way back out,
most likely with a fault on IRET.
These can be addressed by setting TPR to 0x10, which will inhibit
delivery of any errant IPIs in this range, but some extra sanity logic
may not go amiss. An error code on a 64bit stack can be spotted with
`testb $8, %spl` due to %rsp being aligned before pushing the exception
frame.
Another interesting problem is an IPI which its vector 0x80. A cunning
attacker can use this to simulate system calls from unsuspecting
positions in userspace, or for interrupting kernel context. At the very
least the int0x80 path does an unconditional swapgs, so will try to run
with the user gs, and I expect things will explode quickly from there.
One option here is to look at ISR and complain if it is found to be set.
Another option, which I've only just remembered, is that AMD hardware
has the Interrupt Enable Register in its extended APIC space, which may
or may not be good enough to prohibit delivery of 0x80. There isn't
enough information in the APM to be clear, but the name suggests it is
worth experimenting with.
~Andrew
next prev parent reply other threads:[~2019-07-05 15:47 UTC|newest]
Thread overview: 42+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-07-04 15:51 [patch V2 00/25] x86/apic: Support for IPI shorthands Thomas Gleixner
2019-07-04 15:51 ` [patch V2 01/25] x86/kgbd: Use NMI_VECTOR not APIC_DM_NMI Thomas Gleixner
2019-07-05 21:43 ` Thomas Gleixner
2019-07-04 15:51 ` [patch V2 02/25] x86/apic: Invoke perf_events_lapic_init() after enabling APIC Thomas Gleixner
2019-07-04 15:51 ` [patch V2 03/25] x86/apic: Soft disable APIC before initializing it Thomas Gleixner
2019-07-04 15:51 ` [patch V2 04/25] x86/apic: Make apic_pending_intr_clear() more robust Thomas Gleixner
2019-07-05 15:47 ` Andrew Cooper [this message]
2019-07-05 19:06 ` Andy Lutomirski
2019-07-05 20:17 ` Andrew Cooper
2019-07-05 20:36 ` Thomas Gleixner
2019-07-05 20:39 ` Andy Lutomirski
2019-07-07 8:27 ` Thomas Gleixner
2019-07-05 19:19 ` Nadav Amit
2019-07-05 20:47 ` Andrew Cooper
2019-07-05 20:25 ` Thomas Gleixner
2019-07-05 20:37 ` Andy Lutomirski
2019-07-05 20:49 ` Paolo Bonzini
2019-07-05 21:16 ` Andrew Cooper
2019-07-07 8:37 ` Thomas Gleixner
2019-07-09 14:43 ` Thomas Gleixner
2019-07-04 15:51 ` [patch V2 05/25] x86/apic: Move IPI inlines into ipi.c Thomas Gleixner
2019-07-04 15:51 ` [patch V2 06/25] x86/apic: Cleanup the include maze Thomas Gleixner
2019-07-04 15:51 ` [patch V2 07/25] x86/apic: Move ipi header into apic directory Thomas Gleixner
2019-07-04 15:51 ` [patch V2 08/25] x86/apic: Move apic_flat_64 " Thomas Gleixner
2019-07-04 15:51 ` [patch V2 09/25] x86/apic: Consolidate the apic local headers Thomas Gleixner
2019-07-04 15:51 ` [patch V2 10/25] x86/apic/uv: Make x2apic_extra_bits static Thomas Gleixner
2019-07-04 15:51 ` [patch V2 11/25] smp/hotplug: Track booted once CPUs in a cpumask Thomas Gleixner
2019-07-04 15:51 ` [patch V2 12/25] x86/cpu: Move arch_smt_update() to a neutral place Thomas Gleixner
2019-07-04 15:51 ` [patch V2 13/25] x86/hotplug: Silence APIC and NMI when CPU is dead Thomas Gleixner
2019-07-04 15:51 ` [patch V2 14/25] x86/apic: Remove dest argument from __default_send_IPI_shortcut() Thomas Gleixner
2019-07-04 15:52 ` [patch V2 15/25] x86/apic: Add NMI_VECTOR wait to IPI shorthand Thomas Gleixner
2019-07-04 15:52 ` [patch V2 16/25] x86/apic: Move no_ipi_broadcast() out of 32bit Thomas Gleixner
2019-07-04 15:52 ` [patch V2 17/25] x86/apic: Add static key to Control IPI shorthands Thomas Gleixner
2019-07-04 15:52 ` [patch V2 18/25] x86/apic: Provide and use helper for send_IPI_allbutself() Thomas Gleixner
2019-07-04 15:52 ` [patch V2 19/25] cpumask: Implement cpumask_or_equal() Thomas Gleixner
2019-07-04 15:52 ` [patch V2 20/25] x86/smp: Move smp_function_call implementations into IPI code Thomas Gleixner
2019-07-04 15:52 ` [patch V2 21/25] x86/smp: Enhance native_send_call_func_ipi() Thomas Gleixner
2019-07-05 1:26 ` Nadav Amit
2019-07-04 15:52 ` [patch V2 22/25] x86/apic: Remove the shorthand decision logic Thomas Gleixner
2019-07-04 15:52 ` [patch V2 23/25] x86/apic: Share common IPI helpers Thomas Gleixner
2019-07-04 15:52 ` [patch V2 24/25] x86/apic/flat64: Remove the IPI shorthand decision logic Thomas Gleixner
2019-07-04 15:52 ` [patch V2 25/25] x86/apic/x2apic: Implement IPI shorthands support Thomas Gleixner
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=958a67c2-4dc0-52e6-43b2-1ebd25a59232@citrix.com \
--to=andrew.cooper3@citrix.com \
--cc=eranian@google.com \
--cc=feng.tang@intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=luto@kernel.org \
--cc=namit@vmware.com \
--cc=ricardo.neri-calderon@linux.intel.com \
--cc=tglx@linutronix.de \
--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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Powered by JetHome