diff mbox series

[v4] kvm: x86: Disallow invalid exceptions from userspace

Message ID 20181023213222.259677-1-jmattson@google.com (mailing list archive)
State New, archived
Headers show
Series [v4] kvm: x86: Disallow invalid exceptions from userspace | expand

Commit Message

Jim Mattson Oct. 23, 2018, 9:32 p.m. UTC
Userspace should not be able to inject an invalid exception into a kvm
guest via KVM_SET_VCPU_EVENTS, nor should it be able to convert a
valid pending exception to an invalid one via KVM_SET_SREGS.

In particular, only certain exceptions deliver an error code in
protected mode, and no exception delivers an error code in
real-address mode.

Reported-by: syzbot <syzkaller@googlegroups.com>
Signed-off-by: Jim Mattson <jmattson@google.com>
---
 arch/x86/kvm/x86.c | 27 ++++++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

Comments

Krish Sadhukhan Oct. 24, 2018, 1:57 a.m. UTC | #1
On 10/23/2018 02:32 PM, Jim Mattson wrote:
> Userspace should not be able to inject an invalid exception into a kvm
> guest via KVM_SET_VCPU_EVENTS, nor should it be able to convert a
> valid pending exception to an invalid one via KVM_SET_SREGS.
>
> In particular, only certain exceptions deliver an error code in
> protected mode, and no exception delivers an error code in
> real-address mode.
>
> Reported-by: syzbot <syzkaller@googlegroups.com>
> Signed-off-by: Jim Mattson <jmattson@google.com>
> ---
>   arch/x86/kvm/x86.c | 27 ++++++++++++++++++++++++---
>   1 file changed, 24 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 66d66d77caee..47b4d1099467 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -3528,9 +3528,18 @@ static int kvm_vcpu_ioctl_x86_set_vcpu_events(struct kvm_vcpu *vcpu,
>   		events->exception_has_payload = 0;
>   	}
>   
> -	if ((events->exception.injected || events->exception.pending) &&
> -	    (events->exception.nr > 31 || events->exception.nr == NMI_VECTOR))
> -		return -EINVAL;
> +	if (events->exception.injected || events->exception.pending) {
> +		bool needs_error_code;

Nit: For readability purposes, it will make more sense to re-name the 
variable to 'valid_error_code'.

> +
> +		if (events->exception.nr > 31 ||
> +		    events->exception.nr == NMI_VECTOR)
> +			return -EINVAL;
> +
> +		needs_error_code = is_protmode(vcpu) &&
> +			x86_exception_has_error_code(events->exception.nr);
> +		if (!!events->exception.has_error_code != needs_error_code)
> +			return -EINVAL;
> +	}
>   
>   	/* INITs are latched while in SMM */
>   	if (events->flags & KVM_VCPUEVENT_VALID_SMM &&
> @@ -8312,6 +8321,18 @@ static int kvm_valid_sregs(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs)
>   			return -EINVAL;
>   	}
>   
> +	/*
> +	 * Switching between real-address mode and protected mode can
> +	 * potentially invalidate a pending exception.
> +	 */
> +	if (vcpu->arch.exception.injected || vcpu->arch.exception.pending) {
> +		bool needs_error_code = (sregs->cr0 & X86_CR0_PE) &&
> +			x86_exception_has_error_code(vcpu->arch.exception.nr);
> +
> +		if (vcpu->arch.exception.has_error_code != needs_error_code)
> +			return -EINVAL;
> +	}
> +
>   	return 0;
>   }
>   

Reviewed-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
diff mbox series

Patch

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 66d66d77caee..47b4d1099467 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -3528,9 +3528,18 @@  static int kvm_vcpu_ioctl_x86_set_vcpu_events(struct kvm_vcpu *vcpu,
 		events->exception_has_payload = 0;
 	}
 
-	if ((events->exception.injected || events->exception.pending) &&
-	    (events->exception.nr > 31 || events->exception.nr == NMI_VECTOR))
-		return -EINVAL;
+	if (events->exception.injected || events->exception.pending) {
+		bool needs_error_code;
+
+		if (events->exception.nr > 31 ||
+		    events->exception.nr == NMI_VECTOR)
+			return -EINVAL;
+
+		needs_error_code = is_protmode(vcpu) &&
+			x86_exception_has_error_code(events->exception.nr);
+		if (!!events->exception.has_error_code != needs_error_code)
+			return -EINVAL;
+	}
 
 	/* INITs are latched while in SMM */
 	if (events->flags & KVM_VCPUEVENT_VALID_SMM &&
@@ -8312,6 +8321,18 @@  static int kvm_valid_sregs(struct kvm_vcpu *vcpu, struct kvm_sregs *sregs)
 			return -EINVAL;
 	}
 
+	/*
+	 * Switching between real-address mode and protected mode can
+	 * potentially invalidate a pending exception.
+	 */
+	if (vcpu->arch.exception.injected || vcpu->arch.exception.pending) {
+		bool needs_error_code = (sregs->cr0 & X86_CR0_PE) &&
+			x86_exception_has_error_code(vcpu->arch.exception.nr);
+
+		if (vcpu->arch.exception.has_error_code != needs_error_code)
+			return -EINVAL;
+	}
+
 	return 0;
 }