@@ -787,6 +787,49 @@ static void test_mov_pop_ss_code_db(void)
handle_exception(DB_VECTOR, old_db_handler);
}
+static void test_indirect_call(void)
+{
+ long page1, page2, old_sp, sp, dest, tmp;
+ int status;
+
+ page1 = (long)vmap(IORAM_BASE_PHYS, PAGE_SIZE * 2);
+ page2 = page1 + PAGE_SIZE;
+
+ sp = page1 + PAGE_SIZE;
+ dest = page2 + PAGE_SIZE - sizeof(dest);
+
+ assert(((sp - sizeof(sp)) & ~PAGE_MASK) == (dest & ~PAGE_MASK));
+
+ asm volatile("mov %%"R "sp, %[old_sp]\n\t"
+ "mov %[sp], %%"R "sp\n\t"
+
+ "lea Lret_diverted, %0\n\t"
+ "mov %0, -"S"(%%"R "sp)\n\t"
+
+ "lea Lcall_target, %0\n\t"
+ "mov %0, (%[dest])\n\t"
+
+ "mov $-1, %[status]\n\t"
+ "call *(%[dest])\n\t"
+ "jmp Lout\n\t"
+
+ "Lcall_target:\n\t"
+ "endbr" xstr(BITS_PER_LONG)"\n\t"
+ "mov $0, %[status]\n\t"
+ "ret\n\t"
+
+ "Lret_diverted:\n\t"
+ "mov $1, %[status]\n\t"
+
+ "Lout:\n\t"
+ "mov %[old_sp], %%"R "sp\n\t"
+ : "=&r"(tmp), [old_sp]"=&r"(old_sp), [status]"=&r"(status)
+ : [sp]"r"(sp), [dest]"r"(dest)
+ : "memory");
+
+ report(!status, "indirect call (gpa_val), status = %d", status);
+}
+
int main(void)
{
void *mem;
@@ -834,6 +877,7 @@ int main(void)
test_string_io_mmio(mem);
test_illegal_movbe();
test_mov_pop_ss_code_db();
+ test_indirect_call();
#ifdef __x86_64__
test_emulator_64(mem);
It seems em_call_near_abs() is missing a TwoMemOp flag. If there's a match between ~PAGE_MASK (lower 12) bits of - address of memory storing CALL's target, and - stack pointer at the time of pushing the return address, emulator, while pushing the return address, will skip GVA->GPA walk and use GPA provided by nPF. See arch/x86/kvm/x86.c:emulator_read_write_onepage(). Problem is that nPF came from reading CALL's SrcMem (pointer to CALL's target), so the "push" will overwrite CALL's SrcMem, not touching the stack at all. Then RET comes along and attempts to pop the return address, which was never written. I guess it's hight time I admit I have no idea if such indirect CALL + nPF setup was meant to be supported and/or this has any real world consequences :) That said, here's a KUT testcase where the bamboozled emulator makes RET twist the execution flow. Please let me know if this is worth submitting a TwoMemOp patch. em_call_far() appears to have the same "problem". Signed-off-by: Michal Luczaj <mhal@rbox.co> --- x86/emulator.c | 44 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+)