Activity log for bug #1825359

Date Who What changed Old value New value Message
2019-04-18 12:00:19 Shahab Vahedi bug added bug
2019-04-18 12:02:34 Shahab Vahedi description commit 377b155bde451d5ac545fbdcdfbf6ca17a4228f5 Merge: c876180938 328eb60dc1 Author: Peter Maydell <peter.x@x.x> ; masked for anti-spamming purposes Date: Mon Mar 11 18:26:37 2019 +0000 https://github.com/qemu/qemu/commit/377b155bde451d5ac545fbdcdfbf6ca17a4228f5 -------------------------------------------------- cpu_ld*_code() is used for loading code data as the name suggests. Although, it begins accessing memory with MMU_INST_FETCH access type, somewhere down the road, when the "io_readx(..., access_type=MMU_INST_FETCH, ...)" is called, it is ignoring this "access_type" while calling the "tlb_fill()" with a _hardcoded_ MMU_DATA_LOAD: cputlb.c -------- static uint64_t io_readx(..., MMUAccessType access_type, ...) { if (recheck) { CPUTLBEntry *entry; target_ulong tlb_addr; tlb_fill(cpu, addr, size, MMU_DATA_LOAD, mmu_idx, retaddr); ... } -------- This is an issue, because there can exist _small_ regions of memory (smaller than the TARGET_PAGE_SIZE) that are only executable and not readable. TL;DR What happens is at first, a "tlb_fill(..., access_type=MMU_INST_FETCH, ...)" is triggered by "tb_lookup_cpu_state()". To be precise, this is the call stack which is good behavior: --- #0 tlb_fill (cs=..., vaddr=684, size=0, access_type=MMU_INST_FETCH, mmu_idx=0, retaddr=0) at target/arc/mmu.c:602 #1 get_page_addr_code (env=..., addr=684) at accel/tcg/cputlb.c:1045 #2 tb_htable_lookup (cpu=..., pc=684, cs_base=0, flags=0, cf_mask=4278190080) at accel/tcg/cpu-exec.c:337 #3 tb_lookup__cpu_state (cpu=..., pc=..., cs_base=..., flags=..., cf_mask=4278190080) at include/exec/tb-lookup.h:43 #4 tb_find (cpu=..., last_tb=... <code_gen_buffer+17811>, tb_exit=0, cf_mask=0) at accel/tcg/cpu-exec.c:404 #5 cpu_exec (cpu=...) at accel/tcg/cpu-exec.c:729 #6 tcg_cpu_exec (cpu=...) at cpus.c:1430 #7 qemu_tcg_rr_cpu_thread_fn (arg=...) at cpus.c:1531 #8 qemu_thread_start (args=...) at util/qemu-thread-posix.c:502 --- After this call, TLB is filled with an entry that its size field is small, say 32 bytes. This causes a TLB_RECHECK for consequent memory accesses, which is logical. However, in our decoder, we use cpu_lduw_code() to read the instructions and decode them. As mentioned, in the beginning, the access_type=MMU_INST_FETCH is lost in "io_readx()" while calling "tlb_fill()", and now THIS CAUSES A GUEST EXCEPTION BECAUSE THAT REGION IS NOT ALLOWED TO BE READ. Here, comes that trace call of the _bad_ behavior: --- #0 tlb_fill (..., access_type=MMU_DATA_LOAD, ...) at target/arc/mmu.c:605 #1 io_readx (..., access_type=MMU_INST_FETCH, size=2) at accel/tcg/cputlb.c:881 #2 io_readw (..., access_type=MMU_INST_FETCH) at accel/tcg/softmmu_template.h:106 #3 helper_le_ldw_cmmu (..., oi=16, retaddr=0) at accel/tcg/softmmu_template.h:146 #4 cpu_lduw_code_ra (env=..., ptr=684, retaddr=0) at include/exec/cpu_ldst_template.h:102 #5 cpu_lduw_code (env=..., ptr=684) at include/exec/cpu_ldst_template.h:114 #6 read_and_decode_context (ctx=..., opcode_p=...) at target/arc/arc-decoder.c:1479 #7 arc_decode (ctx=...) at target/arc/arc-decoder.c:1736 #8 decode_opc (env=..., ctx=...) at target/arc/translate.c:313 #9 arc_tr_translate_insn (dcbase=..., cpu=...) at target/arc/translate.c:335 #10 translator_loop (.. <code_gen_buffer+18131>) at accel/tcg/translator.c:107 #11 gen_intermediate_code (cpu=..., tb=... <code_gen_buffer+18131>) at target/arc/translate.c:413 #12 tb_gen_code (cpu=..., pc=684, cs_base=0, flags=0, cflags=-16711679) at accel/tcg/translate-all.c:1723 #13 tb_find (cpu=..., last_tb=... <code_gen_buffer+17811>, tb_exit=0, cf_mask=0) at accel/tcg/cpu-exec.c:407 #14 cpu_exec (cpu=...) at accel/tcg/cpu-exec.c:729 #15 tcg_cpu_exec (cpu=...) at cpus.c:1430 --- Do you confirm if this is an issue? Maybe there are other ways to read an instruction with MMU_INST_FETCH access that I don't know about. Last but not least, although this is not a security issue for QEMU per se, but it is hindering a security feature for the guest. commit 377b155bde451d5ac545fbdcdfbf6ca17a4228f5 Merge: c876180938 328eb60dc1 Author: Peter Maydell <peter.x@x.x> ; masked for anti-spamming purposes Date: Mon Mar 11 18:26:37 2019 +0000 https://github.com/qemu/qemu/commit/377b155bde451d5ac545fbdcdfbf6ca17a4228f5 -------------------------------------------------- cpu_ld*_code() is used for loading code data as the name suggests. Although, it begins accessing memory with MMU_INST_FETCH access type, somewhere down the road, when the "io_readx(..., access_type=MMU_INST_FETCH, ...)" is called, it is ignoring this "access_type" while calling the "tlb_fill()" with a _hardcoded_ MMU_DATA_LOAD: cputlb.c -------- static uint64_t io_readx(..., MMUAccessType access_type, ...) {     if (recheck) {         CPUTLBEntry *entry;         target_ulong tlb_addr;         tlb_fill(cpu, addr, size, MMU_DATA_LOAD, mmu_idx, retaddr);         ... } -------- This is an issue, because there can exist _small_ regions of memory (smaller than the TARGET_PAGE_SIZE) that are only executable and not readable. TL;DR What happens is at first, a "tlb_fill(..., access_type=MMU_INST_FETCH, ...)" is triggered by "tb_lookup_cpu_state()". To be precise, this is the call stack which is good behavior: --- #0 tlb_fill (cs=..., vaddr=684, size=0, access_type=MMU_INST_FETCH, mmu_idx=0, retaddr=0) at target/arc/mmu.c:602 #1 get_page_addr_code (env=..., addr=684) at accel/tcg/cputlb.c:1045 #2 tb_htable_lookup (cpu=..., pc=684, cs_base=0, flags=0, cf_mask=4278190080) at accel/tcg/cpu-exec.c:337 #3 tb_lookup__cpu_state (cpu=..., pc=..., cs_base=..., flags=..., cf_mask=4278190080) at include/exec/tb-lookup.h:43 #4 tb_find (cpu=..., last_tb=... <code_gen_buffer+17811>, tb_exit=0, cf_mask=0) at accel/tcg/cpu-exec.c:404 #5 cpu_exec (cpu=...) at accel/tcg/cpu-exec.c:729 #6 tcg_cpu_exec (cpu=...) at cpus.c:1430 #7 qemu_tcg_rr_cpu_thread_fn (arg=...) at cpus.c:1531 #8 qemu_thread_start (args=...) at util/qemu-thread-posix.c:502 --- After this call, TLB is filled with an entry that its size field is small, say 32 bytes. This causes a TLB_RECHECK for consequent memory accesses, which is logical. However, in our decoder, we use cpu_lduw_code() to read the instructions and decode them. As mentioned, in the beginning, the access_type=MMU_INST_FETCH is lost in "io_readx()" while calling tlb_fill()", and now THIS CAUSES A GUEST EXCEPTION BECAUSE THAT REGION IS NOT ALLOWED TO BE READ. Here, comes that trace call of the _bad_ behavior: --- #0 tlb_fill (..., access_type=MMU_DATA_LOAD, ...) at target/arc/mmu.c:605 #1 io_readx (..., access_type=MMU_INST_FETCH, size=2) at accel/tcg/cputlb.c:881 #2 io_readw (..., access_type=MMU_INST_FETCH) at accel/tcg/softmmu_template.h:106 #3 helper_le_ldw_cmmu (..., oi=16, retaddr=0) at accel/tcg/softmmu_template.h:146 #4 cpu_lduw_code_ra (env=..., ptr=684, retaddr=0) at include/exec/cpu_ldst_template.h:102 #5 cpu_lduw_code (env=..., ptr=684) at include/exec/cpu_ldst_template.h:114 #6 read_and_decode_context (ctx=..., opcode_p=...) at target/arc/arc-decoder.c:1479 #7 arc_decode (ctx=...) at target/arc/arc-decoder.c:1736 #8 decode_opc (env=..., ctx=...) at target/arc/translate.c:313 #9 arc_tr_translate_insn (dcbase=..., cpu=...) at target/arc/translate.c:335 #10 translator_loop (.. <code_gen_buffer+18131>) at accel/tcg/translator.c:107 #11 gen_intermediate_code (cpu=..., tb=... <code_gen_buffer+18131>) at target/arc/translate.c:413 #12 tb_gen_code (cpu=..., pc=684, cs_base=0, flags=0, cflags=-16711679) at accel/tcg/translate-all.c:1723 #13 tb_find (cpu=..., last_tb=... <code_gen_buffer+17811>, tb_exit=0, cf_mask=0) at accel/tcg/cpu-exec.c:407 #14 cpu_exec (cpu=...) at accel/tcg/cpu-exec.c:729 #15 tcg_cpu_exec (cpu=...) at cpus.c:1430 --- Do you confirm if this is an issue? Maybe there are other ways to read an instruction with MMU_INST_FETCH access that I don't know about. Last but not least, although this is not a security issue for QEMU per se, but it is hindering a security feature for the guest.
2019-04-18 12:31:15 Peter Maydell qemu: status New Confirmed
2019-04-18 13:27:29 Shahab Vahedi attachment added bug1825359_io_readx.patch https://bugs.launchpad.net/qemu/+bug/1825359/+attachment/5256724/+files/bug1825359_io_readx.patch
2019-04-18 14:11:20 Shahab Vahedi attachment added segfault_bt.txt https://bugs.launchpad.net/qemu/+bug/1825359/+attachment/5256726/+files/segfault_bt.txt
2019-04-18 18:27:09 Shahab Vahedi qemu: assignee Shahab Vahedi (shahab-vahedi)
2019-04-18 18:40:41 Shahab Vahedi attachment removed bug1825359_io_readx.patch https://bugs.launchpad.net/qemu/+bug/1825359/+attachment/5256724/+files/bug1825359_io_readx.patch
2019-04-18 18:46:01 Shahab Vahedi attachment added respect address type for tlb_fill() and while using the address from "tlb_entry" https://bugs.launchpad.net/qemu/+bug/1825359/+attachment/5256796/+files/bug1825359_io_readx.patch
2019-05-03 16:33:19 Peter Maydell qemu: status Confirmed Fix Committed
2019-08-16 04:44:57 Thomas Huth qemu: status Fix Committed Fix Released