[Qemu-devel] [Bug 1825359] [NEW] cpu_ld*_code() triggers MMU_DATA_LOAD i.s.o. MMU_INST_FETCH

[Shahab Vahedi]

Public bug reported:

commit 377b155bde451d5ac545fbdcdfbf6ca17a4228f5
Merge: c876180938 328eb60dc1
Author: Peter Maydell <address@hidden>        ; 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.

** Affects: qemu
     Importance: Undecided
         Status: New


** Tags: fetch mmu mpu

** Description changed:

  commit 377b155bde451d5ac545fbdcdfbf6ca17a4228f5
  Merge: c876180938 328eb60dc1
  Author: Peter Maydell <address@hidden>        ; 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:
+ 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);
-         ...
+     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.
+ 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:
+ 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:
+ 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              
+ #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                     
+ #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.
+ 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.
+ 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.

-- 
You received this bug notification because you are a member of qemu-
devel-ml, which is subscribed to QEMU.
https://bugs.launchpad.net/bugs/1825359

Title:
  cpu_ld*_code() triggers MMU_DATA_LOAD i.s.o. MMU_INST_FETCH

Status in QEMU:
  New

Bug description:
  commit 377b155bde451d5ac545fbdcdfbf6ca17a4228f5
  Merge: c876180938 328eb60dc1
  Author: Peter Maydell <address@hidden>        ; 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.

To manage notifications about this bug go to:
https://bugs.launchpad.net/qemu/+bug/1825359/+subscriptions