[PATCH] x86, kvm: Clear paravirt_enabled on KVM guests for espfix32's benefit
paravirt_enabled has the following effects:
- Disables the F00F bug workaround warning. There is no F00F bug
workaround any more because Linux's standard IDT handling already
works around the F00F bug, but the warning still exists. This
is only cosmetic, and, in any event, there is no such thing as
KVM on a CPU with the F00F bug.
- Disables 32-bit APM BIOS detection. On a KVM paravirt system,
there should be no APM BIOS anyway.
- Disables tboot. I think that the tboot code should check the
CPUID hypervisor bit directly if it matters.
- paravirt_enabled disables espfix32. espfix32 should *not* be
disabled under KVM paravirt.
The last point is the purpose of this patch. It fixes a leak of the
high 16 bits of the kernel stack address on 32-bit KVM paravirt
guests.
While I'm at it, this removes pv_info setup from kvmclock. That
code seems to serve no purpose.
Cc: sta...@vger.kernel.org
Signed-off-by: Andy Lutomirski
---
arch/x86/kernel/kvm.c | 9 -
arch/x86/kernel/kvmclock.c | 2 --
2 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index f6945bef2cd1..94f643484300 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -283,7 +283,14 @@ NOKPROBE_SYMBOL(do_async_page_fault);
static void __init paravirt_ops_setup(void)
{
pv_info.name = "KVM";
- pv_info.paravirt_enabled = 1;
+
+ /*
+* KVM isn't paravirt in the sense of paravirt_enabled. A KVM
+* guest kernel works like a bare metal kernel with additional
+* features, and paravirt_enabled is about features that are
+* missing.
+*/
+ pv_info.paravirt_enabled = 0;
if (kvm_para_has_feature(KVM_FEATURE_NOP_IO_DELAY))
pv_cpu_ops.io_delay = kvm_io_delay;
diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c
index d9156ceecdff..d4d9a8ad7893 100644
--- a/arch/x86/kernel/kvmclock.c
+++ b/arch/x86/kernel/kvmclock.c
@@ -263,8 +263,6 @@ void __init kvmclock_init(void)
#endif
kvm_get_preset_lpj();
clocksource_register_hz(&kvm_clock, NSEC_PER_SEC);
- pv_info.paravirt_enabled = 1;
- pv_info.name = "KVM";
if (kvm_para_has_feature(KVM_FEATURE_CLOCKSOURCE_STABLE_BIT))
pvclock_set_flags(PVCLOCK_TSC_STABLE_BIT);
--
1.9.3
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] x86, kvm: Clear paravirt_enabled on KVM guests for espfix32's benefit
On Fri, Dec 5, 2014 at 7:03 PM, Andy Lutomirski wrote:
> paravirt_enabled has the following effects:
>
> - Disables the F00F bug workaround warning. There is no F00F bug
>workaround any more because Linux's standard IDT handling already
>works around the F00F bug, but the warning still exists. This
>is only cosmetic, and, in any event, there is no such thing as
>KVM on a CPU with the F00F bug.
>
> - Disables 32-bit APM BIOS detection. On a KVM paravirt system,
>there should be no APM BIOS anyway.
>
> - Disables tboot. I think that the tboot code should check the
>CPUID hypervisor bit directly if it matters.
>
> - paravirt_enabled disables espfix32. espfix32 should *not* be
>disabled under KVM paravirt.
>
> The last point is the purpose of this patch. It fixes a leak of the
> high 16 bits of the kernel stack address on 32-bit KVM paravirt
> guests.
>
> While I'm at it, this removes pv_info setup from kvmclock. That
> code seems to serve no purpose.
If you apply this, please add:
Fixes CVE-2014-8134.
Thanks,
Andy
>
> Cc: sta...@vger.kernel.org
> Signed-off-by: Andy Lutomirski
> ---
> arch/x86/kernel/kvm.c | 9 -
> arch/x86/kernel/kvmclock.c | 2 --
> 2 files changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
> index f6945bef2cd1..94f643484300 100644
> --- a/arch/x86/kernel/kvm.c
> +++ b/arch/x86/kernel/kvm.c
> @@ -283,7 +283,14 @@ NOKPROBE_SYMBOL(do_async_page_fault);
> static void __init paravirt_ops_setup(void)
> {
> pv_info.name = "KVM";
> - pv_info.paravirt_enabled = 1;
> +
> + /*
> +* KVM isn't paravirt in the sense of paravirt_enabled. A KVM
> +* guest kernel works like a bare metal kernel with additional
> +* features, and paravirt_enabled is about features that are
> +* missing.
> +*/
> + pv_info.paravirt_enabled = 0;
>
> if (kvm_para_has_feature(KVM_FEATURE_NOP_IO_DELAY))
> pv_cpu_ops.io_delay = kvm_io_delay;
> diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c
> index d9156ceecdff..d4d9a8ad7893 100644
> --- a/arch/x86/kernel/kvmclock.c
> +++ b/arch/x86/kernel/kvmclock.c
> @@ -263,8 +263,6 @@ void __init kvmclock_init(void)
> #endif
> kvm_get_preset_lpj();
> clocksource_register_hz(&kvm_clock, NSEC_PER_SEC);
> - pv_info.paravirt_enabled = 1;
> - pv_info.name = "KVM";
>
> if (kvm_para_has_feature(KVM_FEATURE_CLOCKSOURCE_STABLE_BIT))
> pvclock_set_flags(PVCLOCK_TSC_STABLE_BIT);
> --
> 1.9.3
>
--
Andy Lutomirski
AMA Capital Management, LLC
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] x86, kvm: Clear paravirt_enabled on KVM guests for espfix32's benefit
On Fri, Dec 05, 2014 at 07:03:28PM -0800, Andy Lutomirski wrote:
> paravirt_enabled has the following effects:
>
> - Disables the F00F bug workaround warning. There is no F00F bug
>workaround any more because Linux's standard IDT handling already
>works around the F00F bug, but the warning still exists. This
>is only cosmetic, and, in any event, there is no such thing as
>KVM on a CPU with the F00F bug.
>
> - Disables 32-bit APM BIOS detection. On a KVM paravirt system,
>there should be no APM BIOS anyway.
>
> - Disables tboot. I think that the tboot code should check the
>CPUID hypervisor bit directly if it matters.
>
> - paravirt_enabled disables espfix32. espfix32 should *not* be
>disabled under KVM paravirt.
>
> The last point is the purpose of this patch. It fixes a leak of the
> high 16 bits of the kernel stack address on 32-bit KVM paravirt
> guests.
>
> While I'm at it, this removes pv_info setup from kvmclock. That
> code seems to serve no purpose.
>
> Cc: sta...@vger.kernel.org
> Signed-off-by: Andy Lutomirski
Suggested-by: Konrad Rzeszutek Wilk
> ---
> arch/x86/kernel/kvm.c | 9 -
> arch/x86/kernel/kvmclock.c | 2 --
> 2 files changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
> index f6945bef2cd1..94f643484300 100644
> --- a/arch/x86/kernel/kvm.c
> +++ b/arch/x86/kernel/kvm.c
> @@ -283,7 +283,14 @@ NOKPROBE_SYMBOL(do_async_page_fault);
> static void __init paravirt_ops_setup(void)
> {
> pv_info.name = "KVM";
> - pv_info.paravirt_enabled = 1;
> +
> + /*
> + * KVM isn't paravirt in the sense of paravirt_enabled. A KVM
> + * guest kernel works like a bare metal kernel with additional
> + * features, and paravirt_enabled is about features that are
> + * missing.
> + */
> + pv_info.paravirt_enabled = 0;
>
> if (kvm_para_has_feature(KVM_FEATURE_NOP_IO_DELAY))
> pv_cpu_ops.io_delay = kvm_io_delay;
> diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c
> index d9156ceecdff..d4d9a8ad7893 100644
> --- a/arch/x86/kernel/kvmclock.c
> +++ b/arch/x86/kernel/kvmclock.c
> @@ -263,8 +263,6 @@ void __init kvmclock_init(void)
> #endif
> kvm_get_preset_lpj();
> clocksource_register_hz(&kvm_clock, NSEC_PER_SEC);
> - pv_info.paravirt_enabled = 1;
> - pv_info.name = "KVM";
>
> if (kvm_para_has_feature(KVM_FEATURE_CLOCKSOURCE_STABLE_BIT))
> pvclock_set_flags(PVCLOCK_TSC_STABLE_BIT);
> --
> 1.9.3
>
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] x86, kvm: Clear paravirt_enabled on KVM guests for espfix32's benefit
On Mon, Dec 8, 2014 at 7:45 AM, Konrad Rzeszutek Wilk
wrote:
> On Fri, Dec 05, 2014 at 07:03:28PM -0800, Andy Lutomirski wrote:
>> paravirt_enabled has the following effects:
>>
>> - Disables the F00F bug workaround warning. There is no F00F bug
>>workaround any more because Linux's standard IDT handling already
>>works around the F00F bug, but the warning still exists. This
>>is only cosmetic, and, in any event, there is no such thing as
>>KVM on a CPU with the F00F bug.
>>
>> - Disables 32-bit APM BIOS detection. On a KVM paravirt system,
>>there should be no APM BIOS anyway.
>>
>> - Disables tboot. I think that the tboot code should check the
>>CPUID hypervisor bit directly if it matters.
>>
>> - paravirt_enabled disables espfix32. espfix32 should *not* be
>>disabled under KVM paravirt.
>>
>> The last point is the purpose of this patch. It fixes a leak of the
>> high 16 bits of the kernel stack address on 32-bit KVM paravirt
>> guests.
>>
>> While I'm at it, this removes pv_info setup from kvmclock. That
>> code seems to serve no purpose.
>>
>> Cc: sta...@vger.kernel.org
>> Signed-off-by: Andy Lutomirski
>
> Suggested-by: Konrad Rzeszutek Wilk
Sorry, meant to add that but forgot. Too many patches late last week :(
>> ---
>> arch/x86/kernel/kvm.c | 9 -
>> arch/x86/kernel/kvmclock.c | 2 --
>> 2 files changed, 8 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
>> index f6945bef2cd1..94f643484300 100644
>> --- a/arch/x86/kernel/kvm.c
>> +++ b/arch/x86/kernel/kvm.c
>> @@ -283,7 +283,14 @@ NOKPROBE_SYMBOL(do_async_page_fault);
>> static void __init paravirt_ops_setup(void)
>> {
>> pv_info.name = "KVM";
>> - pv_info.paravirt_enabled = 1;
>> +
>> + /*
>> + * KVM isn't paravirt in the sense of paravirt_enabled. A KVM
>> + * guest kernel works like a bare metal kernel with additional
>> + * features, and paravirt_enabled is about features that are
>> + * missing.
>> + */
>> + pv_info.paravirt_enabled = 0;
>>
>> if (kvm_para_has_feature(KVM_FEATURE_NOP_IO_DELAY))
>> pv_cpu_ops.io_delay = kvm_io_delay;
>> diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c
>> index d9156ceecdff..d4d9a8ad7893 100644
>> --- a/arch/x86/kernel/kvmclock.c
>> +++ b/arch/x86/kernel/kvmclock.c
>> @@ -263,8 +263,6 @@ void __init kvmclock_init(void)
>> #endif
>> kvm_get_preset_lpj();
>> clocksource_register_hz(&kvm_clock, NSEC_PER_SEC);
>> - pv_info.paravirt_enabled = 1;
>> - pv_info.name = "KVM";
>>
>> if (kvm_para_has_feature(KVM_FEATURE_CLOCKSOURCE_STABLE_BIT))
>> pvclock_set_flags(PVCLOCK_TSC_STABLE_BIT);
>> --
>> 1.9.3
>>
--
Andy Lutomirski
AMA Capital Management, LLC
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] x86, kvm: Clear paravirt_enabled on KVM guests for espfix32's benefit
On 06/12/2014 04:03, Andy Lutomirski wrote:
> paravirt_enabled has the following effects:
>
> - Disables the F00F bug workaround warning. There is no F00F bug
>workaround any more because Linux's standard IDT handling already
>works around the F00F bug, but the warning still exists. This
>is only cosmetic, and, in any event, there is no such thing as
>KVM on a CPU with the F00F bug.
>
> - Disables 32-bit APM BIOS detection. On a KVM paravirt system,
>there should be no APM BIOS anyway.
>
> - Disables tboot. I think that the tboot code should check the
>CPUID hypervisor bit directly if it matters.
>
> - paravirt_enabled disables espfix32. espfix32 should *not* be
>disabled under KVM paravirt.
>
> The last point is the purpose of this patch. It fixes a leak of the
> high 16 bits of the kernel stack address on 32-bit KVM paravirt
> guests.
>
> While I'm at it, this removes pv_info setup from kvmclock. That
> code seems to serve no purpose.
kvmclock_init runs before kvm_guest_init, and this is a stable@ patch so
for the sake of extra safety I've left the pv_info.name assignment in.
Applied (locally for now), will be in 3.19.
Paolo
> Cc: sta...@vger.kernel.org
> Signed-off-by: Andy Lutomirski
> ---
> arch/x86/kernel/kvm.c | 9 -
> arch/x86/kernel/kvmclock.c | 2 --
> 2 files changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
> index f6945bef2cd1..94f643484300 100644
> --- a/arch/x86/kernel/kvm.c
> +++ b/arch/x86/kernel/kvm.c
> @@ -283,7 +283,14 @@ NOKPROBE_SYMBOL(do_async_page_fault);
> static void __init paravirt_ops_setup(void)
> {
> pv_info.name = "KVM";
> - pv_info.paravirt_enabled = 1;
> +
> + /*
> + * KVM isn't paravirt in the sense of paravirt_enabled. A KVM
> + * guest kernel works like a bare metal kernel with additional
> + * features, and paravirt_enabled is about features that are
> + * missing.
> + */
> + pv_info.paravirt_enabled = 0;
>
> if (kvm_para_has_feature(KVM_FEATURE_NOP_IO_DELAY))
> pv_cpu_ops.io_delay = kvm_io_delay;
> diff --git a/arch/x86/kernel/kvmclock.c b/arch/x86/kernel/kvmclock.c
> index d9156ceecdff..d4d9a8ad7893 100644
> --- a/arch/x86/kernel/kvmclock.c
> +++ b/arch/x86/kernel/kvmclock.c
> @@ -263,8 +263,6 @@ void __init kvmclock_init(void)
> #endif
> kvm_get_preset_lpj();
> clocksource_register_hz(&kvm_clock, NSEC_PER_SEC);
> - pv_info.paravirt_enabled = 1;
> - pv_info.name = "KVM";
>
> if (kvm_para_has_feature(KVM_FEATURE_CLOCKSOURCE_STABLE_BIT))
> pvclock_set_flags(PVCLOCK_TSC_STABLE_BIT);
>
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] x86, kvm: Clear paravirt_enabled on KVM guests for espfix32's benefit
On Wed, Dec 10, 2014 at 3:49 AM, Paolo Bonzini wrote: > > > On 06/12/2014 04:03, Andy Lutomirski wrote: >> paravirt_enabled has the following effects: >> >> - Disables the F00F bug workaround warning. There is no F00F bug >>workaround any more because Linux's standard IDT handling already >>works around the F00F bug, but the warning still exists. This >>is only cosmetic, and, in any event, there is no such thing as >>KVM on a CPU with the F00F bug. >> >> - Disables 32-bit APM BIOS detection. On a KVM paravirt system, >>there should be no APM BIOS anyway. >> >> - Disables tboot. I think that the tboot code should check the >>CPUID hypervisor bit directly if it matters. >> >> - paravirt_enabled disables espfix32. espfix32 should *not* be >>disabled under KVM paravirt. >> >> The last point is the purpose of this patch. It fixes a leak of the >> high 16 bits of the kernel stack address on 32-bit KVM paravirt >> guests. >> >> While I'm at it, this removes pv_info setup from kvmclock. That >> code seems to serve no purpose. > > kvmclock_init runs before kvm_guest_init, and this is a stable@ patch so > for the sake of extra safety I've left the pv_info.name assignment in. > Applied (locally for now), will be in 3.19. > In the interest of reduced future confusion, would it make sense to drop the duplicate initialization for 3.20? --Andy -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] x86, kvm: Clear paravirt_enabled on KVM guests for espfix32's benefit
> In the interest of reduced future confusion, would it make sense to > drop the duplicate initialization for 3.20? Yup. It would be great if possible to even unify the two init functions, but I haven't checked what happens in the middle. Paolo -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
