Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit fdb28619 authored by Eugene Korenevsky's avatar Eugene Korenevsky Committed by Paolo Bonzini
Browse files

kvm: vmx: segment limit check: use access length 



There is an imperfection in get_vmx_mem_address(): access length is ignored
when checking the limit. To fix this, pass access length as a function argument.
The access length is usually obvious since it is used by callers after
get_vmx_mem_address() call, but for vmread/vmwrite it depends on the
state of 64-bit mode.

Signed-off-by: default avatarEugene Korenevsky <ekorenevsky@gmail.com>
Signed-off-by: default avatarPaolo Bonzini <pbonzini@redhat.com>
parent c1a9acbc

arch/x86/kvm/vmx/nested.c

+16 −12
Original line number Diff line number Diff line
@@ -4008,7 +4008,7 @@ void nested_vmx_vmexit(struct kvm_vcpu *vcpu, u32 exit_reason, 
 * #UD or #GP.

 */

int get_vmx_mem_address(struct kvm_vcpu *vcpu, unsigned long exit_qualification,

			u32 vmx_instruction_info, bool wr, gva_t *ret)

			u32 vmx_instruction_info, bool wr, int len, gva_t *ret)

{

	gva_t off;

	bool exn;
@@ -4115,7 +4115,7 @@ int get_vmx_mem_address(struct kvm_vcpu *vcpu, unsigned long exit_qualification, 
		 */

		if (!(s.base == 0 && s.limit == 0xffffffff &&

		     ((s.type & 8) || !(s.type & 4))))

			exn = exn || ((u64)off + sizeof(u64) - 1 > s.limit);

			exn = exn || ((u64)off + len - 1 > s.limit);

	}

	if (exn) {

		kvm_queue_exception_e(vcpu,
@@ -4134,7 +4134,8 @@ static int nested_vmx_get_vmptr(struct kvm_vcpu *vcpu, gpa_t *vmpointer) 
	struct x86_exception e;



	if (get_vmx_mem_address(vcpu, vmcs_readl(EXIT_QUALIFICATION),

			vmcs_read32(VMX_INSTRUCTION_INFO), false, &gva))

				vmcs_read32(VMX_INSTRUCTION_INFO), false,

				sizeof(*vmpointer), &gva))

		return 1;



	if (kvm_read_guest_virt(vcpu, gva, vmpointer, sizeof(*vmpointer), &e)) {
@@ -4386,6 +4387,7 @@ static int handle_vmread(struct kvm_vcpu *vcpu) 
	u64 field_value;

	unsigned long exit_qualification = vmcs_readl(EXIT_QUALIFICATION);

	u32 vmx_instruction_info = vmcs_read32(VMX_INSTRUCTION_INFO);

	int len;

	gva_t gva = 0;

	struct vmcs12 *vmcs12;
@@ -4423,12 +4425,12 @@ static int handle_vmread(struct kvm_vcpu *vcpu) 
		kvm_register_writel(vcpu, (((vmx_instruction_info) >> 3) & 0xf),

			field_value);

	} else {

		len = is_64_bit_mode(vcpu) ? 8 : 4;

		if (get_vmx_mem_address(vcpu, exit_qualification,

				vmx_instruction_info, true, &gva))

				vmx_instruction_info, true, len, &gva))

			return 1;

		/* _system ok, nested_vmx_check_permission has verified cpl=0 */

		kvm_write_guest_virt_system(vcpu, gva, &field_value,

					    (is_long_mode(vcpu) ? 8 : 4), NULL);

		kvm_write_guest_virt_system(vcpu, gva, &field_value, len, NULL);

	}



	return nested_vmx_succeed(vcpu);
@@ -4438,6 +4440,7 @@ static int handle_vmread(struct kvm_vcpu *vcpu) 
static int handle_vmwrite(struct kvm_vcpu *vcpu)

{

	unsigned long field;

	int len;

	gva_t gva;

	struct vcpu_vmx *vmx = to_vmx(vcpu);

	unsigned long exit_qualification = vmcs_readl(EXIT_QUALIFICATION);
@@ -4463,11 +4466,11 @@ static int handle_vmwrite(struct kvm_vcpu *vcpu) 
		field_value = kvm_register_readl(vcpu,

			(((vmx_instruction_info) >> 3) & 0xf));

	else {

		len = is_64_bit_mode(vcpu) ? 8 : 4;

		if (get_vmx_mem_address(vcpu, exit_qualification,

				vmx_instruction_info, false, &gva))

				vmx_instruction_info, false, len, &gva))

			return 1;

		if (kvm_read_guest_virt(vcpu, gva, &field_value,

					(is_64_bit_mode(vcpu) ? 8 : 4), &e)) {

		if (kvm_read_guest_virt(vcpu, gva, &field_value, len, &e)) {

			kvm_inject_page_fault(vcpu, &e);

			return 1;

		}
@@ -4615,7 +4618,8 @@ static int handle_vmptrst(struct kvm_vcpu *vcpu) 
	if (unlikely(to_vmx(vcpu)->nested.hv_evmcs))

		return 1;



	if (get_vmx_mem_address(vcpu, exit_qual, instr_info, true, &gva))

	if (get_vmx_mem_address(vcpu, exit_qual, instr_info,

				true, sizeof(gpa_t), &gva))

		return 1;

	/* *_system ok, nested_vmx_check_permission has verified cpl=0 */

	if (kvm_write_guest_virt_system(vcpu, gva, (void *)&current_vmptr,
@@ -4661,7 +4665,7 @@ static int handle_invept(struct kvm_vcpu *vcpu) 
	 * operand is read even if it isn't needed (e.g., for type==global)

	 */

	if (get_vmx_mem_address(vcpu, vmcs_readl(EXIT_QUALIFICATION),

			vmx_instruction_info, false, &gva))

			vmx_instruction_info, false, sizeof(operand), &gva))

		return 1;

	if (kvm_read_guest_virt(vcpu, gva, &operand, sizeof(operand), &e)) {

		kvm_inject_page_fault(vcpu, &e);
@@ -4723,7 +4727,7 @@ static int handle_invvpid(struct kvm_vcpu *vcpu) 
	 * operand is read even if it isn't needed (e.g., for type==global)

	 */

	if (get_vmx_mem_address(vcpu, vmcs_readl(EXIT_QUALIFICATION),

			vmx_instruction_info, false, &gva))

			vmx_instruction_info, false, sizeof(operand), &gva))

		return 1;

	if (kvm_read_guest_virt(vcpu, gva, &operand, sizeof(operand), &e)) {

		kvm_inject_page_fault(vcpu, &e);

arch/x86/kvm/vmx/nested.h

+1 −1
Original line number Diff line number Diff line
@@ -21,7 +21,7 @@ void nested_sync_from_vmcs12(struct kvm_vcpu *vcpu); 
int vmx_set_vmx_msr(struct kvm_vcpu *vcpu, u32 msr_index, u64 data);

int vmx_get_vmx_msr(struct nested_vmx_msrs *msrs, u32 msr_index, u64 *pdata);

int get_vmx_mem_address(struct kvm_vcpu *vcpu, unsigned long exit_qualification,

			u32 vmx_instruction_info, bool wr, gva_t *ret);

			u32 vmx_instruction_info, bool wr, int len, gva_t *ret);



static inline struct vmcs12 *get_vmcs12(struct kvm_vcpu *vcpu)

{

arch/x86/kvm/vmx/vmx.c

+2 −1
Original line number Diff line number Diff line
@@ -5345,7 +5345,8 @@ static int handle_invpcid(struct kvm_vcpu *vcpu) 
	 * is read even if it isn't needed (e.g., for type==all)

	 */

	if (get_vmx_mem_address(vcpu, vmcs_readl(EXIT_QUALIFICATION),

				vmx_instruction_info, false, &gva))

				vmx_instruction_info, false,

				sizeof(operand), &gva))

		return 1;



	if (kvm_read_guest_virt(vcpu, gva, &operand, sizeof(operand), &e)) {