> +/* kvm_sync_regs struct included by kvm_run struct */
> struct kvm_sync_regs {
> + /* Members of this structure are potentially malicious.
> + * Care must be taken by code reading, esp. interpreting,
> + * data fields from them inside KVM to prevent TOCTOU and
> + * double-fetch types of vulnerabilities.
> + */
Indeed, did you check if the set functions are safe?
> + struct kvm_regs regs;
> + struct kvm_sregs sregs;
> + struct kvm_vcpu_events events;
> };
>
> #define KVM_X86_QUIRK_LINT0_REENABLED (1 << 0)
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 0204b2b8a293..ffd2c4e5d245 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -100,6 +100,9 @@ static void update_cr8_intercept(struct kvm_vcpu *vcpu);
> static void process_nmi(struct kvm_vcpu *vcpu);
> static void enter_smm(struct kvm_vcpu *vcpu);
> static void __kvm_set_rflags(struct kvm_vcpu *vcpu, unsigned long rflags);
> +static void store_regs(struct kvm_vcpu *vcpu);
> +static int sync_regs(struct kvm_vcpu *vcpu);
> +static int check_valid_regs(struct kvm_vcpu *vcpu);
>
> struct kvm_x86_ops *kvm_x86_ops __read_mostly;
> EXPORT_SYMBOL_GPL(kvm_x86_ops);
> @@ -2743,6 +2746,9 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
> case KVM_CAP_IMMEDIATE_EXIT:
> r = 1;
> break;
> + case KVM_CAP_SYNC_REGS:
> + r = KVM_SYNC_X86_VALID_FIELDS;
> + break;
> case KVM_CAP_ADJUST_CLOCK:
> r = KVM_CLOCK_TSC_STABLE;
> break;
> @@ -7325,6 +7331,12 @@ static int complete_emulated_mmio(struct kvm_vcpu *vcpu)
> return 0;
> }
>
> +static int check_valid_regs(struct kvm_vcpu *vcpu)
> +{
> + if (vcpu->run->kvm_valid_regs & ~KVM_SYNC_X86_VALID_FIELDS)
> + return -EINVAL;
> + return 0;
> +}
>
> int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
> {
> @@ -7351,6 +7363,17 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
> goto out;
> }
>
> + if (vcpu->run->kvm_valid_regs) {
You can check directly, without this if. (and not sure if the extra
function is needed here).
> + r = check_valid_regs(vcpu);
> + if (r != 0)
> + goto out;
> + }
> + if (vcpu->run->kvm_dirty_regs) {
> + r = sync_regs(vcpu);
> + if (r != 0)
> + goto out;
> + }
> +
> /* re-sync apic's tpr */
> if (!lapic_in_kernel(vcpu)) {
> if (kvm_set_cr8(vcpu, kvm_run->cr8) != 0) {
> @@ -7375,6 +7398,8 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
>
Looks good to me
Reviewed-by: David Hildenbrand <david@xxxxxxxxxx>
--
Thanks,
David / dhildenb
