[PATCH] accel/tcg: Fix translation exception on invalid instruction

[PATCH] accel/tcg: Fix translation exception on invalid instruction

[Ilya Leoshkevich]

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(-)

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.
-- 
2.29.2

Re: [PATCH] accel/tcg: Fix translation exception on invalid instruction

[Richard Henderson]

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~