diff mbox series

accel/tcg: Fix translation exception on invalid instruction

Message ID 20210413132349.20520-1-iii@linux.ibm.com (mailing list archive)
State New
Headers show
Series accel/tcg: Fix translation exception on invalid instruction | expand

Commit Message

Ilya Leoshkevich April 13, 2021, 1:23 p.m. UTC
Hitting an uretprobe in a s390x TCG guest causes a SIGSEGV. What
happens is:

* uretprobe maps a userspace page containing an invalid instruction.
* uretprobe replaces the target function's return address with the
  address of that page.
* When tb_gen_code() is called on that page, tb->size ends up being 0
  (because the page starts with the invalid instruction), which causes
  virt_page2 to point to the previous page.
* The previous page is not mapped, so this causes a spurious
  translation exception.

Fix by special-casing tb->size == 0: since there is no useful code, we
don't need to link pages in this case.

Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
 accel/tcg/translate-all.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

Comments

Richard Henderson April 13, 2021, 3:29 p.m. UTC | #1
On 4/13/21 6:23 AM, Ilya Leoshkevich wrote:
> * When tb_gen_code() is called on that page, tb->size ends up being 0

This is the bug, in target/s390x.  Perhaps we need to add an assert that size 
!= 0 after translation...


> Fix by special-casing tb->size == 0: since there is no useful code, we
> don't need to link pages in this case.

Yes we do, because we need to link to the page to notice when changes to that 
page occur.

While this won't happen in the specific case of uretprobe, it affects every 
other instance of a TB which begins with an illegal instruction.


r~
diff mbox series

Patch

diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index ba6ab09790..77043b98c4 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -1848,7 +1848,6 @@  TranslationBlock *tb_gen_code(CPUState *cpu,
     CPUArchState *env = cpu->env_ptr;
     TranslationBlock *tb, *existing_tb;
     tb_page_addr_t phys_pc, phys_page2;
-    target_ulong virt_page2;
     tcg_insn_unit *gen_code_buf;
     int gen_code_size, search_size, max_insns;
 #ifdef CONFIG_PROFILER
@@ -2085,11 +2084,15 @@  TranslationBlock *tb_gen_code(CPUState *cpu,
     }
 
     /* check next page if needed */
-    virt_page2 = (pc + tb->size - 1) & TARGET_PAGE_MASK;
     phys_page2 = -1;
-    if ((pc & TARGET_PAGE_MASK) != virt_page2) {
-        phys_page2 = get_page_addr_code(env, virt_page2);
+    if (tb->size != 0) {
+        target_ulong virt_page2 = (pc + tb->size - 1) & TARGET_PAGE_MASK;
+
+        if ((pc & TARGET_PAGE_MASK) != virt_page2) {
+            phys_page2 = get_page_addr_code(env, virt_page2);
+        }
     }
+
     /*
      * No explicit memory barrier is required -- tb_link_page() makes the
      * TB visible in a consistent state.