Re: [RFC PATCH v2 1/3] KVM: x86: Protect the unused bits in the MSR filtering / exiting flags

[Sean Christopherson <seanjc@xxxxxxxxxx>]

On Tue, Jul 19, 2022, Aaron Lewis wrote:
> The flags used in KVM_CAP_X86_USER_SPACE_MSR and KVM_X86_SET_MSR_FILTER
> have no protection for their unused bits.  Without protection, future
> development for these features will be difficult.  Add the protection
> needed to make it possible to extend these features in the future.
> 
> Signed-off-by: Aaron Lewis <aaronlewis@xxxxxxxxxx>
> ---
>  arch/x86/include/uapi/asm/kvm.h | 1 +
>  arch/x86/kvm/x86.c              | 6 ++++++
>  include/uapi/linux/kvm.h        | 3 +++
>  3 files changed, 10 insertions(+)
> 
> diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h
> index ee3896416c68..63691a4c62d0 100644
> --- a/arch/x86/include/uapi/asm/kvm.h
> +++ b/arch/x86/include/uapi/asm/kvm.h
> @@ -224,6 +224,7 @@ struct kvm_msr_filter_range {
>  struct kvm_msr_filter {
>  #define KVM_MSR_FILTER_DEFAULT_ALLOW (0 << 0)

Well this is silly.  Can we wrap this with

#ifdef __KERNEL__
#define KVM_MSR_FILTER_DEFAULT_ALLOW (0 << 0)
#endif

so that we don't try to use it in the kernel?  E.g. I can see someone doing

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

and getting really confused when that doesn't work.

Or if we're feeling lucky, just remove it entirely as userspace doing

	filter.flags &= KVM_MSR_FILTER_DEFAULT_ALLOW;

is going to make someone sad someday.

>  #define KVM_MSR_FILTER_DEFAULT_DENY  (1 << 0)
> +#define KVM_MSR_FILTER_VALID_MASK (KVM_MSR_FILTER_DEFAULT_DENY)
>  	__u32 flags;
>  	struct kvm_msr_filter_range ranges[KVM_MSR_FILTER_MAX_RANGES];
>  };