Skip to content

Commit 9b37e14

Browse files
yonghuahjren1
authored andcommitted
add IO requrest 'req_buf' check before reference
This address maybe invalid if a hostile address was set in hypercall 'HC_SET_IOREQ_BUFFER'.it should be validated before using. Update: -- save HVA to guest OS's request buffer in hyperviosr -- change type of 'req_buf' from 'uint64_t' to 'void *' -- remove HPA to HVA translation code when using this addr. -- use error number instead of -1 when return error cases. Signed-off-by: Yonghua Huang <yonghua.huang@intel.com>
1 parent 3a3aeac commit 9b37e14

File tree

5 files changed

+29
-19
lines changed

5 files changed

+29
-19
lines changed

hypervisor/arch/x86/ept.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -344,8 +344,9 @@ int dm_emulate_mmio_post(struct vcpu *vcpu)
344344
{
345345
int ret = 0;
346346
int cur = vcpu->vcpu_id;
347-
struct vhm_request_buffer *req_buf =
348-
(void *)HPA2HVA(vcpu->vm->sw.req_buf);
347+
struct vhm_request_buffer *req_buf;
348+
349+
req_buf = (struct vhm_request_buffer *)(vcpu->vm->sw.req_buf);
349350

350351
vcpu->req.reqs.mmio_request.value =
351352
req_buf->req_queue[cur].reqs.mmio_request.value;

hypervisor/arch/x86/guest/vm.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@ int create_vm(struct vm_description *vm_desc, struct vm **rtn_vm)
170170

171171
/* Populate return VM handle */
172172
*rtn_vm = vm;
173-
vm->sw.req_buf = 0;
173+
vm->sw.req_buf = NULL;
174174

175175
status = set_vcpuid_entries(vm);
176176
if (status)

hypervisor/arch/x86/io.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,12 +39,13 @@ int dm_emulate_pio_post(struct vcpu *vcpu)
3939
{
4040
int cur = vcpu->vcpu_id;
4141
int cur_context = vcpu->arch_vcpu.cur_context;
42-
struct vhm_request_buffer *req_buf =
43-
(void *)HPA2HVA(vcpu->vm->sw.req_buf);
42+
struct vhm_request_buffer *req_buf = NULL;
4443
uint32_t mask =
4544
0xFFFFFFFFul >> (32 - 8 * vcpu->req.reqs.pio_request.size);
4645
uint64_t *rax;
4746

47+
req_buf = (struct vhm_request_buffer *)(vcpu->vm->sw.req_buf);
48+
4849
rax = &vcpu->arch_vcpu.contexts[cur_context].guest_cpu_regs.regs.rax;
4950
vcpu->req.reqs.pio_request.value =
5051
req_buf->req_queue[cur].reqs.pio_request.value;

hypervisor/common/hypercall.c

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -213,7 +213,7 @@ int64_t hcall_resume_vm(uint64_t vmid)
213213

214214
if (target_vm == NULL)
215215
return -1;
216-
if (target_vm->sw.req_buf == 0)
216+
if (target_vm->sw.req_buf == NULL)
217217
ret = -1;
218218
else
219219
ret = start_vm(target_vm);
@@ -323,6 +323,7 @@ int64_t hcall_inject_msi(struct vm *vm, uint64_t vmid, uint64_t param)
323323
int64_t hcall_set_ioreq_buffer(struct vm *vm, uint64_t vmid, uint64_t param)
324324
{
325325
int64_t ret = 0;
326+
uint64_t hpa = 0;
326327
struct acrn_set_ioreq_buffer iobuf;
327328
struct vm *target_vm = get_vm_from_vmid(vmid);
328329

@@ -336,11 +337,17 @@ int64_t hcall_set_ioreq_buffer(struct vm *vm, uint64_t vmid, uint64_t param)
336337
return -1;
337338
}
338339

339-
dev_dbg(ACRN_DBG_HYCALL, "[%d] SET BUFFER=0x%x",
340+
dev_dbg(ACRN_DBG_HYCALL, "[%d] SET BUFFER=0x%p",
340341
vmid, iobuf.req_buf);
341342

342-
/* store gpa of guest request_buffer */
343-
target_vm->sw.req_buf = gpa2hpa(vm, iobuf.req_buf);
343+
hpa = gpa2hpa(vm, iobuf.req_buf);
344+
if (hpa == 0) {
345+
pr_err("%s: invalid GPA.\n", __func__);
346+
target_vm->sw.req_buf = NULL;
347+
return -EINVAL;
348+
}
349+
350+
target_vm->sw.req_buf = HPA2HVA(hpa);
344351

345352
return ret;
346353
}
@@ -386,8 +393,8 @@ int64_t hcall_notify_req_finish(uint64_t vmid, uint64_t vcpu_id)
386393
struct vm *target_vm = get_vm_from_vmid(vmid);
387394

388395
/* make sure we have set req_buf */
389-
if (!target_vm || target_vm->sw.req_buf == 0)
390-
return -1;
396+
if (!target_vm || target_vm->sw.req_buf == NULL)
397+
return -EINVAL;
391398

392399
dev_dbg(ACRN_DBG_HYCALL, "[%d] NOTIFY_FINISH for vcpu %d",
393400
vmid, vcpu_id);
@@ -779,16 +786,17 @@ static void acrn_print_request(int vcpu_id, struct vhm_request *req)
779786

780787
int acrn_insert_request_wait(struct vcpu *vcpu, struct vhm_request *req)
781788
{
782-
struct vhm_request_buffer *req_buf =
783-
(void *)HPA2HVA(vcpu->vm->sw.req_buf);
789+
struct vhm_request_buffer *req_buf = NULL;
784790
long cur;
785791

786792
ASSERT(sizeof(*req) == (4096/VHM_REQUEST_MAX),
787793
"vhm_request page broken!");
788794

789795

790-
if (!vcpu || !req || vcpu->vm->sw.req_buf == 0)
791-
return -1;
796+
if (!vcpu || !req || vcpu->vm->sw.req_buf == NULL)
797+
return -EINVAL;
798+
799+
req_buf = (struct vhm_request_buffer *)(vcpu->vm->sw.req_buf);
792800

793801
/* ACRN insert request to VHM and inject upcall */
794802
cur = vcpu->vcpu_id;
@@ -820,9 +828,9 @@ int acrn_insert_request_nowait(struct vcpu *vcpu, struct vhm_request *req)
820828
long cur;
821829

822830
if (!vcpu || !req || !vcpu->vm->sw.req_buf)
823-
return -1;
831+
return -EINVAL;
824832

825-
req_buf = (void *)gpa2hpa(vcpu->vm, vcpu->vm->sw.req_buf);
833+
req_buf = (struct vhm_request_buffer *)(vcpu->vm->sw.req_buf);
826834

827835
/* ACRN insert request to VHM and inject upcall */
828836
cur = vcpu->vcpu_id;

hypervisor/include/arch/x86/guest/vm.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,8 +76,8 @@ struct vm_sw_info {
7676
struct sw_kernel_info kernel_info;
7777
/* Additional information specific to Linux guests */
7878
struct sw_linux linux_info;
79-
/* GPA Address of guest OS's request buffer */
80-
uint64_t req_buf;
79+
/* HVA to guest OS's request buffer */
80+
void *req_buf;
8181
};
8282

8383
struct vm_pm_info {

0 commit comments

Comments
 (0)