diff mbox series

[v4,1/5] KVM: x86: Disallow the use of KVM_MSR_FILTER_DEFAULT_ALLOW in the kernel

Message ID 20220921151525.904162-2-aaronlewis@google.com (mailing list archive)
State New, archived
Headers show
Series MSR filtering and MSR exiting flag clean up | expand

Commit Message

Aaron Lewis Sept. 21, 2022, 3:15 p.m. UTC
Protect the kernel from using the flag KVM_MSR_FILTER_DEFAULT_ALLOW.
Its value is 0, and using it incorrectly could have unintended
consequences. E.g. prevent someone in the kernel from writing something
like this.

if (filter.flags & KVM_MSR_FILTER_DEFAULT_ALLOW)
        <allow the MSR>

and getting confused when it doesn't work.

It would be more ideal to remove this flag altogether, but userspace
may already be using it, so protecting the kernel is all that can
reasonably be done at this point.

Suggested-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Aaron Lewis <aaronlewis@google.com>
---

Google's VMM is already using this flag, so we *know* that dropping the
flag entirely will break userspace.  All we can do at this point is
prevent the kernel from using it.

 arch/x86/include/uapi/asm/kvm.h | 2 ++
 1 file changed, 2 insertions(+)

Comments

Sean Christopherson Oct. 7, 2022, 9:57 p.m. UTC | #1
On Wed, Sep 21, 2022, Aaron Lewis wrote:
> Protect the kernel from using the flag KVM_MSR_FILTER_DEFAULT_ALLOW.
> Its value is 0, and using it incorrectly could have unintended
> consequences. E.g. prevent someone in the kernel from writing something
> like this.
> 
> if (filter.flags & KVM_MSR_FILTER_DEFAULT_ALLOW)
>         <allow the MSR>
> 
> and getting confused when it doesn't work.
> 
> It would be more ideal to remove this flag altogether, but userspace
> may already be using it, so protecting the kernel is all that can
> reasonably be done at this point.
> 
> Suggested-by: Sean Christopherson <seanjc@google.com>
> Signed-off-by: Aaron Lewis <aaronlewis@google.com>
> ---

Reviewed-by: Sean Christopherson <seanjc@google.com>

> Google's VMM is already using this flag, so we *know* that dropping the
> flag entirely will break userspace.  All we can do at this point is
> prevent the kernel from using it.

LOL, nice.
diff mbox series

Patch

diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h
index 46de10a809ec..73ad693aa653 100644
--- a/arch/x86/include/uapi/asm/kvm.h
+++ b/arch/x86/include/uapi/asm/kvm.h
@@ -222,7 +222,9 @@  struct kvm_msr_filter_range {
 
 #define KVM_MSR_FILTER_MAX_RANGES 16
 struct kvm_msr_filter {
+#ifndef __KERNEL__
 #define KVM_MSR_FILTER_DEFAULT_ALLOW (0 << 0)
+#endif
 #define KVM_MSR_FILTER_DEFAULT_DENY  (1 << 0)
 	__u32 flags;
 	struct kvm_msr_filter_range ranges[KVM_MSR_FILTER_MAX_RANGES];